diff --git a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts index 8c24c7186cc01..38f26de275577 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts @@ -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`', + ] + ); }); } @@ -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 }), ], ' ' diff --git a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts index 93ef31c0914c8..6ac334ffe6229 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts @@ -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, @@ -563,7 +563,7 @@ async function getExpressionSuggestionsByType( // collect all fields + variables to suggest const fieldsMap: Map = 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); @@ -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); } @@ -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); @@ -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 @@ -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 = 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)) { @@ -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, @@ -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) { diff --git a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts index 312bd981298eb..a51a754ef1b2e 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts @@ -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`, diff --git a/packages/kbn-monaco/src/esql/lib/ast/definitions/aggs.ts b/packages/kbn-monaco/src/esql/lib/ast/definitions/aggs.ts index c0b3dcd97c787..36615b265dbc1 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/definitions/aggs.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/definitions/aggs.ts @@ -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)`, diff --git a/packages/kbn-monaco/src/esql/lib/ast/shared/constants.ts b/packages/kbn-monaco/src/esql/lib/ast/shared/constants.ts index fffd8af2f68ec..ed4e854f95e97 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/shared/constants.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/shared/constants.ts @@ -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 = '`'; diff --git a/packages/kbn-monaco/src/esql/lib/ast/shared/helpers.ts b/packages/kbn-monaco/src/esql/lib/ast/shared/helpers.ts index 7060ce40aecc2..93826be54bf5b 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/shared/helpers.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/shared/helpers.ts @@ -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') { @@ -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; } @@ -445,9 +444,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 ( diff --git a/packages/kbn-monaco/src/esql/lib/ast/shared/variables.ts b/packages/kbn-monaco/src/esql/lib/ast/shared/variables.ts index 6dd05b788d4d2..f6f8d1cfaf3dd 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/shared/variables.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/shared/variables.ts @@ -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, @@ -26,21 +26,6 @@ function addToVariableOccurrencies(variables: Map, insta variablesOccurrencies.push(instance); } -function replaceTrimmedVariable( - variables: Map, - 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, @@ -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); - } } } } @@ -99,10 +75,11 @@ function getAssignRightHandSideType(item: ESQLAstItem, fields: Map + fieldsMap: Map, + 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(); anyVariables.forEach((value, key) => { if (!currentCommandVariables.has(key)) { @@ -112,36 +89,65 @@ 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, + fields: Map +) { + 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 +) { + 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 + fields: Map, + queryString: string ): Map { const variables = new Map(); 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') { @@ -149,18 +155,12 @@ export function collectVariables( (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); } } } @@ -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 || []; diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/esql_validation_meta_tests.json b/packages/kbn-monaco/src/esql/lib/ast/validation/esql_validation_meta_tests.json index 8a39049ab08c9..6a6331d9c0bce 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/esql_validation_meta_tests.json +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/esql_validation_meta_tests.json @@ -3486,6 +3486,78 @@ "query": "from a_index | where geoPointField Is nOt NuLL", "error": false }, + { + "query": "from a_index | where avg(numberField)", + "error": true + }, + { + "query": "from a_index | where avg(numberField) > 0", + "error": true + }, + { + "query": "from a_index | where max(numberField)", + "error": true + }, + { + "query": "from a_index | where max(numberField) > 0", + "error": true + }, + { + "query": "from a_index | where min(numberField)", + "error": true + }, + { + "query": "from a_index | where min(numberField) > 0", + "error": true + }, + { + "query": "from a_index | where sum(numberField)", + "error": true + }, + { + "query": "from a_index | where sum(numberField) > 0", + "error": true + }, + { + "query": "from a_index | where median(numberField)", + "error": true + }, + { + "query": "from a_index | where median(numberField) > 0", + "error": true + }, + { + "query": "from a_index | where median_absolute_deviation(numberField)", + "error": true + }, + { + "query": "from a_index | where median_absolute_deviation(numberField) > 0", + "error": true + }, + { + "query": "from a_index | where percentile(numberField, 5)", + "error": true + }, + { + "query": "from a_index | where percentile(numberField, 5) > 0", + "error": true + }, + { + "query": "from a_index | where count(stringField)", + "error": true + }, + { + "query": "from a_index | where count(stringField) > 0", + "error": true + }, + { + "query": "from a_index | where count_distinct(stringField, numberField)", + "error": true + }, + { + "query": "from a_index | where count_distinct(stringField, numberField) > 0", + "error": true + }, { "query": "from a_index | where abs(numberField) > 0", "error": false @@ -4426,6 +4498,182 @@ "query": "from a_index | eval %+ numberField", "error": true }, + { + "query": "from a_index | eval var = avg(numberField)", + "error": true + }, + { + "query": "from a_index | eval var = avg(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval avg(numberField)", + "error": true + }, + { + "query": "from a_index | eval avg(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = max(numberField)", + "error": true + }, + { + "query": "from a_index | eval var = max(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval max(numberField)", + "error": true + }, + { + "query": "from a_index | eval max(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = min(numberField)", + "error": true + }, + { + "query": "from a_index | eval var = min(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval min(numberField)", + "error": true + }, + { + "query": "from a_index | eval min(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = sum(numberField)", + "error": true + }, + { + "query": "from a_index | eval var = sum(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval sum(numberField)", + "error": true + }, + { + "query": "from a_index | eval sum(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = median(numberField)", + "error": true + }, + { + "query": "from a_index | eval var = median(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval median(numberField)", + "error": true + }, + { + "query": "from a_index | eval median(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = median_absolute_deviation(numberField)", + "error": true + }, + { + "query": "from a_index | eval var = median_absolute_deviation(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval median_absolute_deviation(numberField)", + "error": true + }, + { + "query": "from a_index | eval median_absolute_deviation(numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = percentile(numberField, 5)", + "error": true + }, + { + "query": "from a_index | eval var = percentile(numberField, 5) > 0", + "error": true + }, + { + "query": "from a_index | eval percentile(numberField, 5)", + "error": true + }, + { + "query": "from a_index | eval percentile(numberField, 5) > 0", + "error": true + }, + { + "query": "from a_index | eval var = count(stringField)", + "error": true + }, + { + "query": "from a_index | eval var = count(stringField) > 0", + "error": true + }, + { + "query": "from a_index | eval count(stringField)", + "error": true + }, + { + "query": "from a_index | eval count(stringField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = count_distinct(stringField, numberField)", + "error": true + }, + { + "query": "from a_index | eval var = count_distinct(stringField, numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval count_distinct(stringField, numberField)", + "error": true + }, + { + "query": "from a_index | eval count_distinct(stringField, numberField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = st_centroid(cartesianPointField)", + "error": true + }, + { + "query": "from a_index | eval var = st_centroid(cartesianPointField) > 0", + "error": true + }, + { + "query": "from a_index | eval st_centroid(cartesianPointField)", + "error": true + }, + { + "query": "from a_index | eval st_centroid(cartesianPointField) > 0", + "error": true + }, + { + "query": "from a_index | eval var = st_centroid(geoPointField)", + "error": true + }, + { + "query": "from a_index | eval var = st_centroid(geoPointField) > 0", + "error": true + }, + { + "query": "from a_index | eval st_centroid(geoPointField)", + "error": true + }, + { + "query": "from a_index | eval st_centroid(geoPointField) > 0", + "error": true + }, { "query": "from a_index | eval var = abs(numberField)", "error": false @@ -7979,27 +8227,27 @@ "error": false }, { - "query": "from a_index | stats var = count_distinct(stringField)", + "query": "from a_index | stats var = count_distinct(stringField, numberField)", "error": false }, { - "query": "from a_index | stats count_distinct(stringField)", + "query": "from a_index | stats count_distinct(stringField, numberField)", "error": false }, { - "query": "from a_index | stats var = round(count_distinct(stringField))", + "query": "from a_index | stats var = round(count_distinct(stringField, numberField))", "error": false }, { - "query": "from a_index | stats round(count_distinct(stringField))", + "query": "from a_index | stats round(count_distinct(stringField, numberField))", "error": false }, { - "query": "from a_index | stats var = round(count_distinct(stringField)) + count_distinct(stringField)", + "query": "from a_index | stats var = round(count_distinct(stringField, numberField)) + count_distinct(stringField, numberField)", "error": false }, { - "query": "from a_index | stats round(count_distinct(stringField)) + count_distinct(stringField)", + "query": "from a_index | stats round(count_distinct(stringField, numberField)) + count_distinct(stringField, numberField)", "error": false }, { @@ -8054,6 +8302,10 @@ "query": "FROM index\n | EVAL numberField * 3.281\n | STATS avg_numberField = AVG(`numberField * 3.281`)", "error": false }, + { + "query": "FROM index | STATS AVG(numberField) by round(numberField) + 1 | EVAL `round(numberField) + 1` / 2", + "error": false + }, { "query": "from a_index | sort ", "error": true @@ -8353,6 +8605,22 @@ { "query": "from a_index | eval numberField = \"5\"", "error": false + }, + { + "query": "from a_index | eval round(numberField) + 1 | eval `round(numberField) + 1` + 1 | keep ```round(numberField) + 1`` + 1`", + "error": false + }, + { + "query": "from a_index | eval round(numberField) + 1 | eval `round(numberField) + 1` + 1 | eval ```round(numberField) + 1`` + 1` + 1 | keep ```````round(numberField) + 1```` + 1`` + 1`", + "error": false + }, + { + "query": "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 | keep ```````````````round(numberField) + 1```````` + 1```` + 1`` + 1`", + "error": false + }, + { + "query": "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 | keep ```````````````````````````````round(numberField) + 1```````````````` + 1```````` + 1```` + 1`` + 1`", + "error": false } ] } \ No newline at end of file diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/types.ts b/packages/kbn-monaco/src/esql/lib/ast/validation/types.ts index b93bb895b6621..dd3cf92248734 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/types.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/types.ts @@ -33,6 +33,7 @@ export interface ReferenceMaps { fields: Map; policies: Map; metadataFields: Set; + query: string; } export interface ValidationErrors { diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts index 10a4a2135fd6d..c30a13c83abdb 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts @@ -60,6 +60,11 @@ const policies = [ }, ]; +const NESTING_LEVELS = 4; +const NESTED_DEPTHS = Array(NESTING_LEVELS) + .fill(0) + .map((_, i) => i + 1); + function getCallbackMocks() { return { getFieldsFor: jest.fn(async ({ query }) => { @@ -962,7 +967,7 @@ describe('validation logic', () => { ]); } - for (const nesting of [1, 2, 3, 4]) { + for (const nesting of NESTED_DEPTHS) { for (const evenOp of ['-', '+']) { for (const oddOp of ['-', '+']) { // This builds a combination of +/- operators @@ -1055,6 +1060,48 @@ describe('validation logic', () => { testErrorsAndWarnings(`from a_index | where ${camelCase(field)}Field Is nOt NuLL`, []); } + for (const { + name, + alias, + signatures, + ...defRest + } of statsAggregationFunctionDefinitions.filter( + ({ name: fnName, signatures: statsSignatures }) => + statsSignatures.some(({ returnType, params }) => ['number'].includes(returnType)) + )) { + for (const { params, infiniteParams, ...signRest } of signatures) { + const fieldMapping = getFieldMapping(params); + + testErrorsAndWarnings( + `from a_index | where ${ + getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMapping, ...signRest }], + }, + { withTypes: false } + )[0].declaration + }`, + [`WHERE does not support function ${name}`] + ); + + testErrorsAndWarnings( + `from a_index | where ${ + getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMapping, ...signRest }], + }, + { withTypes: false } + )[0].declaration + } > 0`, + [`WHERE does not support function ${name}`] + ); + } + } + // Test that all functions work in where const numericOrStringFunctions = evalFunctionsDefinitions.filter(({ name, signatures }) => { return signatures.some( @@ -1168,7 +1215,7 @@ describe('validation logic', () => { testErrorsAndWarnings(`from a_index | eval ${camelCase(field)}Field IS not NULL`, []); } - for (const nesting of [1, 2, 3, 4]) { + for (const nesting of NESTED_DEPTHS) { for (const evenOp of ['-', '+']) { for (const oddOp of ['-', '+']) { // This builds a combination of +/- operators @@ -1198,6 +1245,67 @@ describe('validation logic', () => { ]); } + for (const { name, alias, signatures, ...defRest } of statsAggregationFunctionDefinitions) { + for (const { params, infiniteParams, ...signRest } of signatures) { + const fieldMapping = getFieldMapping(params); + testErrorsAndWarnings( + `from a_index | eval var = ${ + getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMapping, ...signRest }], + }, + { withTypes: false } + )[0].declaration + }`, + [`EVAL does not support function ${name}`] + ); + + testErrorsAndWarnings( + `from a_index | eval var = ${ + getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMapping, ...signRest }], + }, + { withTypes: false } + )[0].declaration + } > 0`, + [`EVAL does not support function ${name}`] + ); + + testErrorsAndWarnings( + `from a_index | eval ${ + getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMapping, ...signRest }], + }, + { withTypes: false } + )[0].declaration + }`, + [`EVAL does not support function ${name}`] + ); + + testErrorsAndWarnings( + `from a_index | eval ${ + getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMapping, ...signRest }], + }, + { withTypes: false } + )[0].declaration + } > 0`, + [`EVAL does not support function ${name}`] + ); + } + } + for (const { name, alias, signatures, ...defRest } of evalFunctionsDefinitions) { for (const { params, infiniteParams, ...signRest } of signatures) { const fieldMapping = getFieldMapping(params); @@ -1629,7 +1737,7 @@ describe('validation logic', () => { 'At least one aggregation function required in [STATS], found [numberField+1]', ]); - for (const nesting of [1, 2, 3, 4]) { + for (const nesting of NESTED_DEPTHS) { const moreBuiltinWrapping = Array(nesting).fill('+1').join(''); testErrorsAndWarnings(`from a_index | stats 5 + avg(numberField) ${moreBuiltinWrapping}`, []); testErrorsAndWarnings(`from a_index | stats 5 ${moreBuiltinWrapping} + avg(numberField)`, []); @@ -1957,6 +2065,11 @@ describe('validation logic', () => { | STATS avg_numberField = AVG(\`numberField * 3.281\`)`, [] ); + + testErrorsAndWarnings( + `FROM index | STATS AVG(numberField) by round(numberField) + 1 | EVAL \`round(numberField) + 1\` / 2`, + [] + ); }); describe('sort', () => { @@ -2132,6 +2245,52 @@ describe('validation logic', () => { ); }); + describe('quoting and escaping expressions', () => { + function getTicks(amount: number) { + return Array(amount).fill('`').join(''); + } + /** + * Given an initial quoted expression, build a new quoted expression + * that appends as many +1 to the previous one based on the nesting level + * i.e. given the expression `round(...) + 1` returns + * ```round(...) + 1`` + 1` (for nesting 1) + * ```````round(...) + 1```` + 1`` + 1` (for nesting 2) + * etc... + * Note how backticks double for each level + wrapping quotes + * The general rule follows an exponential curve given a nesting N: + * (`){ (2^N)-1 } ticks expression (`){ 2^N-1 } +1 (`){ 2^N-2 } +1 ... +1 + * + * Mind that nesting arg here is equivalent to N-1 + */ + function buildNestedExpression(expr: string, nesting: number) { + const openingTicks = getTicks(Math.pow(2, nesting + 1) - 1); + const firstClosingBatch = getTicks(Math.pow(2, nesting)); + const additionalPlusOneswithTicks = Array(nesting) + .fill(' + 1') + .reduce((acc, plusOneAppended, i) => { + // workout how many ticks to add: 2^N-i + const ticks = getTicks(Math.pow(2, nesting - 1 - i)); + return `${acc}${plusOneAppended}${ticks}`; + }, ''); + const ret = `${openingTicks}${expr}${firstClosingBatch}${additionalPlusOneswithTicks}`; + return ret; + } + + for (const nesting of NESTED_DEPTHS) { + // start with a quotable expression + const expr = 'round(numberField) + 1'; + const startingQuery = `from a_index | eval ${expr}`; + // now pipe for each nesting level a new eval command that appends a +1 to the previous quoted expression + const finalQuery = `${startingQuery} | ${Array(nesting) + .fill('') + .map((_, i) => { + return `eval ${buildNestedExpression(expr, i)} + 1`; + }) + .join(' | ')} | keep ${buildNestedExpression(expr, nesting)}`; + testErrorsAndWarnings(finalQuery, []); + } + }); + describe('callbacks', () => { it(`should not fetch source and fields list when a row command is set`, async () => { const callbackMocks = getCallbackMocks(); diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts index 410ce7dba15a1..020fc6ffdd274 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts @@ -227,7 +227,7 @@ function validateFunctionColumnArg( } // do not validate any further for now, only count() accepts wildcard as args... } else { - if (!isEqualType(actualArg, argDef, references, parentCommand)) { + if (!isEqualType(actualArg, argDef, references, parentCommand, nameHit)) { // guaranteed by the check above const columnHit = getColumnHit(nameHit!, references); messages.push( @@ -381,7 +381,9 @@ function validateFunction( parentCommand, parentOption, references, - isNested || !isAssignment(astFunction) + // use the nesting flag for now just for stats + // TODO: revisit this part later on to make it more generic + parentCommand === 'stats' ? isNested || !isAssignment(astFunction) : false ) ); } @@ -867,7 +869,7 @@ export async function validateAst( }); } - const variables = collectVariables(ast, availableFields); + const variables = collectVariables(ast, availableFields, queryString); // notify if the user is rewriting a column as variable with another type messages.push(...validateFieldsShadowing(availableFields, variables)); messages.push(...validateUnsupportedTypeFields(availableFields)); @@ -879,6 +881,7 @@ export async function validateAst( policies: availablePolicies, variables, metadataFields: availableMetadataFields, + query: queryString, }); messages.push(...commandMessages); }