From ac3436461829698a2b5b87f496242197086320c6 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 4 Oct 2024 16:28:41 -0600 Subject: [PATCH 01/17] storing all variables and field names in unescaped form --- .../src/shared/constants.ts | 2 - .../src/shared/helpers.ts | 37 ++----- .../src/shared/variables.ts | 33 ++---- .../field_and_variable_escaping.test.ts | 100 ++++++++++++++++++ 4 files changed, 121 insertions(+), 51 deletions(-) create mode 100644 packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts index f97800d6dbc1e..dc0676eaae3ce 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts @@ -12,7 +12,5 @@ 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 METADATA_FIELDS = ['_version', '_id', '_index', '_source', '_ignored']; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index d58101e9ff8eb..ce47e6cfc6b6e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -259,13 +259,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 }); } /** @@ -552,16 +546,6 @@ export function hasCCSSource(name: string) { return name.includes(':'); } -/** - * 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. * @@ -580,17 +564,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..ba5e1c6b545f4 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -9,7 +9,7 @@ import type { ESQLAstItem, ESQLCommand, ESQLCommandOption, ESQLFunction } from '@kbn/esql-ast'; import type { ESQLVariable, ESQLRealField } from '../validation/types'; -import { DOUBLE_BACKTICK, EDITOR_MARKER, SINGLE_BACKTICK } from './constants'; +import { EDITOR_MARKER } from './constants'; import { isColumnItem, isAssignment, @@ -19,7 +19,7 @@ import { 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,7 +35,7 @@ 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, }; @@ -44,7 +44,7 @@ function addToVariables( const oldRef = fields.get(oldArg.name) || variables.get(oldArg.quoted ? oldArg.text : oldArg.name); if (oldRef) { - addToVariableOccurrencies(variables, newVariable); + addToVariableOccurrences(variables, newVariable); newVariable.type = Array.isArray(oldRef) ? oldRef[0].type : oldRef.type; } } @@ -90,15 +90,6 @@ 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, @@ -106,8 +97,8 @@ function addVariableFromAssignment( ) { if (isColumnItem(assignOperation.args[0])) { const rightHandSideArgType = getAssignRightHandSideType(assignOperation.args[1], fields); - addToVariableOccurrencies(variables, { - name: assignOperation.args[0].name, + addToVariableOccurrences(variables, { + name: assignOperation.args[0].parts.join('.'), type: (rightHandSideArgType as string) || 'double' /* fallback to number */, location: assignOperation.args[0].location, }); @@ -120,15 +111,13 @@ 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, + addToVariableOccurrences(variables, { + name: expressionText, type: expressionType, location: expressionOperation.location, }); diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts new file mode 100644 index 0000000000000..0244fe6b0a147 --- /dev/null +++ b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts @@ -0,0 +1,100 @@ +/* + * 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 { 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`)', + [] + ); + }); + + 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: 'ENRICH' }, + { 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, []); + }); + }); +}); From bb89a6cad4371cb81a29e05928c4871c91d97799 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 11:37:41 -0600 Subject: [PATCH 02/17] add expression test cases --- .../validation/__tests__/field_and_variable_escaping.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts index 0244fe6b0a147..15fd76a9aeabb 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts @@ -48,6 +48,10 @@ describe('field and variable escaping', () => { '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 () => { From 9a3456fc20bb57b1e88bd030441db98d900f672e Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 14:03:53 -0600 Subject: [PATCH 03/17] add tests for variable type checking --- .../field_and_variable_escaping.test.ts | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts index 15fd76a9aeabb..4953afb56af81 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts @@ -7,6 +7,8 @@ * 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', () => { @@ -102,3 +104,102 @@ describe('field and variable escaping', () => { }); }); }); + +describe('variable support', () => { + describe('variable data type detection', () => { + 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}]`; + + test('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')]); + }); + + test('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'), + ]); + }); + + test('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')] + ); + }); + }); +}); From 783a1853ac805d4e3c8177315487f5c86f065067 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 14:04:22 -0600 Subject: [PATCH 04/17] rename file --- ...and_variable_escaping.test.ts => fields_and_variables.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/kbn-esql-validation-autocomplete/src/validation/__tests__/{field_and_variable_escaping.test.ts => fields_and_variables.test.ts} (100%) diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/fields_and_variables.test.ts similarity index 100% rename from packages/kbn-esql-validation-autocomplete/src/validation/__tests__/field_and_variable_escaping.test.ts rename to packages/kbn-esql-validation-autocomplete/src/validation/__tests__/fields_and_variables.test.ts From 8ad99593dfe64289041dff18a000abaa8624dd8a Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 14:38:30 -0600 Subject: [PATCH 05/17] fix rename and enrich variable escaping logic --- .../src/shared/variables.ts | 3 +-- .../src/validation/__tests__/fields_and_variables.test.ts | 6 +++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts index ba5e1c6b545f4..53b16620b5256 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -41,8 +41,7 @@ function addToVariables( }; // 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) { addToVariableOccurrences(variables, newVariable); newVariable.type = Array.isArray(oldRef) ? oldRef[0].type : oldRef.type; 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 index 4953afb56af81..7a07f807e3c6b 100644 --- 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 @@ -78,7 +78,11 @@ describe('field and variable escaping', () => { command: 'ROW `funky`.`stri#$ng` = "lolz" | DISSECT `funky`.`stri#$ng` "%{WORD:firstWord}"', }, { name: 'DROP', command: 'FROM index | DROP kubernetes.`something`.`something`' }, - // { name: 'ENRICH', command: 'ENRICH' }, + { + 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', From 8a28383c5bdb4685cce0c237dd334ced4dd1f550 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 14:51:35 -0600 Subject: [PATCH 06/17] fix grok and dissect --- .../src/definitions/types.ts | 2 +- .../src/validation/validation.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts index a6e297771cebe..36c970585703e 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/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index 428a2d1fcd4f5..90e1a203589f1 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -77,7 +77,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, @@ -876,10 +876,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) From 89534e3fdf4c5b5fc52f95a185fe40dcc4caa117 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 17:12:57 -0600 Subject: [PATCH 07/17] refactor variable traversal --- .../src/shared/helpers.ts | 4 - .../src/shared/variables.ts | 92 ++++++++----------- .../__tests__/fields_and_variables.test.ts | 4 + .../esql_validation_meta_tests.json | 32 +++++-- 4 files changed, 67 insertions(+), 65 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index cc907b37ba907..9c0781e12c0b8 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -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); } diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts index 53b16620b5256..7159fe2c7132d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -7,17 +7,11 @@ * 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 { EDITOR_MARKER } from './constants'; -import { - isColumnItem, - isAssignment, - isExpression, - isOptionItem, - isFunctionItem, - getFunctionDefinition, -} from './helpers'; +import { isColumnItem, isFunctionItem, getFunctionDefinition } from './helpers'; function addToVariableOccurrences(variables: Map, instance: ESQLVariable) { if (!variables.has(instance.name)) { @@ -123,46 +117,34 @@ function addVariableFromExpression( } } -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('visitExpression', (_ctx) => {}) // required for the types :shrug: + .on('visitRenameExpression', (ctx) => { + const [oldArg, newArg] = ctx.node.args; + addToVariables(oldArg, newArg, fields, variables); + }) + .on('visitRenameCommand', (ctx) => { + return [...ctx.visitArguments()]; + }) + .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 || []; if (Array.isArray(oldArg)) { @@ -171,16 +153,20 @@ export function collectVariables( } } } - } - 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'].includes(ctx.node.name)) { + ret.push(...ctx.visitArgs()); } - } - } + if (['stats', 'inlinestats', 'enrich'].includes(ctx.node.name)) { + 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 index 7a07f807e3c6b..9182910f4d37b 100644 --- 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 @@ -63,6 +63,10 @@ describe('field and variable escaping', () => { 'ROW variable.`wi#th`.separator = "lolz" | RENAME variable . /* lolz */ `wi#th` . separator AS foo', [] ); + // await expectErrors( + // 'FROM index | ENRICH policy WITH `new`.name1 = `otherField`, `new.name2` = `yetAnotherField`', + // [] + // ); // function argument await expectErrors( 'ROW variable.`wi#th`.separator = "lolz" | EVAL TRIM(variable . /* lolz */ `wi#th` . separator)', diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json index 1eb861168a13a..1327ed4af98bd 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json +++ b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json @@ -9836,22 +9836,30 @@ }, { "query": "fRoM remote-*:indexes*", - "error": [], + "error": [ + "Unknown index [remote-*:indexes*]" + ], "warning": [] }, { "query": "fRoM remote-*:indexes", - "error": [], + "error": [ + "Unknown index [remote-*:indexes]" + ], "warning": [] }, { "query": "fRoM remote-ccs:indexes", - "error": [], + "error": [ + "Unknown index [remote-ccs:indexes]" + ], "warning": [] }, { "query": "fRoM a_index, remote-ccs:indexes", - "error": [], + "error": [ + "Unknown index [remote-ccs:indexes]" + ], "warning": [] }, { @@ -9988,14 +9996,18 @@ }, { "query": "from remote-ccs:indexes [METADATA _id]", - "error": [], + "error": [ + "Unknown index [remote-ccs:indexes]" + ], "warning": [ "Square brackets '[]' need to be removed from FROM METADATA declaration" ] }, { "query": "from *:indexes [METADATA _id]", - "error": [], + "error": [ + "Unknown index [*:indexes]" + ], "warning": [ "Square brackets '[]' need to be removed from FROM METADATA declaration" ] @@ -10040,12 +10052,16 @@ }, { "query": "from remote-ccs:indexes METADATA _id", - "error": [], + "error": [ + "Unknown index [remote-ccs:indexes]" + ], "warning": [] }, { "query": "from *:indexes METADATA _id", - "error": [], + "error": [ + "Unknown index [*:indexes]" + ], "warning": [] }, { From a5138cd52fbf05efa3e5bcba13b674334efa5aca Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 17:17:06 -0600 Subject: [PATCH 08/17] a bit more simplification --- .../src/shared/variables.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts index 7159fe2c7132d..5995fe94bdf79 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -130,9 +130,6 @@ export function collectVariables( const [oldArg, newArg] = ctx.node.args; addToVariables(oldArg, newArg, fields, variables); }) - .on('visitRenameCommand', (ctx) => { - return [...ctx.visitArguments()]; - }) .on('visitFunctionCallExpression', (ctx) => { if (ctx.node.name === '=') { addVariableFromAssignment(ctx.node, variables, fields); @@ -147,6 +144,7 @@ export function collectVariables( 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); } @@ -156,10 +154,11 @@ export function collectVariables( }) .on('visitCommand', (ctx) => { const ret = []; - if (['row', 'eval', 'stats', 'inlinestats', 'metrics'].includes(ctx.node.name)) { + 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; From 326075d686b53b7ec9683e6fd170186ce42b11c9 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 7 Oct 2024 17:59:23 -0600 Subject: [PATCH 09/17] clarify some parameter checking --- .../src/shared/esql_types.ts | 2 -- .../kbn-esql-validation-autocomplete/src/shared/helpers.ts | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) 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 9c0781e12c0b8..a076a39963948 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -224,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; } @@ -245,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); } /** @@ -435,7 +435,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') { From bca3b868a56334e2b9c7844802f84fc77ad502bb Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 8 Oct 2024 13:40:53 -0600 Subject: [PATCH 10/17] getExpressionType is its own function --- .../src/shared/variables.ts | 80 ++++++++++++++----- .../__tests__/fields_and_variables.test.ts | 19 +++-- 2 files changed, 73 insertions(+), 26 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts index 5995fe94bdf79..40bf6b5d9fb82 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -43,26 +43,61 @@ function addToVariables( } } -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. + * @param root + * @returns + */ +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); } } @@ -89,10 +124,14 @@ function addVariableFromAssignment( fields: Map ) { if (isColumnItem(assignOperation.args[0])) { - const rightHandSideArgType = getAssignRightHandSideType(assignOperation.args[1], fields); + const rightHandSideArgType = getAssignRightHandSideType( + assignOperation.args[1], + fields, + variables + ); addToVariableOccurrences(variables, { name: assignOperation.args[0].parts.join('.'), - type: (rightHandSideArgType as string) || 'double' /* fallback to number */, + type: rightHandSideArgType as string /* fallback to number */, location: assignOperation.args[0].location, }); } @@ -108,7 +147,7 @@ function addVariableFromExpression( expressionOperation.location.min, expressionOperation.location.max + 1 ); - const expressionType = 'double'; + const expressionType = 'double'; // TODO - use getExpressionType once it actually works addToVariableOccurrences(variables, { name: expressionText, type: expressionType, @@ -125,6 +164,9 @@ export function collectVariables( const variables = new Map(); 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; 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 index 9182910f4d37b..fa5777d77b184 100644 --- 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 @@ -63,10 +63,6 @@ describe('field and variable escaping', () => { 'ROW variable.`wi#th`.separator = "lolz" | RENAME variable . /* lolz */ `wi#th` . separator AS foo', [] ); - // await expectErrors( - // 'FROM index | ENRICH policy WITH `new`.name1 = `otherField`, `new.name2` = `yetAnotherField`', - // [] - // ); // function argument await expectErrors( 'ROW variable.`wi#th`.separator = "lolz" | EVAL TRIM(variable . /* lolz */ `wi#th` . separator)', @@ -115,6 +111,8 @@ describe('field and variable escaping', () => { 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 @@ -156,7 +154,7 @@ describe('variable support', () => { const expectType = (type: FunctionParameterType) => `Argument of [test] must be [cartesian_point], found value [var] type [${type}]`; - test('literals', async () => { + test.skip('literals', async () => { const { expectErrors } = await setup(); // literal assignment await expectErrors('FROM index | EVAL var = 1, TEST(var)', [expectType('integer')]); @@ -170,7 +168,14 @@ describe('variable support', () => { await expectErrors('FROM index | EVAL var = textField, TEST(var)', [expectType('text')]); }); - test('inline casting', async () => { + 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]`, + ]); + }); + + test.skip('inline casting', async () => { const { expectErrors } = await setup(); // inline cast assignment await expectErrors('FROM index | EVAL var = doubleField::long, TEST(var)', [ @@ -182,7 +187,7 @@ describe('variable support', () => { ]); }); - test('function results', async () => { + test.skip('function results', async () => { const { expectErrors } = await setup(); // function assignment await expectErrors('FROM index | EVAL var = RETURN_VALUE(doubleField), TEST(var)', [ From 625b76542ac56b873554a29e35bfad37c1d76ae8 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 9 Oct 2024 08:37:04 -0600 Subject: [PATCH 11/17] Fix escaping of variable names --- .../src/autocomplete/autocomplete.ts | 8 ++------ .../src/autocomplete/factories.ts | 2 +- .../src/shared/constants.ts | 1 + 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 41060675a73a8..f13131f42a0b1 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, @@ -1176,11 +1176,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/shared/constants.ts b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts index dc0676eaae3ce..69955f28330ab 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts @@ -12,5 +12,6 @@ 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 DOUBLE_BACKTICK = '``'; export const METADATA_FIELDS = ['_version', '_id', '_index', '_source', '_ignored']; From a6f68064a2bda558fe50ef57cb3ec60a15d78699 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 9 Oct 2024 09:56:40 -0600 Subject: [PATCH 12/17] partially support escaped fields in autocomplete --- .../autocomplete.command.sort.test.ts | 30 +++++++++++++++++++ .../src/autocomplete/autocomplete.test.ts | 9 ++---- .../src/shared/constants.ts | 1 + .../src/shared/helpers.ts | 7 ++++- 4 files changed, 39 insertions(+), 8 deletions(-) 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 095cb2ffc9d14..3836103c04820 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -1137,22 +1137,17 @@ 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 | KEEP `foo.bar`/', ['foo.bar'], undefined, [ + testSuggestions('FROM a | KEEP `foo.bar`/', ['`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('FROM a | KEEP `foo.bar`/', ['foo.bar, ', 'foo.bar | '], undefined, [ - [{ name: 'foo.bar', type: 'double' }], - ]); testSuggestions.skip( 'FROM a | KEEP `foo`.`bar`/', ['foo.bar, ', 'foo.bar | '], undefined, [[{ name: 'foo.bar', type: 'double' }]] ); - testSuggestions.skip('FROM a | KEEP `any#Char$Field`/', [ + testSuggestions('FROM a | KEEP `any#Char$Field`/', [ '`any#Char$Field`, ', '`any#Char$Field` | ', ]); diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts index 69955f28330ab..c1942118a41e2 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/constants.ts @@ -13,5 +13,6 @@ export const TICKS_REGEX = /^`{1}|`{1}$/g; export const DOUBLE_TICKS_REGEX = /``/g; export const SINGLE_TICK_REGEX = /`/g; 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/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index a076a39963948..115a1b07a6f72 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 { EDITOR_MARKER, SINGLE_BACKTICK } from './constants'; import type { EditorContext } from '../autocomplete/types'; export function nonNullable(v: T): v is NonNullable { @@ -265,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); + } return fields.get(columnName) || variables.get(columnName)?.[0]; } From 9ce6e12e4fbfd0e10e9122e8043ef773a6cdf18b Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 9 Oct 2024 10:39:06 -0600 Subject: [PATCH 13/17] fix problem export --- packages/kbn-esql-validation-autocomplete/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kbn-esql-validation-autocomplete/index.ts b/packages/kbn-esql-validation-autocomplete/index.ts index bd41d6ec43a5a..130aff676155d 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'; From 648dee5eb626bcfaeb51fcd8fadb2f12e2bcc50e Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 9 Oct 2024 15:29:23 -0600 Subject: [PATCH 14/17] restore old meta tests --- .../esql_validation_meta_tests.json | 32 +++++-------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json index 1327ed4af98bd..1eb861168a13a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json +++ b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json @@ -9836,30 +9836,22 @@ }, { "query": "fRoM remote-*:indexes*", - "error": [ - "Unknown index [remote-*:indexes*]" - ], + "error": [], "warning": [] }, { "query": "fRoM remote-*:indexes", - "error": [ - "Unknown index [remote-*:indexes]" - ], + "error": [], "warning": [] }, { "query": "fRoM remote-ccs:indexes", - "error": [ - "Unknown index [remote-ccs:indexes]" - ], + "error": [], "warning": [] }, { "query": "fRoM a_index, remote-ccs:indexes", - "error": [ - "Unknown index [remote-ccs:indexes]" - ], + "error": [], "warning": [] }, { @@ -9996,18 +9988,14 @@ }, { "query": "from remote-ccs:indexes [METADATA _id]", - "error": [ - "Unknown index [remote-ccs:indexes]" - ], + "error": [], "warning": [ "Square brackets '[]' need to be removed from FROM METADATA declaration" ] }, { "query": "from *:indexes [METADATA _id]", - "error": [ - "Unknown index [*:indexes]" - ], + "error": [], "warning": [ "Square brackets '[]' need to be removed from FROM METADATA declaration" ] @@ -10052,16 +10040,12 @@ }, { "query": "from remote-ccs:indexes METADATA _id", - "error": [ - "Unknown index [remote-ccs:indexes]" - ], + "error": [], "warning": [] }, { "query": "from *:indexes METADATA _id", - "error": [ - "Unknown index [*:indexes]" - ], + "error": [], "warning": [] }, { From c90680729b95f5cafa23ddfdf0cbfee1044aa1c7 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 9 Oct 2024 15:52:10 -0600 Subject: [PATCH 15/17] add some more todos --- .../src/validation/__tests__/fields_and_variables.test.ts | 4 ++++ 1 file changed, 4 insertions(+) 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 index fa5777d77b184..e0004d603d9c2 100644 --- 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 @@ -154,6 +154,7 @@ describe('variable support', () => { 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 @@ -168,6 +169,7 @@ describe('variable support', () => { 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)', [ @@ -175,6 +177,7 @@ describe('variable support', () => { ]); }); + // @todo unskip after https://github.com/elastic/kibana/issues/195682 test.skip('inline casting', async () => { const { expectErrors } = await setup(); // inline cast assignment @@ -187,6 +190,7 @@ describe('variable support', () => { ]); }); + // @todo unskip after https://github.com/elastic/kibana/issues/195682 test.skip('function results', async () => { const { expectErrors } = await setup(); // function assignment From b2914044702bd7e1c48560bcca32accd16cd94cc Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 10 Oct 2024 11:21:48 -0600 Subject: [PATCH 16/17] support escaped backticks in autocomplete field lists --- .../src/autocomplete/autocomplete.test.ts | 11 +++++++++++ .../src/shared/helpers.ts | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) 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 a68fa9900445e..8524cd4950955 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -1207,6 +1207,17 @@ describe('autocomplete', () => { ], ] ); + testSuggestions( + `FROM a | ${commandName} \`foo\`\`\`\`bar\`\`baz\`/`, + ['`foo````bar``baz`, ', '`foo````bar``baz` | '], + undefined, + [ + [ + { name: 'foo``bar`baz', type: 'double' }, + { name: 'baz', type: 'date' }, // added so that we get a comma suggestion + ], + ] + ); testSuggestions(`FROM a | ${commandName} \`any#Char$Field\`/`, [ '`any#Char$Field`, ', '`any#Char$Field` | ', diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index 115a1b07a6f72..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, SINGLE_BACKTICK } 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 { @@ -268,7 +268,7 @@ export function getColumnByName( // 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); + columnName = columnName.slice(1, -1).replace(DOUBLE_TICKS_REGEX, SINGLE_BACKTICK); } return fields.get(columnName) || variables.get(columnName)?.[0]; } From 9a524970d7bfe36274d9ecf65430fe5001a513f1 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 10 Oct 2024 11:22:42 -0600 Subject: [PATCH 17/17] fix some docs --- .../kbn-esql-validation-autocomplete/src/shared/variables.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts index 40bf6b5d9fb82..29656d2f581ed 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -47,9 +47,7 @@ function addToVariables( * 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. - * @param root - * @returns + * which it isn't currently doing. See https://github.com/elastic/kibana/issues/195682 */ function getExpressionType( root: ESQLAstItem,