Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.13] [ES|QL] Fix some validation misconfiguration (#177783) #178252

Merged
merged 2 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,18 @@ describe('autocomplete', () => {
`from a | ${command} stringField, `,
getFieldNamesByType('any').filter((name) => name !== 'stringField')
);

testSuggestions(
`from a_index | eval round(numberField) + 1 | eval \`round(numberField) + 1\` + 1 | eval \`\`\`round(numberField) + 1\`\` + 1\` + 1 | eval \`\`\`\`\`\`\`round(numberField) + 1\`\`\`\` + 1\`\` + 1\` + 1 | eval \`\`\`\`\`\`\`\`\`\`\`\`\`\`\`round(numberField) + 1\`\`\`\`\`\`\`\` + 1\`\`\`\` + 1\`\` + 1\` + 1 | ${command} `,
[
...getFieldNamesByType('any'),
'`round(numberField) + 1`',
'```round(numberField) + 1`` + 1`',
'```````round(numberField) + 1```` + 1`` + 1`',
'```````````````round(numberField) + 1```````` + 1```` + 1`` + 1`',
'```````````````````````````````round(numberField) + 1```````````````` + 1```````` + 1```` + 1`` + 1`',
]
);
});
}

Expand Down Expand Up @@ -927,10 +939,7 @@ describe('autocomplete', () => {
[
'var0 =',
...getFieldNamesByType('any'),
// @TODO: leverage the location data to get the original text
// For now return back the trimmed version:
// the ANTLR parser trims all text so that's what it's stored in the AST
'`abs(numberField)+1`',
'`abs(numberField) + 1`',
...getFunctionSignaturesByReturnType('eval', 'any', { evalMath: true }),
],
' '
Expand Down
32 changes: 22 additions & 10 deletions packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import {
buildOptionDefinition,
buildSettingDefinitions,
} from './factories';
import { EDITOR_MARKER } from '../shared/constants';
import { EDITOR_MARKER, SINGLE_BACKTICK } from '../shared/constants';
import { getAstContext, removeMarkerArgFromArgsList } from '../shared/context';
import {
buildQueryUntilPreviousCommand,
Expand Down Expand Up @@ -563,7 +563,7 @@ async function getExpressionSuggestionsByType(

// collect all fields + variables to suggest
const fieldsMap: Map<string, ESQLRealField> = await (argDef ? getFieldsMap() : new Map());
const anyVariables = collectVariables(commands, fieldsMap);
const anyVariables = collectVariables(commands, fieldsMap, innerText);

// enrich with assignment has some special rules who are handled somewhere else
const canHaveAssignments = ['eval', 'stats', 'row'].includes(command.name);
Expand Down Expand Up @@ -1017,13 +1017,20 @@ async function getFieldsOrFunctionsSuggestions(
}
// due to a bug on the ES|QL table side, filter out fields list with underscored variable names (??)
// avg( numberField ) => avg_numberField_
const ALPHANUMERIC_REGEXP = /[^a-zA-Z\d]/g;
if (
filteredVariablesByType.length &&
filteredVariablesByType.some((v) => /[^a-zA-Z\d]/.test(v))
filteredVariablesByType.some((v) => ALPHANUMERIC_REGEXP.test(v))
) {
for (const variable of filteredVariablesByType) {
const underscoredName = variable.replace(/[^a-zA-Z\d]/g, '_');
const index = filteredFieldsByType.findIndex(({ label }) => underscoredName === label);
// remove backticks if present
const sanitizedVariable = variable.startsWith(SINGLE_BACKTICK)
? variable.slice(1, variable.length - 1)
: variable;
const underscoredName = sanitizedVariable.replace(ALPHANUMERIC_REGEXP, '_');
const index = filteredFieldsByType.findIndex(
({ label }) => underscoredName === label || `_${underscoredName}_` === label
);
if (index >= 0) {
filteredFieldsByType.splice(index);
}
Expand Down Expand Up @@ -1067,7 +1074,8 @@ async function getFunctionArgsSuggestions(
const variablesExcludingCurrentCommandOnes = excludeVariablesFromCurrentCommand(
commands,
command,
fieldsMap
fieldsMap,
innerText
);
// pick the type of the next arg
const shouldGetNextArgument = node.text.includes(EDITOR_MARKER);
Expand Down Expand Up @@ -1102,7 +1110,10 @@ async function getFunctionArgsSuggestions(
const isUnknownColumn =
arg &&
isColumnItem(arg) &&
!columnExists(arg, { fields: fieldsMap, variables: variablesExcludingCurrentCommandOnes }).hit;
!columnExists(arg, {
fields: fieldsMap,
variables: variablesExcludingCurrentCommandOnes,
}).hit;
if (noArgDefined || isUnknownColumn) {
const commandArgIndex = command.args.findIndex(
(cmdArg) => isSingleItem(cmdArg) && cmdArg.location.max >= node.location.max
Expand Down Expand Up @@ -1213,7 +1224,7 @@ async function getListArgsSuggestions(
// so extract the type of the first argument and suggest fields of that type
if (node && isFunctionItem(node)) {
const fieldsMap: Map<string, ESQLRealField> = await getFieldsMaps();
const anyVariables = collectVariables(commands, fieldsMap);
const anyVariables = collectVariables(commands, fieldsMap, innerText);
// extract the current node from the variables inferred
anyVariables.forEach((values, key) => {
if (values.some((v) => v.location === node.location)) {
Expand Down Expand Up @@ -1301,7 +1312,7 @@ async function getOptionArgsSuggestions(
const isNewExpression = isRestartingExpression(innerText) || option.args.length === 0;

const fieldsMap = await getFieldsMaps();
const anyVariables = collectVariables(commands, fieldsMap);
const anyVariables = collectVariables(commands, fieldsMap, innerText);

const references = {
fields: fieldsMap,
Expand Down Expand Up @@ -1339,7 +1350,8 @@ async function getOptionArgsSuggestions(
const policyMetadata = await getPolicyMetadata(policyName);
const anyEnhancedVariables = collectVariables(
commands,
appendEnrichFields(fieldsMap, policyMetadata)
appendEnrichFields(fieldsMap, policyMetadata),
innerText
);

if (isNewExpression) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export const buildFieldsDefinitions = (fields: string[]): AutocompleteCommandDef
export const buildVariablesDefinitions = (variables: string[]): AutocompleteCommandDefinition[] =>
variables.map((label) => ({
label,
insertText: getSafeInsertText(label),
insertText: label,
kind: 4,
detail: i18n.translate('monaco.esql.autocomplete.variableDefinition', {
defaultMessage: `Variable specified by the user within the ES|QL query`,
Expand Down
5 changes: 4 additions & 1 deletion packages/kbn-monaco/src/esql/lib/ast/definitions/aggs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [
supportedCommands: ['stats'],
signatures: [
{
params: [{ name: 'column', type: 'any', noNestingFunctions: true }],
params: [
{ name: 'column', type: 'any', noNestingFunctions: true },
{ name: 'precision', type: 'number', noNestingFunctions: true, optional: true },
],
returnType: 'number',
examples: [
`from index | stats result = count_distinct(field)`,
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-monaco/src/esql/lib/ast/shared/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

export const EDITOR_MARKER = 'marker_esql_editor';

export const TICKS_REGEX = /^(`)|(`)$/g;
export const TICKS_REGEX = /^`{1}|`{1}$/g;
export const DOUBLE_TICKS_REGEX = /``/g;
export const SINGLE_TICK_REGEX = /`/g;
export const SINGLE_BACKTICK = '`';
Expand Down
15 changes: 7 additions & 8 deletions packages/kbn-monaco/src/esql/lib/ast/shared/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ export function isEqualType(
item: ESQLSingleAstItem,
argDef: SignatureArgType,
references: ReferenceMaps,
parentCommand?: string
parentCommand?: string,
nameHit?: string
) {
const argType = 'innerType' in argDef && argDef.innerType ? argDef.innerType : argDef.type;
if (argType === 'any') {
Expand All @@ -375,10 +376,8 @@ export function isEqualType(
// anything goes, so avoid any effort here
return true;
}
// perform a double check, but give priority to the non trimmed version
const hit = getColumnHit(item.name, references);
const hitTrimmed = getColumnHit(item.name.replace(/\s/g, ''), references);
const validHit = hit || hitTrimmed;
const hit = getColumnHit(nameHit ?? item.name, references);
const validHit = hit;
if (!validHit) {
return false;
}
Expand Down Expand Up @@ -462,9 +461,9 @@ export function columnExists(
return { hit: true, nameHit: column.name };
}
if (column.quoted) {
const trimmedName = column.name.replace(/`/g, '``').replace(/\s/g, '');
if (variables.has(trimmedName)) {
return { hit: true, nameHit: trimmedName };
const originalName = column.text;
if (variables.has(originalName)) {
return { hit: true, nameHit: originalName };
}
}
if (
Expand Down
133 changes: 67 additions & 66 deletions packages/kbn-monaco/src/esql/lib/ast/shared/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* Side Public License, v 1.
*/

import type { ESQLColumn, ESQLAstItem, ESQLCommand, ESQLCommandOption } from '../types';
import type { ESQLAstItem, ESQLCommand, ESQLCommandOption, ESQLFunction } from '../types';
import type { ESQLVariable, ESQLRealField } from '../validation/types';
import { DOUBLE_TICKS_REGEX, EDITOR_MARKER, SINGLE_BACKTICK, TICKS_REGEX } from './constants';
import { DOUBLE_BACKTICK, EDITOR_MARKER, SINGLE_BACKTICK } from './constants';
import {
isColumnItem,
isAssignment,
Expand All @@ -26,21 +26,6 @@ function addToVariableOccurrencies(variables: Map<string, ESQLVariable[]>, insta
variablesOccurrencies.push(instance);
}

function replaceTrimmedVariable(
variables: Map<string, ESQLVariable[]>,
newRef: ESQLColumn,
oldRef: ESQLVariable[]
) {
// now replace the existing trimmed version with this original one
addToVariableOccurrencies(variables, {
name: newRef.name,
type: oldRef[0].type,
location: newRef.location,
});
// remove the trimmed one
variables.delete(oldRef[0].name);
}

function addToVariables(
oldArg: ESQLAstItem,
newArg: ESQLAstItem,
Expand All @@ -55,20 +40,11 @@ function addToVariables(
};
// Now workout the exact type
// it can be a rename of another variable as well
let oldRef = fields.get(oldArg.name) || variables.get(oldArg.name);
const oldRef =
fields.get(oldArg.name) || variables.get(oldArg.quoted ? oldArg.text : oldArg.name);
if (oldRef) {
addToVariableOccurrencies(variables, newVariable);
newVariable.type = Array.isArray(oldRef) ? oldRef[0].type : oldRef.type;
} else if (oldArg.quoted) {
// a last attempt in case the user tried to rename an expression:
// trim every space and try a new hit
const expressionTrimmedRef = oldArg.name.replace(/\s/g, '');
oldRef = variables.get(expressionTrimmedRef);
if (oldRef) {
addToVariableOccurrencies(variables, newVariable);
newVariable.type = oldRef[0].type;
replaceTrimmedVariable(variables, oldArg, oldRef);
}
}
}
}
Expand Down Expand Up @@ -99,10 +75,11 @@ function getAssignRightHandSideType(item: ESQLAstItem, fields: Map<string, ESQLR
export function excludeVariablesFromCurrentCommand(
commands: ESQLCommand[],
currentCommand: ESQLCommand,
fieldsMap: Map<string, ESQLRealField>
fieldsMap: Map<string, ESQLRealField>,
queryString: string
) {
const anyVariables = collectVariables(commands, fieldsMap);
const currentCommandVariables = collectVariables([currentCommand], fieldsMap);
const anyVariables = collectVariables(commands, fieldsMap, queryString);
const currentCommandVariables = collectVariables([currentCommand], fieldsMap, queryString);
const resultVariables = new Map<string, ESQLVariable[]>();
anyVariables.forEach((value, key) => {
if (!currentCommandVariables.has(key)) {
Expand All @@ -112,55 +89,78 @@ export function excludeVariablesFromCurrentCommand(
return resultVariables;
}

function extractExpressionAsQuotedVariable(
originalQuery: string,
location: { min: number; max: number }
) {
const extractExpressionText = originalQuery.substring(location.min, location.max + 1);
// now inject quotes and save it as variable
return `\`${extractExpressionText.replaceAll(SINGLE_BACKTICK, DOUBLE_BACKTICK)}\``;
}

function addVariableFromAssignment(
assignOperation: ESQLFunction,
variables: Map<string, ESQLVariable[]>,
fields: Map<string, ESQLRealField>
) {
if (isColumnItem(assignOperation.args[0])) {
const rightHandSideArgType = getAssignRightHandSideType(assignOperation.args[1], fields);
addToVariableOccurrencies(variables, {
name: assignOperation.args[0].name,
type: rightHandSideArgType || 'number' /* fallback to number */,
location: assignOperation.args[0].location,
});
}
}

function addVariableFromExpression(
expressionOperation: ESQLFunction,
queryString: string,
variables: Map<string, ESQLVariable[]>
) {
if (!expressionOperation.text.includes(EDITOR_MARKER)) {
// save the variable in its quoted usable way
// (a bit of forward thinking here to simplyfy lookups later)
const forwardThinkingVariableName = extractExpressionAsQuotedVariable(
queryString,
expressionOperation.location
);
const expressionType = 'number';
addToVariableOccurrencies(variables, {
name: forwardThinkingVariableName,
type: expressionType,
location: expressionOperation.location,
});
}
}

export function collectVariables(
commands: ESQLCommand[],
fields: Map<string, ESQLRealField>
fields: Map<string, ESQLRealField>,
queryString: string
): Map<string, ESQLVariable[]> {
const variables = new Map<string, ESQLVariable[]>();
for (const command of commands) {
if (['row', 'eval', 'stats'].includes(command.name)) {
const assignOperations = command.args.filter(isAssignment);
for (const assignOperation of assignOperations) {
if (isColumnItem(assignOperation.args[0])) {
const rightHandSideArgType = getAssignRightHandSideType(assignOperation.args[1], fields);
addToVariableOccurrencies(variables, {
name: assignOperation.args[0].name,
type: rightHandSideArgType || 'number' /* fallback to number */,
location: assignOperation.args[0].location,
});
for (const arg of command.args) {
if (isAssignment(arg)) {
addVariableFromAssignment(arg, variables, fields);
}
}
const expressionOperations = command.args.filter(isExpression);
for (const expressionOperation of expressionOperations) {
if (!expressionOperation.text.includes(EDITOR_MARKER)) {
// just save the entire expression as variable string
const expressionType = 'number';
addToVariableOccurrencies(variables, {
name: expressionOperation.text
.replace(TICKS_REGEX, '')
.replace(DOUBLE_TICKS_REGEX, SINGLE_BACKTICK),
type: expressionType,
location: expressionOperation.location,
});
if (isExpression(arg)) {
addVariableFromExpression(arg, queryString, variables);
}
}
if (command.name === 'stats') {
const commandOptionsWithAssignment = command.args.filter(
(arg) => isOptionItem(arg) && arg.name === 'by'
) as ESQLCommandOption[];
for (const commandOption of commandOptionsWithAssignment) {
const optionAssignOperations = commandOption.args.filter(isAssignment);
for (const assignOperation of optionAssignOperations) {
if (isColumnItem(assignOperation.args[0])) {
const rightHandSideArgType = getAssignRightHandSideType(
assignOperation.args[1],
fields
);
addToVariableOccurrencies(variables, {
name: assignOperation.args[0].name,
type: rightHandSideArgType || 'number' /* fallback to number */,
location: assignOperation.args[0].location,
});
for (const optArg of commandOption.args) {
if (isAssignment(optArg)) {
addVariableFromAssignment(optArg, variables, fields);
}
if (isExpression(optArg)) {
addVariableFromExpression(optArg, queryString, variables);
}
}
}
Expand All @@ -171,6 +171,7 @@ export function collectVariables(
(arg) => isOptionItem(arg) && arg.name === 'with'
) as ESQLCommandOption[];
for (const commandOption of commandOptionsWithAssignment) {
// Enrich assignment has some special behaviour, so do not use the version above here...
for (const assignFn of commandOption.args) {
if (isFunctionItem(assignFn)) {
const [newArg, oldArg] = assignFn?.args || [];
Expand Down
Loading