From b0af8afea6f1251ea96bd623863f8343cab145d5 Mon Sep 17 00:00:00 2001 From: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com> Date: Thu, 7 Nov 2024 14:23:42 +0100 Subject: [PATCH] [ES|QL] Support params in column nodes and function names (#198957) ## Summary Closes https://github.com/elastic/kibana/issues/198251 Adds support for the new ES|QL param syntax in the following places: - Support all param types in `column` AST node types (single level and nested) - Support those new language features in: - AST - Parser - Traversal: `Walker` - Support in validation and autocomplete: - Makes tests pass - Make validation and autocomplete not complain on param usage in - Function names - Function arguments - In function names where aggregation functions expected - Column names - First level - Nested - Exception out of "unknownField" errors ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) --------- Co-authored-by: Stratoula Kalafateli --- packages/kbn-esql-ast/src/builder/builder.ts | 14 +- .../src/mutate/commands/from/metadata.test.ts | 37 ++- .../src/mutate/commands/from/metadata.ts | 22 +- .../src/mutate/commands/sort/index.test.ts | 4 +- .../src/mutate/commands/sort/index.ts | 30 +- .../src/parser/__tests__/columns.test.ts | 305 +++++++++++++++++- packages/kbn-esql-ast/src/parser/factories.ts | 108 ++++--- .../src/pretty_print/leaf_printer.ts | 58 ++-- packages/kbn-esql-ast/src/types.ts | 9 + .../kbn-esql-ast/src/walker/walker.test.ts | 106 ++++++ packages/kbn-esql-ast/src/walker/walker.ts | 24 +- .../src/autocomplete/helper.ts | 17 +- .../src/code_actions/actions.ts | 5 +- .../src/definitions/commands.ts | 3 +- .../src/shared/context.ts | 5 +- .../src/shared/helpers.ts | 67 ++-- .../validation/__tests__/functions.test.ts | 6 +- .../__tests__/validation.params.test.ts | 165 ++++++++++ .../src/validation/errors.ts | 12 +- .../esql_validation_meta_tests.json | 10 +- .../src/validation/validation.test.ts | 2 - .../src/validation/validation.ts | 90 +++--- 22 files changed, 912 insertions(+), 187 deletions(-) diff --git a/packages/kbn-esql-ast/src/builder/builder.ts b/packages/kbn-esql-ast/src/builder/builder.ts index 894ab99e5b3e8..fae1981b454c2 100644 --- a/packages/kbn-esql-ast/src/builder/builder.ts +++ b/packages/kbn-esql-ast/src/builder/builder.ts @@ -9,6 +9,7 @@ /* eslint-disable @typescript-eslint/no-namespace */ +import { LeafPrinter } from '../pretty_print'; import { ESQLAstComment, ESQLAstCommentMultiLine, @@ -125,16 +126,23 @@ export namespace Builder { }; export const column = ( - template: Omit, 'name' | 'quoted'>, + template: Omit, 'name' | 'quoted' | 'parts'>, fromParser?: Partial ): ESQLColumn => { - return { + const node: ESQLColumn = { ...template, ...Builder.parserFields(fromParser), + parts: template.args.map((arg) => + arg.type === 'identifier' ? arg.name : LeafPrinter.param(arg) + ), quoted: false, - name: template.parts.join('.'), + name: '', type: 'column', }; + + node.name = LeafPrinter.column(node); + + return node; }; export const order = ( diff --git a/packages/kbn-esql-ast/src/mutate/commands/from/metadata.test.ts b/packages/kbn-esql-ast/src/mutate/commands/from/metadata.test.ts index b6cb485395a6c..e4161994d224b 100644 --- a/packages/kbn-esql-ast/src/mutate/commands/from/metadata.test.ts +++ b/packages/kbn-esql-ast/src/mutate/commands/from/metadata.test.ts @@ -28,7 +28,13 @@ describe('commands.from.metadata', () => { expect(column).toMatchObject({ type: 'column', - parts: ['a'], + args: [ + { + type: 'identifier', + name: 'a', + }, + ], + // parts: ['a'], }); }); @@ -40,19 +46,39 @@ describe('commands.from.metadata', () => { expect(columns).toMatchObject([ { type: 'column', - parts: ['a'], + args: [ + { + type: 'identifier', + name: 'a', + }, + ], }, { type: 'column', - parts: ['b'], + args: [ + { + type: 'identifier', + name: 'b', + }, + ], }, { type: 'column', - parts: ['_id'], + args: [ + { + type: 'identifier', + name: '_id', + }, + ], }, { type: 'column', - parts: ['_lang'], + args: [ + { + type: 'identifier', + name: '_lang', + }, + ], }, ]); }); @@ -156,7 +182,6 @@ describe('commands.from.metadata', () => { it('return inserted `column` node, and parent `option` node', () => { const src1 = 'FROM index METADATA a'; const { root } = parse(src1); - const tuple = commands.from.metadata.insert(root, 'b'); expect(tuple).toMatchObject([ diff --git a/packages/kbn-esql-ast/src/mutate/commands/from/metadata.ts b/packages/kbn-esql-ast/src/mutate/commands/from/metadata.ts index 5160ab65954cb..4d637a1fd0570 100644 --- a/packages/kbn-esql-ast/src/mutate/commands/from/metadata.ts +++ b/packages/kbn-esql-ast/src/mutate/commands/from/metadata.ts @@ -74,7 +74,10 @@ export const find = ( } const predicate: Predicate<[ESQLColumn, unknown]> = ([field]) => - cmpArr(field.parts, fieldName as string[]); + cmpArr( + field.args.map((arg) => (arg.type === 'identifier' ? arg.name : '')), + fieldName as string[] + ); return findByPredicate(list(ast), predicate); }; @@ -128,7 +131,12 @@ export const remove = ( fieldName = [fieldName]; } - return removeByPredicate(ast, (field) => cmpArr(field.parts, fieldName as string[])); + return removeByPredicate(ast, (field) => + cmpArr( + field.args.map((arg) => (arg.type === 'identifier' ? arg.name : '')), + fieldName as string[] + ) + ); }; /** @@ -161,7 +169,8 @@ export const insert = ( } const parts: string[] = typeof fieldName === 'string' ? [fieldName] : fieldName; - const column = Builder.expression.column({ parts }); + const args = parts.map((part) => Builder.identifier({ name: part })); + const column = Builder.expression.column({ args }); if (index === -1) { option.args.push(column); @@ -195,7 +204,12 @@ export const upsert = ( const parts = Array.isArray(fieldName) ? fieldName : [fieldName]; const existing = Walker.find( option, - (node) => node.type === 'column' && cmpArr(node.parts, parts) + (node) => + node.type === 'column' && + cmpArr( + node.args.map((arg) => (arg.type === 'identifier' ? arg.name : '')), + parts + ) ); if (existing) { return undefined; diff --git a/packages/kbn-esql-ast/src/mutate/commands/sort/index.test.ts b/packages/kbn-esql-ast/src/mutate/commands/sort/index.test.ts index d04f79b96541a..1342a059254fd 100644 --- a/packages/kbn-esql-ast/src/mutate/commands/sort/index.test.ts +++ b/packages/kbn-esql-ast/src/mutate/commands/sort/index.test.ts @@ -244,13 +244,13 @@ describe('commands.sort', () => { args: [ { type: 'column', - parts: ['b', 'a'], + args: [{ name: 'b' }, { name: 'a' }], }, ], }); expect(node2).toMatchObject({ type: 'column', - parts: ['a', 'b'], + args: [{ name: 'a' }, { name: 'b' }], }); }); diff --git a/packages/kbn-esql-ast/src/mutate/commands/sort/index.ts b/packages/kbn-esql-ast/src/mutate/commands/sort/index.ts index d2b2c7cd5f3d4..d2f64e5b6a1cb 100644 --- a/packages/kbn-esql-ast/src/mutate/commands/sort/index.ts +++ b/packages/kbn-esql-ast/src/mutate/commands/sort/index.ts @@ -49,15 +49,17 @@ export type NewSortExpressionTemplate = const createSortExpression = ( template: string | string[] | NewSortExpressionTemplate ): SortExpression => { + const parts: string[] = + typeof template === 'string' + ? [template] + : Array.isArray(template) + ? template + : typeof template.parts === 'string' + ? [template.parts] + : template.parts; + const identifiers = parts.map((part) => Builder.identifier({ name: part })); const column = Builder.expression.column({ - parts: - typeof template === 'string' - ? [template] - : Array.isArray(template) - ? template - : typeof template.parts === 'string' - ? [template.parts] - : template.parts, + args: identifiers, }); if (typeof template === 'string' || Array.isArray(template)) { @@ -189,12 +191,18 @@ export const find = ( return findByPredicate(ast, ([node]) => { let isMatch = false; if (node.type === 'column') { - isMatch = util.cmpArr(node.parts, arrParts); + isMatch = util.cmpArr( + node.args.map((arg) => (arg.type === 'identifier' ? arg.name : '')), + arrParts + ); } else if (node.type === 'order') { - const columnParts = (node.args[0] as ESQLColumn)?.parts; + const columnParts = (node.args[0] as ESQLColumn)?.args; if (Array.isArray(columnParts)) { - isMatch = util.cmpArr(columnParts, arrParts); + isMatch = util.cmpArr( + columnParts.map((arg) => (arg.type === 'identifier' ? arg.name : '')), + arrParts + ); } } diff --git a/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts index 38e98104d41bd..f235005a2c10f 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/columns.test.ts @@ -10,21 +10,86 @@ import { parse } from '..'; describe('Column Identifier Expressions', () => { + it('can parse star column as function argument', () => { + const text = 'ROW fn(*)'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { + type: 'command', + name: 'row', + args: [ + { + type: 'function', + name: 'fn', + args: [ + { + type: 'column', + args: [ + { + type: 'identifier', + name: '*', + }, + ], + }, + ], + }, + ], + }, + ]); + }); + + it('can parse a single identifier', () => { + const text = 'ROW hello'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { + type: 'command', + args: [ + { + type: 'column', + args: [ + { + type: 'identifier', + name: 'hello', + }, + ], + }, + ], + }, + ]); + }); + it('can parse un-quoted identifiers', () => { const text = 'ROW a, b.c'; - const { ast } = parse(text); + const { root } = parse(text); - expect(ast).toMatchObject([ + expect(root.commands).toMatchObject([ { type: 'command', args: [ { type: 'column', - parts: ['a'], + args: [ + { + type: 'identifier', + name: 'a', + }, + ], }, { type: 'column', - parts: ['b', 'c'], + args: [ + { + type: 'identifier', + name: 'b', + }, + { + type: 'identifier', + name: 'c', + }, + ], }, ], }, @@ -33,23 +98,50 @@ describe('Column Identifier Expressions', () => { it('can parse quoted identifiers', () => { const text = 'ROW `a`, `b`.c, `d`.`👍`.`123``123`'; - const { ast } = parse(text); + const { root } = parse(text); - expect(ast).toMatchObject([ + expect(root.commands).toMatchObject([ { type: 'command', args: [ { type: 'column', - parts: ['a'], + args: [ + { + type: 'identifier', + name: 'a', + }, + ], }, { type: 'column', - parts: ['b', 'c'], + args: [ + { + type: 'identifier', + name: 'b', + }, + { + type: 'identifier', + name: 'c', + }, + ], }, { type: 'column', - parts: ['d', '👍', '123`123'], + args: [ + { + type: 'identifier', + name: 'd', + }, + { + type: 'identifier', + name: '👍', + }, + { + type: 'identifier', + name: '123`123', + }, + ], }, ], }, @@ -58,15 +150,28 @@ describe('Column Identifier Expressions', () => { it('can mix quoted and un-quoted identifiers', () => { const text = 'ROW part1.part2.`part``3️⃣`'; - const { ast } = parse(text); + const { root } = parse(text); - expect(ast).toMatchObject([ + expect(root.commands).toMatchObject([ { type: 'command', args: [ { type: 'column', - parts: ['part1', 'part2', 'part`3️⃣'], + args: [ + { + type: 'identifier', + name: 'part1', + }, + { + type: 'identifier', + name: 'part2', + }, + { + type: 'identifier', + name: 'part`3️⃣', + }, + ], }, ], }, @@ -75,19 +180,189 @@ describe('Column Identifier Expressions', () => { it('in KEEP command', () => { const text = 'FROM a | KEEP a.b'; - const { ast } = parse(text); + const { root } = parse(text); - expect(ast).toMatchObject([ + expect(root.commands).toMatchObject([ {}, { type: 'command', args: [ { type: 'column', - parts: ['a', 'b'], + args: [ + { + type: 'identifier', + name: 'a', + }, + { + type: 'identifier', + name: 'b', + }, + ], }, ], }, ]); }); + + describe('params', () => { + it('can parse named param as a single param node', () => { + const text = 'ROW ?test'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { + type: 'command', + args: [ + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'test', + }, + ], + }, + ]); + }); + + it('can parse nested named params as column', () => { + const text = 'ROW ?test1.?test2'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { + type: 'command', + args: [ + { + type: 'column', + args: [ + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'test1', + }, + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'test2', + }, + ], + }, + ], + }, + ]); + }); + + it('can mix param and identifier in column name', () => { + const text = 'ROW ?par.id'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { + type: 'command', + args: [ + { + type: 'column', + args: [ + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'par', + }, + { + type: 'identifier', + name: 'id', + }, + ], + }, + ], + }, + ]); + }); + + it('can mix param and identifier in column name - 2', () => { + const text = 'ROW `😱`.?par'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { + type: 'command', + args: [ + { + type: 'column', + args: [ + { + type: 'identifier', + name: '😱', + }, + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'par', + }, + ], + }, + ], + }, + ]); + }); + + it('supports all three different param types', () => { + const text = 'ROW ?.?name.?123'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { + type: 'command', + args: [ + { + type: 'column', + args: [ + { + type: 'literal', + literalType: 'param', + paramType: 'unnamed', + }, + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'name', + }, + { + type: 'literal', + literalType: 'param', + paramType: 'positional', + value: 123, + }, + ], + }, + ], + }, + ]); + }); + + it('parses DROP command args as "column" nodes', () => { + const text = 'FROM index | DROP any#Char$Field'; + const { root } = parse(text); + + expect(root.commands).toMatchObject([ + { type: 'command' }, + { + type: 'command', + name: 'drop', + args: [ + { + type: 'column', + name: 'any', + }, + ], + }, + ]); + }); + }); }); diff --git a/packages/kbn-esql-ast/src/parser/factories.ts b/packages/kbn-esql-ast/src/parser/factories.ts index b575447f7e744..311dcced8a617 100644 --- a/packages/kbn-esql-ast/src/parser/factories.ts +++ b/packages/kbn-esql-ast/src/parser/factories.ts @@ -31,6 +31,7 @@ import { IdentifierContext, InputParamContext, InputNamedOrPositionalParamContext, + IdentifierOrParameterContext, } from '../antlr/esql_parser'; import { DOUBLE_TICKS_REGEX, SINGLE_BACKTICK, TICKS_REGEX } from './constants'; import type { @@ -227,26 +228,35 @@ export const createFunctionCall = (ctx: FunctionContext): ESQLFunctionCallExpres }; const identifierOrParameter = functionName.identifierOrParameter(); - if (identifierOrParameter) { - const identifier = identifierOrParameter.identifier(); - if (identifier) { - node.operator = createIdentifier(identifier); - } else { - const parameter = identifierOrParameter.parameter(); - if (parameter) { - node.operator = createParam(parameter); - } + + if (identifierOrParameter instanceof IdentifierOrParameterContext) { + const operator = createIdentifierOrParam(identifierOrParameter); + + if (operator) { + node.operator = operator; } } return node; }; -const createIdentifier = (identifier: IdentifierContext): ESQLIdentifier => { - return Builder.identifier( - { name: identifier.getText().toLowerCase() }, - createParserFields(identifier) - ); +export const createIdentifierOrParam = (ctx: IdentifierOrParameterContext) => { + const identifier = ctx.identifier(); + if (identifier) { + return createIdentifier(identifier); + } else { + const parameter = ctx.parameter(); + if (parameter) { + return createParam(parameter); + } + } +}; + +export const createIdentifier = (identifier: IdentifierContext): ESQLIdentifier => { + const text = identifier.getText(); + const name = parseIdentifier(text); + + return Builder.identifier({ name }, createParserFields(identifier)); }; export const createParam = (ctx: ParseTree) => { @@ -473,42 +483,68 @@ export function createSource( export function createColumnStar(ctx: TerminalNode): ESQLColumn { const text = ctx.getText(); - - return { - type: 'column', - name: text, - parts: [text], + const parserFields = { text, location: getPosition(ctx.symbol), incomplete: ctx.getText() === '', quoted: false, }; + const node = Builder.expression.column( + { args: [Builder.identifier({ name: '*' }, parserFields)] }, + parserFields + ); + + node.name = text; + + return node; } export function createColumn(ctx: ParserRuleContext): ESQLColumn { - const parts: string[] = []; + const args: ESQLColumn['args'] = []; + if (ctx instanceof QualifiedNamePatternContext) { - parts.push( - ...ctx.identifierPattern_list().map((identifier) => parseIdentifier(identifier.getText())) - ); + const list = ctx.identifierPattern_list(); + + for (const identifier of list) { + const name = parseIdentifier(identifier.getText()); + const node = Builder.identifier({ name }, createParserFields(identifier)); + + args.push(node); + } } else if (ctx instanceof QualifiedNameContext) { - parts.push( - ...ctx.identifierOrParameter_list().map((identifier) => parseIdentifier(identifier.getText())) - ); + const list = ctx.identifierOrParameter_list(); + + for (const item of list) { + if (item instanceof IdentifierOrParameterContext) { + const node = createIdentifierOrParam(item); + + if (node) { + args.push(node); + } + } + } } else { - parts.push(sanitizeIdentifierString(ctx)); + const name = sanitizeIdentifierString(ctx); + const node = Builder.identifier({ name }, createParserFields(ctx)); + + args.push(node); } + const text = sanitizeIdentifierString(ctx); const hasQuotes = Boolean(getQuotedText(ctx) || isQuoted(ctx.getText())); - return { - type: 'column' as const, - name: text, - parts, - text: ctx.getText(), - location: getPosition(ctx.start, ctx.stop), - incomplete: Boolean(ctx.exception || text === ''), - quoted: hasQuotes, - }; + const column = Builder.expression.column( + { args }, + { + text: ctx.getText(), + location: getPosition(ctx.start, ctx.stop), + incomplete: Boolean(ctx.exception || text === ''), + } + ); + + column.name = text; + column.quoted = hasQuotes; + + return column; } export function createOption(name: string, ctx: ParserRuleContext): ESQLCommandOption { diff --git a/packages/kbn-esql-ast/src/pretty_print/leaf_printer.ts b/packages/kbn-esql-ast/src/pretty_print/leaf_printer.ts index 3c12de90e4454..b413234cbe263 100644 --- a/packages/kbn-esql-ast/src/pretty_print/leaf_printer.ts +++ b/packages/kbn-esql-ast/src/pretty_print/leaf_printer.ts @@ -12,6 +12,7 @@ import { ESQLAstCommentMultiLine, ESQLColumn, ESQLLiteral, + ESQLParamLiteral, ESQLSource, ESQLTimeInterval, } from '../types'; @@ -27,20 +28,37 @@ export const LeafPrinter = { source: (node: ESQLSource) => node.name, column: (node: ESQLColumn) => { - const parts: string[] = node.parts; + const args = node.args; let formatted = ''; - for (const part of parts) { - if (formatted.length > 0) { - formatted += '.'; - } - if (regexUnquotedIdPattern.test(part)) { - formatted += part; - } else { - // Escape backticks "`" with double backticks "``". - const escaped = part.replace(/`/g, '``'); - formatted += '`' + escaped + '`'; + for (const arg of args) { + switch (arg.type) { + case 'identifier': { + const name = arg.name; + + if (formatted.length > 0) { + formatted += '.'; + } + if (regexUnquotedIdPattern.test(name)) { + formatted += name; + } else { + // Escape backticks "`" with double backticks "``". + const escaped = name.replace(/`/g, '``'); + formatted += '`' + escaped + '`'; + } + + break; + } + case 'literal': { + if (formatted.length > 0) { + formatted += '.'; + } + + formatted += LeafPrinter.literal(arg); + + break; + } } } @@ -56,13 +74,7 @@ export const LeafPrinter = { return String(node.value).toUpperCase() === 'TRUE' ? 'TRUE' : 'FALSE'; } case 'param': { - switch (node.paramType) { - case 'named': - case 'positional': - return '?' + node.value; - default: - return '?'; - } + return LeafPrinter.param(node); } case 'keyword': { return String(node.value); @@ -82,6 +94,16 @@ export const LeafPrinter = { } }, + param: (node: ESQLParamLiteral) => { + switch (node.paramType) { + case 'named': + case 'positional': + return '?' + node.value; + default: + return '?'; + } + }, + timeInterval: (node: ESQLTimeInterval) => { const { quantity, unit } = node; diff --git a/packages/kbn-esql-ast/src/types.ts b/packages/kbn-esql-ast/src/types.ts index ea76fc3e0b9a4..2a8513fc2ced1 100644 --- a/packages/kbn-esql-ast/src/types.ts +++ b/packages/kbn-esql-ast/src/types.ts @@ -276,6 +276,15 @@ export interface ESQLSource extends ESQLAstBaseItem { export interface ESQLColumn extends ESQLAstBaseItem { type: 'column'; + /** + * A ES|QL column name can be composed of multiple parts, + * e.g: part1.part2.`part``3️⃣`.?param. Where parts can be quoted, or not + * quoted, or even be a parameter. + * + * The args list contains the parts of the column name. + */ + args: Array; + /** * An identifier can be composed of multiple parts, e.g: part1.part2.`part``3️⃣`. * This property contains the parsed unquoted parts of the identifier. diff --git a/packages/kbn-esql-ast/src/walker/walker.test.ts b/packages/kbn-esql-ast/src/walker/walker.test.ts index 980e1499e62aa..49c50a0f7fa5d 100644 --- a/packages/kbn-esql-ast/src/walker/walker.test.ts +++ b/packages/kbn-esql-ast/src/walker/walker.test.ts @@ -812,6 +812,112 @@ describe('Walker.params()', () => { }, ]); }); + + test('can collect params from column names', () => { + const query = 'ROW ?a.?b'; + const { ast } = parse(query); + const params = Walker.params(ast); + + expect(params).toMatchObject([ + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'a', + }, + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'b', + }, + ]); + }); + + test('can collect params from column names, where first part is not a param', () => { + const query = 'ROW a.?b'; + const { ast } = parse(query); + const params = Walker.params(ast); + + expect(params).toMatchObject([ + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'b', + }, + ]); + }); + + test('can collect all types of param from column name', () => { + const query = 'ROW ?.?0.?a'; + const { ast } = parse(query); + const params = Walker.params(ast); + + expect(params).toMatchObject([ + { + type: 'literal', + literalType: 'param', + paramType: 'unnamed', + }, + { + type: 'literal', + literalType: 'param', + paramType: 'positional', + value: 0, + }, + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'a', + }, + ]); + }); + + test('can collect params from function names', () => { + const query = 'FROM a | STATS ?lala()'; + const { ast } = parse(query); + const params = Walker.params(ast); + + expect(params).toMatchObject([ + { + type: 'literal', + literalType: 'param', + paramType: 'named', + value: 'lala', + }, + ]); + }); + + test('can collect params from function names (unnamed)', () => { + const query = 'FROM a | STATS ?()'; + const { ast } = parse(query); + const params = Walker.params(ast); + + expect(params).toMatchObject([ + { + type: 'literal', + literalType: 'param', + paramType: 'unnamed', + }, + ]); + }); + + test('can collect params from function names (positional)', () => { + const query = 'FROM a | STATS agg(test), ?123()'; + const { ast } = parse(query); + const params = Walker.params(ast); + + expect(params).toMatchObject([ + { + type: 'literal', + literalType: 'param', + paramType: 'positional', + value: 123, + }, + ]); + }); }); describe('Walker.find()', () => { diff --git a/packages/kbn-esql-ast/src/walker/walker.ts b/packages/kbn-esql-ast/src/walker/walker.ts index dbbbc3b090f29..f3b6de91649b7 100644 --- a/packages/kbn-esql-ast/src/walker/walker.ts +++ b/packages/kbn-esql-ast/src/walker/walker.ts @@ -20,6 +20,7 @@ import type { ESQLCommandMode, ESQLCommandOption, ESQLFunction, + ESQLIdentifier, ESQLInlineCast, ESQLList, ESQLLiteral, @@ -49,6 +50,7 @@ export interface WalkerOptions { visitTimeIntervalLiteral?: (node: ESQLTimeInterval) => void; visitInlineCast?: (node: ESQLInlineCast) => void; visitUnknown?: (node: ESQLUnknownItem) => void; + visitIdentifier?: (node: ESQLIdentifier) => void; /** * Called for any node type that does not have a specific visitor. @@ -346,11 +348,27 @@ export class Walker { } } + public walkColumn(node: ESQLColumn): void { + const { options } = this; + const { args } = node; + + (options.visitColumn ?? options.visitAny)?.(node); + + if (args) { + for (const value of args) { + this.walkAstItem(value); + } + } + } + public walkFunction(node: ESQLFunction): void { const { options } = this; (options.visitFunction ?? options.visitAny)?.(node); const args = node.args; const length = args.length; + + if (node.operator) this.walkAstItem(node.operator); + for (let i = 0; i < length; i++) { const arg = args[i]; this.walkAstItem(arg); @@ -393,7 +411,7 @@ export class Walker { break; } case 'column': { - (options.visitColumn ?? options.visitAny)?.(node); + this.walkColumn(node); break; } case 'literal': { @@ -412,6 +430,10 @@ export class Walker { (options.visitInlineCast ?? options.visitAny)?.(node); break; } + case 'identifier': { + (options.visitIdentifier ?? options.visitAny)?.(node); + break; + } case 'unknown': { (options.visitUnknown ?? options.visitAny)?.(node); break; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index 9724878611b01..3ccddfc5ff241 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -27,6 +27,7 @@ import { isAssignment, isColumnItem, isFunctionItem, + isIdentifier, isLiteralItem, isTimeIntervalItem, } from '../shared/helpers'; @@ -457,15 +458,13 @@ export function extractTypeFromASTArg( if (Array.isArray(arg)) { return extractTypeFromASTArg(arg[0], references); } - if (isColumnItem(arg) || isLiteralItem(arg)) { - if (isLiteralItem(arg)) { - return arg.literalType; - } - if (isColumnItem(arg)) { - const hit = getColumnForASTNode(arg, references); - if (hit) { - return hit.type; - } + if (isLiteralItem(arg)) { + return arg.literalType; + } + if (isColumnItem(arg) || isIdentifier(arg)) { + const hit = getColumnForASTNode(arg, references); + if (hit) { + return hit.type; } } if (isTimeIntervalItem(arg)) { diff --git a/packages/kbn-esql-validation-autocomplete/src/code_actions/actions.ts b/packages/kbn-esql-validation-autocomplete/src/code_actions/actions.ts index 37ab56350ffb2..7c9d5d7ae8ba2 100644 --- a/packages/kbn-esql-validation-autocomplete/src/code_actions/actions.ts +++ b/packages/kbn-esql-validation-autocomplete/src/code_actions/actions.ts @@ -20,6 +20,7 @@ import { getAllFunctions, getCommandDefinition, isColumnItem, + isIdentifier, isSourceItem, shouldBeQuotedText, } from '../shared/helpers'; @@ -138,7 +139,7 @@ function extractUnquotedFieldText( if (errorType === 'syntaxError') { // scope it down to column items for now const { node } = getAstContext(query, ast, possibleStart - 1); - if (node && isColumnItem(node)) { + if (node && (isColumnItem(node) || isIdentifier(node))) { return { start: node.location.min + 1, name: query.substring(node.location.min, end).trimEnd(), @@ -379,7 +380,7 @@ function inferCodeFromError( if (error.message.startsWith('SyntaxError: token recognition error at:')) { // scope it down to column items for now const { node } = getAstContext(rawText, ast, error.startColumn - 2); - return node && isColumnItem(node) ? 'quotableFields' : undefined; + return node && (isColumnItem(node) || isIdentifier(node)) ? 'quotableFields' : undefined; } } diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts index d07511665ad31..fd6dbfbd453e5 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts @@ -20,6 +20,7 @@ import { isAssignment, isColumnItem, isFunctionItem, + isFunctionOperatorParam, isLiteralItem, } from '../shared/helpers'; import { ENRICH_MODES } from './settings'; @@ -72,7 +73,7 @@ const statsValidator = (command: ESQLCommand) => { function checkAggExistence(arg: ESQLFunction): boolean { // TODO the grouping function check may not // hold true for all future cases - if (isAggFunction(arg)) { + if (isAggFunction(arg) || isFunctionOperatorParam(arg)) { return true; } if (isOtherFunction(arg)) { diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/context.ts b/packages/kbn-esql-validation-autocomplete/src/shared/context.ts index 42e63d7623e49..cc7c36abf64f7 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/context.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/context.ts @@ -26,6 +26,7 @@ import { isSettingItem, pipePrecedesCurrentWord, getFunctionDefinition, + isIdentifier, } from './helpers'; function findNode(nodes: ESQLAstItem[], offset: number): ESQLSingleAstItem | undefined { @@ -87,7 +88,9 @@ function findCommandSubType( function isMarkerNode(node: ESQLSingleAstItem | undefined): boolean { return Boolean( - node && (isColumnItem(node) || isSourceItem(node)) && node.name.endsWith(EDITOR_MARKER) + node && + (isColumnItem(node) || isIdentifier(node) || isSourceItem(node)) && + node.name.endsWith(EDITOR_MARKER) ); } diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index 43bbb2b571a50..e86cb4f6ae8f2 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -7,18 +7,24 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import type { - ESQLAstItem, - ESQLColumn, - ESQLCommandMode, - ESQLCommandOption, - ESQLFunction, - ESQLLiteral, - ESQLSingleAstItem, - ESQLSource, - ESQLTimeInterval, +import { + Walker, + type ESQLAstItem, + type ESQLColumn, + type ESQLCommandMode, + type ESQLCommandOption, + type ESQLFunction, + type ESQLLiteral, + type ESQLSingleAstItem, + type ESQLSource, + type ESQLTimeInterval, } from '@kbn/esql-ast'; -import { ESQLInlineCast, ESQLParamLiteral } from '@kbn/esql-ast/src/types'; +import { + ESQLIdentifier, + ESQLInlineCast, + ESQLParamLiteral, + ESQLProperNode, +} from '@kbn/esql-ast/src/types'; import { aggregationFunctionDefinitions } from '../definitions/generated/aggregation_functions'; import { builtinFunctions } from '../definitions/builtin'; import { commandDefinitions } from '../definitions/commands'; @@ -78,6 +84,10 @@ export function isColumnItem(arg: ESQLAstItem): arg is ESQLColumn { return isSingleItem(arg) && arg.type === 'column'; } +export function isIdentifier(arg: ESQLAstItem): arg is ESQLIdentifier { + return isSingleItem(arg) && arg.type === 'identifier'; +} + export function isLiteralItem(arg: ESQLAstItem): arg is ESQLLiteral { return isSingleItem(arg) && arg.type === 'literal'; } @@ -254,10 +264,11 @@ function doesLiteralMatchParameterType(argType: FunctionParameterType, item: ESQ * This function returns the variable or field matching a column */ export function getColumnForASTNode( - column: ESQLColumn, + node: ESQLColumn | ESQLIdentifier, { fields, variables }: Pick ): ESQLRealField | ESQLVariable | undefined { - return getColumnByName(column.parts.join('.'), { fields, variables }); + const formatted = node.type === 'identifier' ? node.name : node.parts.join('.'); + return getColumnByName(formatted, { fields, variables }); } /** @@ -438,7 +449,10 @@ export function checkFunctionArgMatchesDefinition( parentCommand?: string ) { const argType = parameterDefinition.type; - if (argType === 'any' || isParam(arg)) { + if (argType === 'any') { + return true; + } + if (isParam(arg)) { return true; } if (arg.type === 'literal') { @@ -465,7 +479,8 @@ export function checkFunctionArgMatchesDefinition( const wrappedTypes: Array<(typeof validHit)['type']> = Array.isArray(validHit.type) ? validHit.type : [validHit.type]; - return wrappedTypes.some((ct) => ct === argType || ct === 'null'); + + return wrappedTypes.some((ct) => ct === argType || ct === 'null' || ct === 'unknown'); } if (arg.type === 'inlineCast') { const lowerArgType = argType?.toLowerCase(); @@ -543,20 +558,20 @@ export function isVariable( * * E.g. "`bytes`" will be "`bytes`" * - * @param column + * @param node * @returns */ -export const getQuotedColumnName = (column: ESQLColumn) => - column.quoted ? column.text : column.name; +export const getQuotedColumnName = (node: ESQLColumn | ESQLIdentifier) => + node.type === 'identifier' ? node.name : node.quoted ? node.text : node.name; /** * TODO - consider calling lookupColumn under the hood of this function. Seems like they should really do the same thing. */ export function getColumnExists( - column: ESQLColumn, + node: ESQLColumn | ESQLIdentifier, { fields, variables }: Pick ) { - const columnName = column.parts.join('.'); + const columnName = node.type === 'identifier' ? node.name : node.parts.join('.'); if (fields.has(columnName) || variables.has(columnName)) { return true; } @@ -645,6 +660,18 @@ export const isParam = (x: unknown): x is ESQLParamLiteral => (x as ESQLParamLiteral).type === 'literal' && (x as ESQLParamLiteral).literalType === 'param'; +export const isFunctionOperatorParam = (fn: ESQLFunction): boolean => + !!fn.operator && isParam(fn.operator); + +/** + * Returns `true` if the function is an aggregation function or a function + * name is a parameter, which potentially could be an aggregation function. + */ +export const isMaybeAggFunction = (fn: ESQLFunction): boolean => + isAggFunction(fn) || isFunctionOperatorParam(fn); + +export const isParametrized = (node: ESQLProperNode): boolean => Walker.params(node).length > 0; + /** * Compares two strings in a case-insensitive manner */ diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/functions.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/functions.test.ts index ec0fbe5395334..d0d2a1bfec0a4 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/functions.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/functions.test.ts @@ -552,11 +552,7 @@ describe('function validation', () => { | EVAL foo = TEST1(1.) | EVAL TEST2(foo) | EVAL TEST3(foo)`, - [ - 'Argument of [test1] must be [keyword], found value [1.] type [double]', - 'Argument of [test2] must be [keyword], found value [foo] type [unknown]', - 'Argument of [test3] must be [long], found value [foo] type [unknown]', - ] + ['Argument of [test1] must be [keyword], found value [1.] type [double]'] ); }); diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/validation.params.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/validation.params.test.ts index bbb2867981425..e2de846651b07 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/validation.params.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/__tests__/validation.params.test.ts @@ -42,3 +42,168 @@ test('allow params in WHERE command expressions', async () => { expect(res2).toMatchObject({ errors: [], warnings: [] }); expect(res3).toMatchObject({ errors: [], warnings: [] }); }); + +describe('allows named params', () => { + test('WHERE boolean expression can contain a param', async () => { + const { validate } = await setup(); + + const res0 = await validate('FROM index | STATS var = ?func(?field) | WHERE var >= ?value'); + expect(res0).toMatchObject({ errors: [], warnings: [] }); + + const res1 = await validate('FROM index | STATS var = ?param | WHERE var >= ?value'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('FROM index | STATS var = ?param | WHERE var >= ?value'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + + const res3 = await validate('FROM index | STATS var = ?param | WHERE ?value >= var'); + expect(res3).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in column names', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW ?test'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('ROW ?test, ?one_more, ?asldfkjasldkfjasldkfj'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in nested column names', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW ?test.?test2'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('ROW ?test, ?test.?test2.?test3'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in nested column names, where first part is not a param', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW not_a_param.?test2'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('ROW not_a_param.?asdfasdfasdf, not_a_param.?test2.?test3'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in function name, function arg, and column name in STATS command', async () => { + const { validate } = await setup(); + + const res1 = await validate('FROM index | STATS x = max(doubleField) BY textField'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('FROM index | STATS x = max(?param1) BY textField'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + + const res3 = await validate('FROM index | STATS x = max(?param1) BY ?param2'); + expect(res3).toMatchObject({ errors: [], warnings: [] }); + + const res4 = await validate('FROM index | STATS x = ?param3(?param1) BY ?param2'); + expect(res4).toMatchObject({ errors: [], warnings: [] }); + + const res5 = await validate( + 'FROM index | STATS x = ?param3(?param1, ?param4), y = ?param4(?param4, ?param4, ?param4) BY ?param2, ?param5' + ); + expect(res5).toMatchObject({ errors: [], warnings: [] }); + }); +}); + +describe('allows unnamed params', () => { + test('in column names', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW ?'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in nested column names', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW ?.?'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('ROW ?, ?.?.?'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in nested column names, where first part is not a param', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW not_a_param.?'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('ROW not_a_param.?, not_a_param.?.?'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in function name, function arg, and column name in STATS command', async () => { + const { validate } = await setup(); + + const res1 = await validate('FROM index | STATS x = max(doubleField) BY textField'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('FROM index | STATS x = max(?) BY textField'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + + const res3 = await validate('FROM index | STATS x = max(?) BY ?'); + expect(res3).toMatchObject({ errors: [], warnings: [] }); + + const res4 = await validate('FROM index | STATS x = ?(?) BY ?'); + expect(res4).toMatchObject({ errors: [], warnings: [] }); + + const res5 = await validate('FROM index | STATS x = ?(?, ?), y = ?(?, ?, ?) BY ?, ?'); + expect(res5).toMatchObject({ errors: [], warnings: [] }); + }); +}); + +describe('allows positional params', () => { + test('in column names', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW ?0'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in nested column names', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW ?0.?0'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('ROW ?0, ?0.?0.?0'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in nested column names, where first part is not a param', async () => { + const { validate } = await setup(); + + const res1 = await validate('ROW not_a_param.?1'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('ROW not_a_param.?2, not_a_param.?3.?4'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + }); + + test('in function name, function arg, and column name in STATS command', async () => { + const { validate } = await setup(); + + const res1 = await validate('FROM index | STATS x = max(doubleField) BY textField'); + expect(res1).toMatchObject({ errors: [], warnings: [] }); + + const res2 = await validate('FROM index | STATS x = max(?0) BY textField'); + expect(res2).toMatchObject({ errors: [], warnings: [] }); + + const res3 = await validate('FROM index | STATS x = max(?0) BY ?0'); + expect(res3).toMatchObject({ errors: [], warnings: [] }); + + const res4 = await validate('FROM index | STATS x = ?1(?1) BY ?1'); + expect(res4).toMatchObject({ errors: [], warnings: [] }); + + const res5 = await validate('FROM index | STATS x = ?0(?0, ?0), y = ?2(?2, ?2, ?2) BY ?3, ?3'); + expect(res5).toMatchObject({ errors: [], warnings: [] }); + }); +}); diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts b/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts index ae00f300c1878..0f82d7fe4aad9 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts @@ -15,6 +15,7 @@ import type { ESQLLocation, ESQLMessage, } from '@kbn/esql-ast'; +import { ESQLIdentifier } from '@kbn/esql-ast/src/types'; import type { ErrorTypes, ErrorValues } from './types'; function getMessageAndTypeFromId({ @@ -477,7 +478,7 @@ export const errors = { unknownFunction: (fn: ESQLFunction): ESQLMessage => errors.byId('unknownFunction', fn.location, fn), - unknownColumn: (column: ESQLColumn): ESQLMessage => + unknownColumn: (column: ESQLColumn | ESQLIdentifier): ESQLMessage => errors.byId('unknownColumn', column.location, { name: column.name, }), @@ -494,9 +495,12 @@ export const errors = { expression: fn.text, }), - unknownAggFunction: (col: ESQLColumn, type: string = 'FieldAttribute'): ESQLMessage => - errors.byId('unknownAggregateFunction', col.location, { - value: col.name, + unknownAggFunction: ( + node: ESQLColumn | ESQLIdentifier, + type: string = 'FieldAttribute' + ): ESQLMessage => + errors.byId('unknownAggregateFunction', node.location, { + value: node.name, type, }), 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 25fb880da5ff0..3639a6be4d010 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 @@ -2666,8 +2666,7 @@ { "query": "from a_index | dissect textField .", "error": [ - "SyntaxError: mismatched input '' expecting {'?', NAMED_OR_POSITIONAL_PARAM, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER}", - "Unknown column [textField.]" + "SyntaxError: mismatched input '' expecting {'?', NAMED_OR_POSITIONAL_PARAM, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER}" ], "warning": [] }, @@ -2761,8 +2760,7 @@ { "query": "from a_index | grok textField .", "error": [ - "SyntaxError: mismatched input '' expecting {'?', NAMED_OR_POSITIONAL_PARAM, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER}", - "Unknown column [textField.]" + "SyntaxError: mismatched input '' expecting {'?', NAMED_OR_POSITIONAL_PARAM, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER}" ], "warning": [] }, @@ -9955,11 +9953,11 @@ "warning": [] }, { - "query": "from index METADATA _id, _source METADATA _id2", + "query": "from index metadata _id, _source METADATA _id2", "error": [ "SyntaxError: mismatched input 'METADATA' expecting " ], "warning": [] } ] -} +} \ No newline at end of file diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts index e6546ec996547..6cfa511c0386e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts @@ -700,7 +700,6 @@ describe('validation logic', () => { ]); testErrorsAndWarnings('from a_index | dissect textField .', [ "SyntaxError: mismatched input '' expecting {'?', NAMED_OR_POSITIONAL_PARAM, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER}", - 'Unknown column [textField.]', ]); testErrorsAndWarnings('from a_index | dissect textField %a', [ "SyntaxError: mismatched input '%' expecting QUOTED_STRING", @@ -751,7 +750,6 @@ describe('validation logic', () => { ]); testErrorsAndWarnings('from a_index | grok textField .', [ "SyntaxError: mismatched input '' expecting {'?', NAMED_OR_POSITIONAL_PARAM, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER}", - 'Unknown column [textField.]', ]); testErrorsAndWarnings('from a_index | grok textField %a', [ "SyntaxError: mismatched input '%' expecting QUOTED_STRING", diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index 111fe79b3f5e0..b43a9e5c336b5 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -21,7 +21,7 @@ import { ESQLSource, walk, } from '@kbn/esql-ast'; -import type { ESQLAstField } from '@kbn/esql-ast/src/types'; +import type { ESQLAstField, ESQLIdentifier } from '@kbn/esql-ast/src/types'; import { CommandModeDefinition, CommandOptionsDefinition, @@ -54,6 +54,10 @@ import { getQuotedColumnName, isInlineCastItem, getSignaturesWithMatchingArity, + isIdentifier, + isFunctionOperatorParam, + isMaybeAggFunction, + isParametrized, } from '../shared/helpers'; import { collectVariables } from '../shared/variables'; import { getMessageFromId, errors } from './errors'; @@ -235,7 +239,7 @@ function validateFunctionColumnArg( parentCommand: string ) { const messages: ESQLMessage[] = []; - if (!isColumnItem(actualArg)) { + if (!(isColumnItem(actualArg) || isIdentifier(actualArg))) { return messages; } @@ -317,7 +321,7 @@ function removeInlineCasts(arg: ESQLAstItem): ESQLAstItem { } function validateFunction( - astFunction: ESQLFunction, + fn: ESQLFunction, parentCommand: string, parentOption: string | undefined, references: ReferenceMaps, @@ -326,16 +330,20 @@ function validateFunction( ): ESQLMessage[] { const messages: ESQLMessage[] = []; - if (astFunction.incomplete) { + if (fn.incomplete) { return messages; } - const fnDefinition = getFunctionDefinition(astFunction.name)!; - const isFnSupported = isSupportedFunction(astFunction.name, parentCommand, parentOption); + if (isFunctionOperatorParam(fn)) { + return messages; + } + + const fnDefinition = getFunctionDefinition(fn.name)!; + const isFnSupported = isSupportedFunction(fn.name, parentCommand, parentOption); if (!isFnSupported.supported) { if (isFnSupported.reason === 'unknownFunction') { - messages.push(errors.unknownFunction(astFunction)); + messages.push(errors.unknownFunction(fn)); } // for nested functions skip this check and make the nested check fail later on if (isFnSupported.reason === 'unsupportedFunction' && !isNested) { @@ -344,16 +352,16 @@ function validateFunction( ? getMessageFromId({ messageId: 'unsupportedFunctionForCommandOption', values: { - name: astFunction.name, + name: fn.name, command: parentCommand.toUpperCase(), option: parentOption.toUpperCase(), }, - locations: astFunction.location, + locations: fn.location, }) : getMessageFromId({ messageId: 'unsupportedFunctionForCommand', - values: { name: astFunction.name, command: parentCommand.toUpperCase() }, - locations: astFunction.location, + values: { name: fn.name, command: parentCommand.toUpperCase() }, + locations: fn.location, }) ); } @@ -361,7 +369,7 @@ function validateFunction( return messages; } } - const matchingSignatures = getSignaturesWithMatchingArity(fnDefinition, astFunction); + const matchingSignatures = getSignaturesWithMatchingArity(fnDefinition, fn); if (!matchingSignatures.length) { const { max, min } = getMaxMinNumberOfParams(fnDefinition); if (max === min) { @@ -369,24 +377,24 @@ function validateFunction( getMessageFromId({ messageId: 'wrongArgumentNumber', values: { - fn: astFunction.name, + fn: fn.name, numArgs: max, - passedArgs: astFunction.args.length, + passedArgs: fn.args.length, }, - locations: astFunction.location, + locations: fn.location, }) ); - } else if (astFunction.args.length > max) { + } else if (fn.args.length > max) { messages.push( getMessageFromId({ messageId: 'wrongArgumentNumberTooMany', values: { - fn: astFunction.name, + fn: fn.name, numArgs: max, - passedArgs: astFunction.args.length, - extraArgs: astFunction.args.length - max, + passedArgs: fn.args.length, + extraArgs: fn.args.length - max, }, - locations: astFunction.location, + locations: fn.location, }) ); } else { @@ -394,19 +402,19 @@ function validateFunction( getMessageFromId({ messageId: 'wrongArgumentNumberTooFew', values: { - fn: astFunction.name, + fn: fn.name, numArgs: min, - passedArgs: astFunction.args.length, - missingArgs: min - astFunction.args.length, + passedArgs: fn.args.length, + missingArgs: min - fn.args.length, }, - locations: astFunction.location, + locations: fn.location, }) ); } } // now perform the same check on all functions args - for (let i = 0; i < astFunction.args.length; i++) { - const arg = astFunction.args[i]; + for (let i = 0; i < fn.args.length; i++) { + const arg = fn.args[i]; const allMatchingArgDefinitionsAreConstantOnly = matchingSignatures.every((signature) => { return signature.params[i]?.constantOnly; @@ -446,7 +454,7 @@ function validateFunction( // use the nesting flag for now just for stats and metrics // TODO: revisit this part later on to make it more generic ['stats', 'inlinestats', 'metrics'].includes(parentCommand) - ? isNested || !isAssignment(astFunction) + ? isNested || !isAssignment(fn) : false ); @@ -454,7 +462,7 @@ function validateFunction( const consolidatedMessage = getMessageFromId({ messageId: 'expectedConstant', values: { - fn: astFunction.name, + fn: fn.name, given: subArg.text, }, locations: subArg.location, @@ -472,7 +480,7 @@ function validateFunction( } // check if the definition has some specific validation to apply: if (fnDefinition.validate) { - const payloads = fnDefinition.validate(astFunction); + const payloads = fnDefinition.validate(fn); if (payloads.length) { messages.push(...payloads); } @@ -481,7 +489,7 @@ function validateFunction( const failingSignatures: ESQLMessage[][] = []; for (const signature of matchingSignatures) { const failingSignature: ESQLMessage[] = []; - astFunction.args.forEach((outerArg, index) => { + fn.args.forEach((outerArg, index) => { const argDef = getParamAtPosition(signature, index); if ((!outerArg && argDef?.optional) || !argDef) { // that's ok, just skip it @@ -502,7 +510,7 @@ function validateFunction( validateInlineCastArg, ].flatMap((validateFn) => { return validateFn( - astFunction, + fn, arg, { ...argDef, @@ -521,7 +529,7 @@ function validateFunction( ? collapseWrongArgumentTypeMessages( messagesFromAllArgElements, outerArg, - astFunction.name, + fn.name, argDef.type as string, parentCommand, references @@ -599,10 +607,11 @@ function validateSetting( * recursively terminate at either a literal or an aggregate function. */ const isFunctionAggClosed = (fn: ESQLFunction): boolean => - isAggFunction(fn) || areFunctionArgsAggClosed(fn); + isMaybeAggFunction(fn) || areFunctionArgsAggClosed(fn); const areFunctionArgsAggClosed = (fn: ESQLFunction): boolean => - fn.args.every((arg) => isLiteralItem(arg) || (isFunctionItem(arg) && isFunctionAggClosed(arg))); + fn.args.every((arg) => isLiteralItem(arg) || (isFunctionItem(arg) && isFunctionAggClosed(arg))) || + isFunctionOperatorParam(fn); /** * Looks for first nested aggregate function in an aggregate function, recursively. @@ -610,7 +619,7 @@ const areFunctionArgsAggClosed = (fn: ESQLFunction): boolean => const findNestedAggFunctionInAggFunction = (agg: ESQLFunction): ESQLFunction | undefined => { for (const arg of agg.args) { if (isFunctionItem(arg)) { - return isAggFunction(arg) ? arg : findNestedAggFunctionInAggFunction(arg); + return isMaybeAggFunction(arg) ? arg : findNestedAggFunctionInAggFunction(arg); } } }; @@ -627,7 +636,7 @@ const findNestedAggFunction = ( fn: ESQLFunction, parentIsAgg: boolean = false ): ESQLFunction | undefined => { - if (isAggFunction(fn)) { + if (isMaybeAggFunction(fn)) { return parentIsAgg ? fn : findNestedAggFunctionInAggFunction(fn); } @@ -675,7 +684,7 @@ const validateAggregates = ( hasMissingAggregationFunctionError = true; messages.push(errors.noAggFunction(command, aggregate)); } - } else if (isColumnItem(aggregate)) { + } else if (isColumnItem(aggregate) || isIdentifier(aggregate)) { messages.push(errors.unknownAggFunction(aggregate)); } else { // Should never happen. @@ -834,14 +843,13 @@ function validateSource( } function validateColumnForCommand( - column: ESQLColumn, + column: ESQLColumn | ESQLIdentifier, commandName: string, references: ReferenceMaps ): ESQLMessage[] { const messages: ESQLMessage[] = []; - if (commandName === 'row') { - if (!references.variables.has(column.name)) { + if (!references.variables.has(column.name) && !isParametrized(column)) { messages.push(errors.unknownColumn(column)); } } else { @@ -990,7 +998,7 @@ function validateCommand(command: ESQLCommand, references: ReferenceMaps): ESQLM ) ); } - if (isColumnItem(arg)) { + if (isColumnItem(arg) || isIdentifier(arg)) { if (command.name === 'stats' || command.name === 'inlinestats') { messages.push(errors.unknownAggFunction(arg)); } else {