diff --git a/packages/kbn-esql-validation-autocomplete/index.ts b/packages/kbn-esql-validation-autocomplete/index.ts index 0bfc4274fe84d..67fdd72880150 100644 --- a/packages/kbn-esql-validation-autocomplete/index.ts +++ b/packages/kbn-esql-validation-autocomplete/index.ts @@ -62,7 +62,6 @@ export { isLiteralItem, isTimeIntervalItem, isAssignment, - isExpression, isAssignmentComplete, isSingleItem, } from './src/shared/helpers'; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts index c934c4d72e35a..a33235dda581b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts @@ -62,6 +62,36 @@ describe('autocomplete.suggest', () => { }, ].map(attachTriggerCommand) ); + + await assertSuggestions( + 'from a | sort `keywordField`/', + [ + { + filterText: '`keywordField`', + text: '`keywordField`, ', + }, + { + filterText: '`keywordField`', + text: '`keywordField` | ', + }, + { + filterText: '`keywordField`', + text: '`keywordField` ASC', + }, + { + filterText: '`keywordField`', + text: '`keywordField` DESC', + }, + { + filterText: '`keywordField`', + text: '`keywordField` NULLS FIRST', + }, + { + filterText: '`keywordField`', + text: '`keywordField` NULLS LAST', + }, + ].map(attachTriggerCommand) + ); }); it('suggests subsequent column after comma', async () => { const { assertSuggestions } = await setup(); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index a0a4a359c5ff6..8524cd4950955 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -1196,28 +1196,39 @@ describe('autocomplete', () => { ); describe('escaped field names', () => { - // This isn't actually the behavior we want, but this test is here - // to make sure no weird suggestions start cropping up in this case. - testSuggestions(`FROM a | ${commandName} \`foo.bar\`/`, ['foo.bar'], undefined, [ - [{ name: 'foo.bar', type: 'double' }], - ]); - // @todo re-enable these tests when we can use AST to support this case - testSuggestions.skip( + testSuggestions( `FROM a | ${commandName} \`foo.bar\`/`, - ['foo.bar, ', 'foo.bar | '], + ['`foo.bar`, ', '`foo.bar` | '], undefined, - [[{ name: 'foo.bar', type: 'double' }]] + [ + [ + { name: 'foo.bar', type: 'double' }, + { name: 'baz', type: 'date' }, // added so that we get a comma suggestion + ], + ] ); - testSuggestions.skip( - `FROM a | ${commandName} \`foo\`.\`bar\`/`, - ['foo.bar, ', 'foo.bar | '], + testSuggestions( + `FROM a | ${commandName} \`foo\`\`\`\`bar\`\`baz\`/`, + ['`foo````bar``baz`, ', '`foo````bar``baz` | '], undefined, - [[{ name: 'foo.bar', type: 'double' }]] + [ + [ + { name: 'foo``bar`baz', type: 'double' }, + { name: 'baz', type: 'date' }, // added so that we get a comma suggestion + ], + ] ); - testSuggestions.skip(`FROM a | ${commandName} \`any#Char$Field\`/`, [ + testSuggestions(`FROM a | ${commandName} \`any#Char$Field\`/`, [ '`any#Char$Field`, ', '`any#Char$Field` | ', ]); + // @todo enable this test when we can use AST to support this case + testSuggestions.skip( + `FROM a | ${commandName} \`foo\`.\`bar\`/`, + ['`foo`.`bar`, ', '`foo`.`bar` | '], + undefined, + [[{ name: 'foo.bar', type: 'double' }]] + ); }); // Subsequent fields diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 6f9fb66a8c715..5d885379f1a94 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -83,7 +83,7 @@ import { TRIGGER_SUGGESTION_COMMAND, getAddDateHistogramSnippet, } from './factories'; -import { EDITOR_MARKER, SINGLE_BACKTICK, METADATA_FIELDS } from '../shared/constants'; +import { EDITOR_MARKER, METADATA_FIELDS } from '../shared/constants'; import { getAstContext, removeMarkerArgFromArgsList } from '../shared/context'; import { buildQueryUntilPreviousCommand, @@ -1215,11 +1215,7 @@ async function getFieldsOrFunctionsSuggestions( filteredVariablesByType.some((v) => ALPHANUMERIC_REGEXP.test(v)) ) { for (const variable of filteredVariablesByType) { - // remove backticks if present - const sanitizedVariable = variable.startsWith(SINGLE_BACKTICK) - ? variable.slice(1, variable.length - 1) - : variable; - const underscoredName = sanitizedVariable.replace(ALPHANUMERIC_REGEXP, '_'); + const underscoredName = variable.replace(ALPHANUMERIC_REGEXP, '_'); const index = filteredFieldsByType.findIndex( ({ label }) => underscoredName === label || `_${underscoredName}_` === label ); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts index cb08e0f8d4e91..72f5759d0d555 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts @@ -171,7 +171,7 @@ export const buildFieldsDefinitions = (fields: string[]): SuggestionRawDefinitio export const buildVariablesDefinitions = (variables: string[]): SuggestionRawDefinition[] => variables.map((label) => ({ label, - text: label, + text: getSafeInsertText(label), kind: 'Variable', detail: i18n.translate( 'kbn-esql-validation-autocomplete.esql.autocomplete.variableDefinition', diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts index 2b1bd618449c3..9ce286796c971 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts @@ -172,7 +172,7 @@ export interface CommandBaseDefinition { name: string; type: string; optional?: boolean; - innerTypes?: string[]; + innerTypes?: Array; values?: string[]; valueDescriptions?: string[]; constantOnly?: boolean; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts index f97800d6dbc1e..c1942118a41e2 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts @@ -12,7 +12,7 @@ export const EDITOR_MARKER = 'marker_esql_editor'; export const TICKS_REGEX = /^`{1}|`{1}$/g; export const DOUBLE_TICKS_REGEX = /``/g; export const SINGLE_TICK_REGEX = /`/g; -export const SINGLE_BACKTICK = '`'; export const DOUBLE_BACKTICK = '``'; +export const SINGLE_BACKTICK = '`'; export const METADATA_FIELDS = ['_version', '_id', '_index', '_source', '_ignored']; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts b/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts index 6fb9725211478..66f985505a43d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts @@ -83,8 +83,6 @@ export const compareTypesWithLiterals = ( return b === 'timeInterval'; if (b === 'time_literal' || b === 'time_duration' || b === 'date_period') return a === 'timeInterval'; - if (a === 'time_literal') return b === 'time_duration'; - if (b === 'time_literal') return a === 'time_duration'; return false; }; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index fd53d08e67f90..ce9cec58575fc 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -48,7 +48,7 @@ import type { ESQLRealField, ESQLVariable, ReferenceMaps } from '../validation/t import { removeMarkerArgFromArgsList } from './context'; import { isNumericDecimalType } from './esql_types'; import type { ReasonTypes } from './types'; -import { EDITOR_MARKER } from './constants'; +import { DOUBLE_TICKS_REGEX, EDITOR_MARKER, SINGLE_BACKTICK } from './constants'; import type { EditorContext } from '../autocomplete/types'; export function nonNullable(v: T): v is NonNullable { @@ -99,10 +99,6 @@ export function isAssignmentComplete(node: ESQLFunction | undefined) { return Boolean(assignExpression && Array.isArray(assignExpression) && assignExpression.length); } -export function isExpression(arg: ESQLAstItem): arg is ESQLFunction { - return isFunctionItem(arg) && arg.name !== '='; -} - export function isIncompleteItem(arg: ESQLAstItem): boolean { return !arg || (!Array.isArray(arg) && arg.incomplete); } @@ -228,7 +224,7 @@ export function getCommandOption(optionName: CommandOptionsDefinition['name']) { ); } -function compareLiteralType(argType: string, item: ESQLLiteral) { +function doesLiteralMatchParameterType(argType: FunctionParameterType, item: ESQLLiteral) { if (item.literalType === 'null') { return true; } @@ -249,7 +245,7 @@ function compareLiteralType(argType: string, item: ESQLLiteral) { } // date-type parameters accept string literals because of ES auto-casting - return ['string', 'date', 'date', 'date_period'].includes(argType); + return ['string', 'date', 'date_period'].includes(argType); } /** @@ -259,13 +255,7 @@ export function getColumnForASTNode( column: ESQLColumn, { fields, variables }: Pick ): ESQLRealField | ESQLVariable | undefined { - const columnName = getQuotedColumnName(column); - return ( - getColumnByName(columnName, { fields, variables }) || - // It's possible columnName has backticks "`fieldName`" - // so we need to access the original name as well - getColumnByName(column.name, { fields, variables }) - ); + return getColumnByName(column.parts.join('.'), { fields, variables }); } /** @@ -275,6 +265,11 @@ export function getColumnByName( columnName: string, { fields, variables }: Pick ): ESQLRealField | ESQLVariable | undefined { + // TODO this doesn't cover all escaping scenarios... the best thing to do would be + // to use the AST column node parts array, but in some cases the AST node isn't available. + if (columnName.startsWith(SINGLE_BACKTICK) && columnName.endsWith(SINGLE_BACKTICK)) { + columnName = columnName.slice(1, -1).replace(DOUBLE_TICKS_REGEX, SINGLE_BACKTICK); + } return fields.get(columnName) || variables.get(columnName)?.[0]; } @@ -445,7 +440,7 @@ export function checkFunctionArgMatchesDefinition( return true; } if (arg.type === 'literal') { - const matched = compareLiteralType(argType as string, arg); + const matched = doesLiteralMatchParameterType(argType, arg); return matched; } if (arg.type === 'function') { @@ -549,16 +544,6 @@ export function isVariable( return Boolean(column && 'location' in column); } -/** - * This will return the name without any quotes. - * - * E.g. "`bytes`" will become "bytes" - * - * @param column - * @returns - */ -export const getUnquotedColumnName = (column: ESQLColumn) => column.name; - /** * This returns the name with any quotes that were present. * @@ -577,17 +562,16 @@ export function getColumnExists( column: ESQLColumn, { fields, variables }: Pick ) { - const namesToCheck = [getUnquotedColumnName(column), getQuotedColumnName(column)]; - - for (const name of namesToCheck) { - if (fields.has(name) || variables.has(name)) { - return true; - } + const columnName = column.parts.join('.'); + if (fields.has(columnName) || variables.has(columnName)) { + return true; + } - // TODO — I don't see this fuzzy searching in lookupColumn... should it be there? - if (Boolean(fuzzySearch(name, fields.keys()) || fuzzySearch(name, variables.keys()))) { - return true; - } + // TODO — I don't see this fuzzy searching in lookupColumn... should it be there? + if ( + Boolean(fuzzySearch(columnName, fields.keys()) || fuzzySearch(columnName, variables.keys())) + ) { + return true; } return false; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts index 54e6d38736007..29656d2f581ed 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -7,19 +7,13 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import type { ESQLAstItem, ESQLCommand, ESQLCommandOption, ESQLFunction } from '@kbn/esql-ast'; +import type { ESQLAst, ESQLAstItem, ESQLCommand, ESQLFunction } from '@kbn/esql-ast'; +import { Visitor } from '@kbn/esql-ast/src/visitor'; import type { ESQLVariable, ESQLRealField } from '../validation/types'; -import { DOUBLE_BACKTICK, EDITOR_MARKER, SINGLE_BACKTICK } from './constants'; -import { - isColumnItem, - isAssignment, - isExpression, - isOptionItem, - isFunctionItem, - getFunctionDefinition, -} from './helpers'; +import { EDITOR_MARKER } from './constants'; +import { isColumnItem, isFunctionItem, getFunctionDefinition } from './helpers'; -function addToVariableOccurrencies(variables: Map, instance: ESQLVariable) { +function addToVariableOccurrences(variables: Map, instance: ESQLVariable) { if (!variables.has(instance.name)) { variables.set(instance.name, []); } @@ -35,41 +29,73 @@ function addToVariables( ) { if (isColumnItem(oldArg) && isColumnItem(newArg)) { const newVariable: ESQLVariable = { - name: newArg.name, + name: newArg.parts.join('.'), type: 'double' /* fallback to number */, location: newArg.location, }; // Now workout the exact type // it can be a rename of another variable as well - const oldRef = - fields.get(oldArg.name) || variables.get(oldArg.quoted ? oldArg.text : oldArg.name); + const oldRef = fields.get(oldArg.parts.join('.')) || variables.get(oldArg.parts.join('.')); if (oldRef) { - addToVariableOccurrencies(variables, newVariable); + addToVariableOccurrences(variables, newVariable); newVariable.type = Array.isArray(oldRef) ? oldRef[0].type : oldRef.type; } } } -function getAssignRightHandSideType(item: ESQLAstItem, fields: Map) { - if (Array.isArray(item)) { - const firstArg = item[0]; - if (Array.isArray(firstArg) || !firstArg) { - return; +/** + * Determines the type of the expression + * + * TODO - this function needs a lot of work. For example, it needs to find the best-matching function signature + * which it isn't currently doing. See https://github.com/elastic/kibana/issues/195682 + */ +function getExpressionType( + root: ESQLAstItem, + fields: Map, + variables: Map +): string { + const fallback = 'double'; + + if (Array.isArray(root) || !root) { + return fallback; + } + if (root.type === 'literal') { + return root.literalType; + } + if (root.type === 'inlineCast') { + if (root.castType === 'int') { + return 'integer'; } - if (firstArg.type === 'literal') { - return firstArg.literalType; + if (root.castType === 'bool') { + return 'boolean'; } - if (isColumnItem(firstArg)) { - const field = fields.get(firstArg.name); - if (field) { - return field.type; - } + return root.castType; + } + if (isColumnItem(root)) { + const field = fields.get(root.parts.join('.')); + if (field) { + return field.type; } - if (isFunctionItem(firstArg)) { - const fnDefinition = getFunctionDefinition(firstArg.name); - return fnDefinition?.signatures[0].returnType; + const variable = variables.get(root.parts.join('.')); + if (variable) { + return variable[0].type; } - return firstArg.type; + } + if (isFunctionItem(root)) { + const fnDefinition = getFunctionDefinition(root.name); + return fnDefinition?.signatures[0].returnType ?? fallback; + } + return fallback; +} + +function getAssignRightHandSideType( + item: ESQLAstItem, + fields: Map, + variables: Map +) { + if (Array.isArray(item)) { + const firstArg = item[0]; + return getExpressionType(firstArg, fields, variables); } } @@ -90,25 +116,20 @@ 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 as string) || 'double' /* fallback to number */, + const rightHandSideArgType = getAssignRightHandSideType( + assignOperation.args[1], + fields, + variables + ); + addToVariableOccurrences(variables, { + name: assignOperation.args[0].parts.join('.'), + type: rightHandSideArgType as string /* fallback to number */, location: assignOperation.args[0].location, }); } @@ -120,79 +141,71 @@ function addVariableFromExpression( 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 expressionText = queryString.substring( + expressionOperation.location.min, + expressionOperation.location.max + 1 ); - const expressionType = 'double'; - addToVariableOccurrencies(variables, { - name: forwardThinkingVariableName, + const expressionType = 'double'; // TODO - use getExpressionType once it actually works + addToVariableOccurrences(variables, { + name: expressionText, type: expressionType, location: expressionOperation.location, }); } } -export const collectVariablesFromList = ( - list: ESQLAstItem[], - fields: Map, - queryString: string, - variables: Map -) => { - for (const arg of list) { - if (isAssignment(arg)) { - addVariableFromAssignment(arg, variables, fields); - } else if (isExpression(arg)) { - addVariableFromExpression(arg, queryString, variables); - } - } -}; - export function collectVariables( - commands: ESQLCommand[], + ast: ESQLAst, fields: Map, queryString: string ): Map { const variables = new Map(); - for (const command of commands) { - if (['row', 'eval', 'stats', 'inlinestats', 'metrics'].includes(command.name)) { - collectVariablesFromList(command.args, fields, queryString, variables); - if (command.name === 'stats' || command.name === 'inlinestats') { - const commandOptionsWithAssignment = command.args.filter( - (arg) => isOptionItem(arg) && arg.name === 'by' - ) as ESQLCommandOption[]; - for (const commandOption of commandOptionsWithAssignment) { - collectVariablesFromList(commandOption.args, fields, queryString, variables); - } + + const visitor = new Visitor() + .on('visitLiteralExpression', (ctx) => { + // TODO - add these as variables + }) + .on('visitExpression', (_ctx) => {}) // required for the types :shrug: + .on('visitRenameExpression', (ctx) => { + const [oldArg, newArg] = ctx.node.args; + addToVariables(oldArg, newArg, fields, variables); + }) + .on('visitFunctionCallExpression', (ctx) => { + if (ctx.node.name === '=') { + addVariableFromAssignment(ctx.node, variables, fields); + } else { + addVariableFromExpression(ctx.node, queryString, variables); } - } - if (command.name === 'enrich') { - const commandOptionsWithAssignment = command.args.filter( - (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) { + }) + .on('visitCommandOption', (ctx) => { + if (ctx.node.name === 'by') { + return [...ctx.visitArguments()]; + } else if (ctx.node.name === 'with') { + for (const assignFn of ctx.node.args) { if (isFunctionItem(assignFn)) { const [newArg, oldArg] = assignFn?.args || []; + // TODO why is oldArg an array? if (Array.isArray(oldArg)) { addToVariables(oldArg[0], newArg, fields, variables); } } } } - } - if (command.name === 'rename') { - const commandOptionsWithAssignment = command.args.filter( - (arg) => isOptionItem(arg) && arg.name === 'as' - ) as ESQLCommandOption[]; - for (const commandOption of commandOptionsWithAssignment) { - const [oldArg, newArg] = commandOption.args; - addToVariables(oldArg, newArg, fields, variables); + }) + .on('visitCommand', (ctx) => { + const ret = []; + if (['row', 'eval', 'stats', 'inlinestats', 'metrics', 'rename'].includes(ctx.node.name)) { + ret.push(...ctx.visitArgs()); } - } - } + if (['stats', 'inlinestats', 'enrich'].includes(ctx.node.name)) { + // BY and WITH can contain variables + ret.push(...ctx.visitOptions()); + } + return ret; + }) + .on('visitQuery', (ctx) => [...ctx.visitCommands()]); + + visitor.visitQuery(ast); + return variables; } diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/fields_and_variables.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/fields_and_variables.test.ts new file mode 100644 index 0000000000000..e0004d603d9c2 --- /dev/null +++ b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/fields_and_variables.test.ts @@ -0,0 +1,222 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { FunctionParameterType } from '../../definitions/types'; +import { setTestFunctions } from '../../shared/test_functions'; +import { setup } from './helpers'; + +describe('field and variable escaping', () => { + it('recognizes escaped fields', async () => { + const { expectErrors } = await setup(); + // command level + await expectErrors( + 'FROM index | KEEP `kubernetes`.`something`.`something` | EVAL `kubernetes.something.something` + 12', + [] + ); + // function argument + await expectErrors('FROM index | EVAL ABS(`kubernetes`.`something`.`something`)', []); + }); + + it('recognizes field names with spaces and comments', async () => { + const { expectErrors } = await setup(); + // command level + await expectErrors('FROM index | KEEP kubernetes . something . /* gotcha! */ something', []); + // function argument + await expectErrors( + 'FROM index | EVAL ABS(kubernetes . something . /* gotcha! */ something)', + [] + ); + }); + + it('recognizes escaped variables', async () => { + const { expectErrors } = await setup(); + // command level + await expectErrors('ROW `var$iable` = 1 | EVAL `var$iable`', []); + + // command level, different escaping in declaration + await expectErrors( + 'ROW variable.`wi#th`.separator = "lolz" | EVAL `variable`.`wi#th`.`separator`', + [] + ); + + // function arguments + await expectErrors( + 'ROW `var$iable` = 1, variable.`wi#th`.separator = "lolz" | EVAL ABS(`var$iable`), TRIM(variable.`wi#th`.`separator`)', + [] + ); + + // expression variable + await expectErrors('FROM index | EVAL doubleField + 20 | EVAL `doubleField + 20`', []); + await expectErrors('ROW 21 + 20 | STATS AVG(`21 + 20`)', []); + }); + + it('recognizes variables with spaces and comments', async () => { + const { expectErrors } = await setup(); + // command level + await expectErrors( + 'ROW variable.`wi#th`.separator = "lolz" | RENAME variable . /* lolz */ `wi#th` . separator AS foo', + [] + ); + // function argument + await expectErrors( + 'ROW variable.`wi#th`.separator = "lolz" | EVAL TRIM(variable . /* lolz */ `wi#th` . separator)', + [] + ); + }); + + describe('as part of various commands', () => { + const cases = [ + { name: 'ROW', command: 'ROW `var$iable` = 1, variable.`wi#th`.separator = "lolz"' }, + { + name: 'DISSECT', + command: 'ROW `funky`.`stri#$ng` = "lolz" | DISSECT `funky`.`stri#$ng` "%{WORD:firstWord}"', + }, + { name: 'DROP', command: 'FROM index | DROP kubernetes.`something`.`something`' }, + { + name: 'ENRICH', + command: + 'FROM index | ENRICH policy WITH `new`.name1 = `otherField`, `new.name2` = `yetAnotherField`', + }, + { name: 'EVAL', command: 'FROM index | EVAL kubernetes.`something`.`something` + 12' }, + { + name: 'GROK', + command: 'ROW `funky`.`stri#$ng` = "lolz" | GROK `funky`.`stri#$ng` "%{WORD:firstWord}"', + }, + { name: 'KEEP', command: 'FROM index | KEEP kubernetes.`something`.`something`' }, + { + name: 'RENAME', + command: 'FROM index | RENAME kubernetes.`something`.`something` as foobar', + }, + { name: 'SORT', command: 'FROM index | SORT kubernetes.`something`.`something` DESC' }, + { + name: 'STATS ... BY', + command: + 'FROM index | STATS AVG(kubernetes.`something`.`something`) BY `kubernetes`.`something`.`something`', + }, + { name: 'WHERE', command: 'FROM index | WHERE kubernetes.`something`.`something` == 12' }, + ]; + + it.each(cases)('$name accepts escaped fields', async ({ command }) => { + const { expectErrors } = await setup(); + await expectErrors(command, []); + }); + }); +}); + +describe('variable support', () => { + describe('variable data type detection', () => { + // most of these tests are aspirational (and skipped) because we don't have + // a good way to compute the type of an expression yet. + beforeAll(() => { + setTestFunctions([ + // this test function is just used to test the type of the variable + { + type: 'eval', + description: 'Test function', + supportedCommands: ['eval'], + name: 'test', + signatures: [ + { params: [{ name: 'arg', type: 'cartesian_point' }], returnType: 'cartesian_point' }, + ], + }, + // this test function is used to check that the correct return type is used + // when determining variable types + { + type: 'eval', + description: 'Test function', + supportedCommands: ['eval'], + name: 'return_value', + signatures: [ + { params: [{ name: 'arg', type: 'text' }], returnType: 'text' }, + { params: [{ name: 'arg', type: 'double' }], returnType: 'double' }, + { + params: [ + { name: 'arg', type: 'double' }, + { name: 'arg', type: 'text' }, + ], + returnType: 'long', + }, + ], + }, + ]); + }); + + afterAll(() => { + setTestFunctions([]); + }); + + const expectType = (type: FunctionParameterType) => + `Argument of [test] must be [cartesian_point], found value [var] type [${type}]`; + + // @todo unskip after https://github.com/elastic/kibana/issues/195682 + test.skip('literals', async () => { + const { expectErrors } = await setup(); + // literal assignment + await expectErrors('FROM index | EVAL var = 1, TEST(var)', [expectType('integer')]); + // literal expression + await expectErrors('FROM index | EVAL 1, TEST(`1`)', [expectType('integer')]); + }); + + test('fields', async () => { + const { expectErrors } = await setup(); + // field assignment + await expectErrors('FROM index | EVAL var = textField, TEST(var)', [expectType('text')]); + }); + + // @todo unskip after https://github.com/elastic/kibana/issues/195682 + test.skip('variables', async () => { + const { expectErrors } = await setup(); + await expectErrors('FROM index | EVAL var = textField, var2 = var, TEST(var2)', [ + `Argument of [test] must be [cartesian_point], found value [var2] type [text]`, + ]); + }); + + // @todo unskip after https://github.com/elastic/kibana/issues/195682 + test.skip('inline casting', async () => { + const { expectErrors } = await setup(); + // inline cast assignment + await expectErrors('FROM index | EVAL var = doubleField::long, TEST(var)', [ + expectType('long'), + ]); + // inline cast expression + await expectErrors('FROM index | EVAL doubleField::long, TEST(`doubleField::long`)', [ + expectType('long'), + ]); + }); + + // @todo unskip after https://github.com/elastic/kibana/issues/195682 + test.skip('function results', async () => { + const { expectErrors } = await setup(); + // function assignment + await expectErrors('FROM index | EVAL var = RETURN_VALUE(doubleField), TEST(var)', [ + expectType('double'), + ]); + await expectErrors('FROM index | EVAL var = RETURN_VALUE(textField), TEST(var)', [ + expectType('text'), + ]); + await expectErrors( + 'FROM index | EVAL var = RETURN_VALUE(doubleField, textField), TEST(var)', + [expectType('long')] + ); + // function expression + await expectErrors( + 'FROM index | EVAL RETURN_VALUE(doubleField), TEST(`RETURN_VALUE(doubleField)`)', + [expectType('double')] + ); + await expectErrors( + 'FROM index | EVAL RETURN_VALUE(textField), TEST(`RETURN_VALUE(textField)`)', + [expectType('text')] + ); + await expectErrors( + 'FROM index | EVAL RETURN_VALUE(doubleField, textField), TEST(`RETURN_VALUE(doubleField, textField)`)', + [expectType('long')] + ); + }); + }); +}); diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index fdfe03852fc48..23508eeedd234 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -76,7 +76,7 @@ import { import { collapseWrongArgumentTypeMessages, getMaxMinNumberOfParams } from './helpers'; import { getParamAtPosition } from '../autocomplete/helper'; import { METADATA_FIELDS } from '../shared/constants'; -import { isStringType } from '../shared/esql_types'; +import { compareTypesWithLiterals } from '../shared/esql_types'; function validateFunctionLiteralArg( astFunction: ESQLFunction, @@ -870,10 +870,12 @@ function validateColumnForCommand( const columnRef = getColumnForASTNode(column, references)!; if (columnParamsWithInnerTypes.length) { - const hasSomeWrongInnerTypes = columnParamsWithInnerTypes.every(({ innerTypes }) => { - if (innerTypes?.includes('string') && isStringType(columnRef.type)) return false; - return innerTypes && !innerTypes.includes('any') && !innerTypes.includes(columnRef.type); - }); + const hasSomeWrongInnerTypes = columnParamsWithInnerTypes.every( + ({ innerTypes }) => + innerTypes && + !innerTypes.includes('any') && + !innerTypes.some((type) => compareTypesWithLiterals(type, columnRef.type)) + ); if (hasSomeWrongInnerTypes) { const supportedTypes: string[] = columnParamsWithInnerTypes .map(({ innerTypes }) => innerTypes)