From 070ea622ed0c781dd0d297abbaba6408a6817fe1 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Sat, 19 Oct 2024 05:02:43 +1100 Subject: [PATCH] [8.x] [ES|QL] Create expression type evaluator (#195989) (#196922) # Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] Create expression type evaluator (#195989)](https://github.com/elastic/kibana/pull/195989) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) Co-authored-by: Drew Tate --- .../src/parser/__tests__/literal.test.ts | 4 +- packages/kbn-esql-ast/src/parser/factories.ts | 9 +- packages/kbn-esql-ast/src/parser/walkers.ts | 14 +- .../wrapping_pretty_printer.comments.test.ts | 2 +- .../src/pretty_print/leaf_printer.ts | 4 +- packages/kbn-esql-ast/src/types.ts | 31 +- packages/kbn-esql-ast/src/visitor/contexts.ts | 2 +- .../kbn-esql-ast/src/walker/walker.test.ts | 20 +- .../scripts/generate_function_definitions.ts | 2 +- .../autocomplete.command.stats.test.ts | 9 +- .../src/autocomplete/autocomplete.ts | 9 +- .../src/autocomplete/complete_items.ts | 6 +- .../src/autocomplete/helper.ts | 17 - .../src/definitions/builtin.ts | 47 +-- .../definitions/generated/scalar_functions.ts | 2 +- .../src/definitions/options.ts | 2 +- .../src/definitions/types.ts | 6 +- .../src/shared/context.ts | 2 +- .../src/shared/esql_types.ts | 36 +-- .../src/shared/helpers.test.ts | 297 +++++++++++++++++- .../src/shared/helpers.ts | 192 +++++++++-- .../src/shared/variables.ts | 73 +---- .../validation/__tests__/functions.test.ts | 24 +- .../esql_validation_meta_tests.json | 22 +- .../src/validation/types.ts | 7 +- .../src/validation/validation.test.ts | 14 +- .../src/validation/validation.ts | 23 +- .../translations/translations/fr-FR.json | 1 - .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 30 files changed, 609 insertions(+), 270 deletions(-) diff --git a/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts b/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts index 7f50198c96047..ede899d4d8058 100644 --- a/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts +++ b/packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts @@ -24,7 +24,7 @@ describe('literal expression', () => { }); }); - it('decimals vs integers', () => { + it('doubles vs integers', () => { const text = 'ROW a(1.0, 1)'; const { ast } = parse(text); @@ -36,7 +36,7 @@ describe('literal expression', () => { args: [ { type: 'literal', - literalType: 'decimal', + literalType: 'double', }, { type: 'literal', diff --git a/packages/kbn-esql-ast/src/parser/factories.ts b/packages/kbn-esql-ast/src/parser/factories.ts index 321ca6a40dcd0..0fffb3a970e4c 100644 --- a/packages/kbn-esql-ast/src/parser/factories.ts +++ b/packages/kbn-esql-ast/src/parser/factories.ts @@ -41,6 +41,7 @@ import type { FunctionSubtype, ESQLNumericLiteral, ESQLOrderExpression, + InlineCastingType, } from '../types'; import { parseIdentifier, getPosition } from './helpers'; import { Builder, type AstNodeParserFields } from '../builder'; @@ -72,7 +73,7 @@ export const createCommand = (name: string, ctx: ParserRuleContext) => export const createInlineCast = (ctx: InlineCastContext, value: ESQLInlineCast['value']) => Builder.expression.inlineCast( - { castType: ctx.dataType().getText(), value }, + { castType: ctx.dataType().getText().toLowerCase() as InlineCastingType, value }, createParserFields(ctx) ); @@ -107,7 +108,7 @@ export function createLiteralString(token: Token): ESQLLiteral { const text = token.text!; return { type: 'literal', - literalType: 'string', + literalType: 'keyword', text, name: text, value: text, @@ -149,13 +150,13 @@ export function createLiteral( location: getPosition(node.symbol), incomplete: isMissingText(text), }; - if (type === 'decimal' || type === 'integer') { + if (type === 'double' || type === 'integer') { return { ...partialLiteral, literalType: type, value: Number(text), paramType: 'number', - } as ESQLNumericLiteral<'decimal'> | ESQLNumericLiteral<'integer'>; + } as ESQLNumericLiteral<'double'> | ESQLNumericLiteral<'integer'>; } else if (type === 'param') { throw new Error('Should never happen'); } diff --git a/packages/kbn-esql-ast/src/parser/walkers.ts b/packages/kbn-esql-ast/src/parser/walkers.ts index cccc215ec365e..30c17c56483f8 100644 --- a/packages/kbn-esql-ast/src/parser/walkers.ts +++ b/packages/kbn-esql-ast/src/parser/walkers.ts @@ -346,7 +346,7 @@ function getConstant(ctx: ConstantContext): ESQLAstItem { // Decimal type covers multiple ES|QL types: long, double, etc. if (ctx instanceof DecimalLiteralContext) { - return createNumericLiteral(ctx.decimalValue(), 'decimal'); + return createNumericLiteral(ctx.decimalValue(), 'double'); } // Integer type encompasses integer @@ -358,7 +358,7 @@ function getConstant(ctx: ConstantContext): ESQLAstItem { } if (ctx instanceof StringLiteralContext) { // String literal covers multiple ES|QL types: text and keyword types - return createLiteral('string', ctx.string_().QUOTED_STRING()); + return createLiteral('keyword', ctx.string_().QUOTED_STRING()); } if ( ctx instanceof NumericArrayLiteralContext || @@ -371,14 +371,14 @@ function getConstant(ctx: ConstantContext): ESQLAstItem { const isDecimal = numericValue.decimalValue() !== null && numericValue.decimalValue() !== undefined; const value = numericValue.decimalValue() || numericValue.integerValue(); - values.push(createNumericLiteral(value!, isDecimal ? 'decimal' : 'integer')); + values.push(createNumericLiteral(value!, isDecimal ? 'double' : 'integer')); } for (const booleanValue of ctx.getTypedRuleContexts(BooleanValueContext)) { values.push(getBooleanValue(booleanValue)!); } for (const string of ctx.getTypedRuleContexts(StringContext)) { // String literal covers multiple ES|QL types: text and keyword types - const literal = createLiteral('string', string.QUOTED_STRING()); + const literal = createLiteral('keyword', string.QUOTED_STRING()); if (literal) { values.push(literal); } @@ -534,7 +534,7 @@ function collectRegexExpression(ctx: BooleanExpressionContext): ESQLFunction[] { const arg = visitValueExpression(regex.valueExpression()); if (arg) { fn.args.push(arg); - const literal = createLiteral('string', regex._pattern.QUOTED_STRING()); + const literal = createLiteral('keyword', regex._pattern.QUOTED_STRING()); if (literal) { fn.args.push(literal); } @@ -672,7 +672,7 @@ export function visitDissect(ctx: DissectCommandContext) { return [ visitPrimaryExpression(ctx.primaryExpression()), ...(pattern && textExistsAndIsValid(pattern.getText()) - ? [createLiteral('string', pattern), ...visitDissectOptions(ctx.commandOptions())] + ? [createLiteral('keyword', pattern), ...visitDissectOptions(ctx.commandOptions())] : []), ].filter(nonNullable); } @@ -682,7 +682,7 @@ export function visitGrok(ctx: GrokCommandContext) { return [ visitPrimaryExpression(ctx.primaryExpression()), ...(pattern && textExistsAndIsValid(pattern.getText()) - ? [createLiteral('string', pattern)] + ? [createLiteral('keyword', pattern)] : []), ].filter(nonNullable); } diff --git a/packages/kbn-esql-ast/src/pretty_print/__tests__/wrapping_pretty_printer.comments.test.ts b/packages/kbn-esql-ast/src/pretty_print/__tests__/wrapping_pretty_printer.comments.test.ts index 3ac79acda8af3..861d274493a42 100644 --- a/packages/kbn-esql-ast/src/pretty_print/__tests__/wrapping_pretty_printer.comments.test.ts +++ b/packages/kbn-esql-ast/src/pretty_print/__tests__/wrapping_pretty_printer.comments.test.ts @@ -399,7 +399,7 @@ ROW // 2 /* 3 */ // 4 - /* 5 */ /* 6 */ 1::INTEGER /* 7 */ /* 8 */ // 9`); + /* 5 */ /* 6 */ 1::integer /* 7 */ /* 8 */ // 9`); }); }); 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 eb7eaf1075c70..3c12de90e4454 100644 --- a/packages/kbn-esql-ast/src/pretty_print/leaf_printer.ts +++ b/packages/kbn-esql-ast/src/pretty_print/leaf_printer.ts @@ -64,10 +64,10 @@ export const LeafPrinter = { return '?'; } } - case 'string': { + case 'keyword': { return String(node.value); } - case 'decimal': { + case 'double': { const isRounded = node.value % 1 === 0; if (isRounded) { diff --git a/packages/kbn-esql-ast/src/types.ts b/packages/kbn-esql-ast/src/types.ts index 1bac6e0cff5b3..0df75ee2e8f24 100644 --- a/packages/kbn-esql-ast/src/types.ts +++ b/packages/kbn-esql-ast/src/types.ts @@ -193,10 +193,33 @@ export type BinaryExpressionAssignmentOperator = '='; export type BinaryExpressionComparisonOperator = '==' | '=~' | '!=' | '<' | '<=' | '>' | '>='; export type BinaryExpressionRegexOperator = 'like' | 'not_like' | 'rlike' | 'not_rlike'; +// from https://github.com/elastic/elasticsearch/blob/122e7288200ee03e9087c98dff6cebbc94e774aa/docs/reference/esql/functions/kibana/inline_cast.json +export type InlineCastingType = + | 'bool' + | 'boolean' + | 'cartesian_point' + | 'cartesian_shape' + | 'date_nanos' + | 'date_period' + | 'datetime' + | 'double' + | 'geo_point' + | 'geo_shape' + | 'int' + | 'integer' + | 'ip' + | 'keyword' + | 'long' + | 'string' + | 'text' + | 'time_duration' + | 'unsigned_long' + | 'version'; + export interface ESQLInlineCast extends ESQLAstBaseItem { type: 'inlineCast'; value: ValueType; - castType: string; + castType: InlineCastingType; } /** @@ -270,7 +293,7 @@ export interface ESQLList extends ESQLAstBaseItem { values: ESQLLiteral[]; } -export type ESQLNumericLiteralType = 'decimal' | 'integer'; +export type ESQLNumericLiteralType = 'double' | 'integer'; export type ESQLLiteral = | ESQLDecimalLiteral @@ -290,7 +313,7 @@ export interface ESQLNumericLiteral extends ES } // We cast anything as decimal (e.g. 32.12) as generic decimal numeric type here // @internal -export type ESQLDecimalLiteral = ESQLNumericLiteral<'decimal'>; +export type ESQLDecimalLiteral = ESQLNumericLiteral<'double'>; // @internal export type ESQLIntegerLiteral = ESQLNumericLiteral<'integer'>; @@ -312,7 +335,7 @@ export interface ESQLNullLiteral extends ESQLAstBaseItem { // @internal export interface ESQLStringLiteral extends ESQLAstBaseItem { type: 'literal'; - literalType: 'string'; + literalType: 'keyword'; value: string; } diff --git a/packages/kbn-esql-ast/src/visitor/contexts.ts b/packages/kbn-esql-ast/src/visitor/contexts.ts index 4b4f04fdca4bb..086a217d8f117 100644 --- a/packages/kbn-esql-ast/src/visitor/contexts.ts +++ b/packages/kbn-esql-ast/src/visitor/contexts.ts @@ -337,7 +337,7 @@ export class LimitCommandVisitorContext< if ( arg && arg.type === 'literal' && - (arg.literalType === 'integer' || arg.literalType === 'decimal') + (arg.literalType === 'integer' || arg.literalType === 'double') ) { return arg; } diff --git a/packages/kbn-esql-ast/src/walker/walker.test.ts b/packages/kbn-esql-ast/src/walker/walker.test.ts index 9900f586dc4a0..8dd40b1a87bd1 100644 --- a/packages/kbn-esql-ast/src/walker/walker.test.ts +++ b/packages/kbn-esql-ast/src/walker/walker.test.ts @@ -342,7 +342,7 @@ describe('structurally can walk all nodes', () => { }, { type: 'literal', - literalType: 'string', + literalType: 'keyword', name: '"foo"', }, { @@ -375,7 +375,7 @@ describe('structurally can walk all nodes', () => { }, { type: 'literal', - literalType: 'string', + literalType: 'keyword', name: '"2"', }, { @@ -390,7 +390,7 @@ describe('structurally can walk all nodes', () => { }, { type: 'literal', - literalType: 'decimal', + literalType: 'double', name: '3.14', }, ]); @@ -473,7 +473,7 @@ describe('structurally can walk all nodes', () => { values: [ { type: 'literal', - literalType: 'decimal', + literalType: 'double', name: '3.3', }, ], @@ -492,7 +492,7 @@ describe('structurally can walk all nodes', () => { }, { type: 'literal', - literalType: 'decimal', + literalType: 'double', name: '3.3', }, ]); @@ -600,27 +600,27 @@ describe('structurally can walk all nodes', () => { expect(literals).toMatchObject([ { type: 'literal', - literalType: 'string', + literalType: 'keyword', name: '"a"', }, { type: 'literal', - literalType: 'string', + literalType: 'keyword', name: '"b"', }, { type: 'literal', - literalType: 'string', + literalType: 'keyword', name: '"c"', }, { type: 'literal', - literalType: 'string', + literalType: 'keyword', name: '"d"', }, { type: 'literal', - literalType: 'string', + literalType: 'keyword', name: '"e"', }, ]); diff --git a/packages/kbn-esql-validation-autocomplete/scripts/generate_function_definitions.ts b/packages/kbn-esql-validation-autocomplete/scripts/generate_function_definitions.ts index 13f0b2c66ce32..fe2a85456aa12 100644 --- a/packages/kbn-esql-validation-autocomplete/scripts/generate_function_definitions.ts +++ b/packages/kbn-esql-validation-autocomplete/scripts/generate_function_definitions.ts @@ -52,7 +52,7 @@ const extraFunctions: FunctionDefinition[] = [ { name: 'value', type: 'any' }, ], minParams: 2, - returnType: 'any', + returnType: 'unknown', }, ], examples: [ diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts index 6b2f5fea0cc8d..b3884f5cb96be 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts @@ -259,7 +259,6 @@ describe('autocomplete.suggest', () => { ...getFieldNamesByType('integer'), ...getFieldNamesByType('double'), ...getFieldNamesByType('long'), - '`avg(b)`', ...getFunctionSignaturesByReturnType('eval', ['integer', 'double', 'long'], { scalar: true, }), @@ -284,11 +283,19 @@ describe('autocomplete.suggest', () => { const { assertSuggestions } = await setup(); await assertSuggestions('from a | stats avg(b) by doubleField % 2 /', [',', '| ']); + await assertSuggestions('from a | stats avg(b) by doubleField % 2 /', [',', '| '], { + triggerCharacter: ' ', + }); await assertSuggestions( 'from a | stats var0 = AVG(doubleField) BY var1 = BUCKET(dateField, 1 day)/', [',', '| ', '+ $0', '- $0'] ); + await assertSuggestions( + 'from a | stats var0 = AVG(doubleField) BY var1 = BUCKET(dateField, 1 day) /', + [',', '| ', '+ $0', '- $0'], + { triggerCharacter: ' ' } + ); }); test('on space within bucket()', async () => { const { assertSuggestions } = await setup(); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index cf41fd2506c72..cbc84232b8eb6 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1780,10 +1780,15 @@ async function getOptionArgsSuggestions( innerText, command, option, - { type: argDef?.type || 'any' }, + { type: argDef?.type || 'unknown' }, nodeArg, nodeArgType as string, - references, + { + fields: references.fields, + // you can't use a variable defined + // in the stats command in the by clause + variables: new Map(), + }, getFieldsByType )) ); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/complete_items.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/complete_items.ts index 8d598eb5f2f11..000c196b49e5e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/complete_items.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/complete_items.ts @@ -65,7 +65,6 @@ export const getBuiltinCompatibleFunctionDefinition = ( ({ name, supportedCommands, supportedOptions, signatures, ignoreAsSuggestion }) => (command === 'where' && commandsToInclude ? commandsToInclude.indexOf(name) > -1 : true) && !ignoreAsSuggestion && - !/not_/.test(name) && (!skipAssign || name !== '=') && (option ? supportedOptions?.includes(option) : supportedCommands.includes(command)) && signatures.some( @@ -79,7 +78,10 @@ export const getBuiltinCompatibleFunctionDefinition = ( return compatibleFunctions .filter((mathDefinition) => mathDefinition.signatures.some( - (signature) => returnTypes[0] === 'any' || returnTypes.includes(signature.returnType) + (signature) => + returnTypes[0] === 'unknown' || + returnTypes[0] === 'any' || + returnTypes.includes(signature.returnType) ) ) .map(getSuggestionBuiltinDefinition); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index 41f6a92dc313d..dd450e28b66a9 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -52,23 +52,6 @@ export function getFunctionsToIgnoreForStats(command: ESQLCommand, argIndex: num return isFunctionItem(arg) ? getFnContent(arg) : []; } -/** - * Given a function signature, returns the parameter at the given position. - * - * Takes into account variadic functions (minParams), returning the last - * parameter if the position is greater than the number of parameters. - * - * @param signature - * @param position - * @returns - */ -export function getParamAtPosition( - { params, minParams }: FunctionDefinition['signatures'][number], - position: number -) { - return params.length > position ? params[position] : minParams ? params[params.length - 1] : null; -} - /** * Given a function signature, returns the parameter at the given position, even if it's undefined or null * diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts index c59daa2130417..e71ed32e4c79d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts @@ -423,6 +423,20 @@ const likeFunctions: FunctionDefinition[] = [ ], returnType: 'boolean', }, + { + params: [ + { name: 'left', type: 'text' as const }, + { name: 'right', type: 'keyword' as const }, + ], + returnType: 'boolean', + }, + { + params: [ + { name: 'left', type: 'keyword' as const }, + { name: 'right', type: 'text' as const }, + ], + returnType: 'boolean', + }, { params: [ { name: 'left', type: 'keyword' as const }, @@ -609,25 +623,12 @@ const otherDefinitions: FunctionDefinition[] = [ { name: 'left', type: 'any' }, { name: 'right', type: 'any' }, ], - returnType: 'void', - }, - ], - }, - { - name: 'functions', - type: 'builtin', - description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.functionsDoc', { - defaultMessage: 'Show ES|QL avaialble functions with signatures', - }), - supportedCommands: ['meta'], - signatures: [ - { - params: [], - returnType: 'void', + returnType: 'unknown', }, ], }, { + // TODO — this shouldn't be a function or an operator... name: 'info', type: 'builtin', description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.infoDoc', { @@ -637,21 +638,7 @@ const otherDefinitions: FunctionDefinition[] = [ signatures: [ { params: [], - returnType: 'void', - }, - ], - }, - { - name: 'order-expression', - type: 'builtin', - description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.infoDoc', { - defaultMessage: 'Specify column sorting modifiers', - }), - supportedCommands: ['sort'], - signatures: [ - { - params: [{ name: 'column', type: 'any' }], - returnType: 'void', + returnType: 'unknown', // meaningless }, ], }, diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/generated/scalar_functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/generated/scalar_functions.ts index b25d3ad8b6563..ea5f8f86e1909 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/generated/scalar_functions.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/generated/scalar_functions.ts @@ -9033,7 +9033,7 @@ const caseDefinition: FunctionDefinition = { }, ], minParams: 2, - returnType: 'any', + returnType: 'unknown', }, ], supportedCommands: ['stats', 'inlinestats', 'metrics', 'eval', 'where', 'row', 'sort'], diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/options.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/options.ts index 2e6fbc791b747..31d443a8cbb2b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/options.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/options.ts @@ -129,7 +129,7 @@ export const appendSeparatorOption: CommandOptionsDefinition = { const [firstArg] = option.args; if ( !Array.isArray(firstArg) && - (!isLiteralItem(firstArg) || firstArg.literalType !== 'string') + (!isLiteralItem(firstArg) || firstArg.literalType !== 'keyword') ) { const value = 'value' in firstArg && !isInlineCastItem(firstArg) ? firstArg.value : firstArg.name; diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts index 9ce286796c971..dee08766745df 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts @@ -100,12 +100,14 @@ export const isParameterType = (str: string | undefined): str is FunctionParamet /** * This is the return type of a function definition. + * + * TODO: remove `any` */ -export type FunctionReturnType = Exclude | 'any' | 'void'; +export type FunctionReturnType = Exclude | 'unknown' | 'any'; export const isReturnType = (str: string | FunctionParameterType): str is FunctionReturnType => str !== 'unsupported' && - (dataTypes.includes(str as SupportedDataType) || str === 'any' || str === 'void'); + (dataTypes.includes(str as SupportedDataType) || str === 'unknown' || str === 'any'); export interface FunctionDefinition { type: 'builtin' | 'agg' | 'eval'; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/context.ts b/packages/kbn-esql-validation-autocomplete/src/shared/context.ts index 0f7f830c1417a..1c2e9075e95ff 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/context.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/context.ts @@ -153,7 +153,7 @@ function isBuiltinFunction(node: ESQLFunction) { export function getAstContext(queryString: string, ast: ESQLAst, offset: number) { const { command, option, setting, node } = findAstPosition(ast, offset); if (node) { - if (node.type === 'literal' && node.literalType === 'string') { + if (node.type === 'literal' && node.literalType === 'keyword') { // command ... "" return { type: 'value' as const, command, node, option, setting }; } 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 66f985505a43d..dbf45437dce92 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { ESQLDecimalLiteral, ESQLLiteral, ESQLNumericLiteralType } from '@kbn/esql-ast/src/types'; +import { ESQLLiteral, ESQLNumericLiteralType } from '@kbn/esql-ast/src/types'; import { FunctionParameterType } from '../definitions/types'; export const ESQL_COMMON_NUMERIC_TYPES = ['double', 'long', 'integer'] as const; @@ -27,15 +27,6 @@ export const ESQL_NUMBER_TYPES = [ export const ESQL_STRING_TYPES = ['keyword', 'text'] as const; export const ESQL_DATE_TYPES = ['datetime', 'date_period'] as const; -/** - * - * @param type - * @returns - */ -export function isStringType(type: unknown) { - return typeof type === 'string' && ['keyword', 'text'].includes(type); -} - export function isNumericType(type: unknown): type is ESQLNumericLiteralType { return ( typeof type === 'string' && @@ -43,37 +34,18 @@ export function isNumericType(type: unknown): type is ESQLNumericLiteralType { ); } -export function isNumericDecimalType(type: unknown): type is ESQLDecimalLiteral { - return ( - typeof type === 'string' && - ESQL_NUMERIC_DECIMAL_TYPES.includes(type as (typeof ESQL_NUMERIC_DECIMAL_TYPES)[number]) - ); -} - /** * Compares two types, taking into account literal types * @TODO strengthen typing here (remove `string`) + * @TODO — clean up time duration and date period */ export const compareTypesWithLiterals = ( - a: ESQLLiteral['literalType'] | FunctionParameterType | string, - b: ESQLLiteral['literalType'] | FunctionParameterType | string + a: ESQLLiteral['literalType'] | FunctionParameterType | 'timeInterval' | string, + b: ESQLLiteral['literalType'] | FunctionParameterType | 'timeInterval' | string ) => { if (a === b) { return true; } - if (a === 'decimal') { - return isNumericDecimalType(b); - } - if (b === 'decimal') { - return isNumericDecimalType(a); - } - if (a === 'string') { - return isStringType(b); - } - if (b === 'string') { - return isStringType(a); - } - // In Elasticsearch function definitions, time_literal and time_duration are used // time_duration is seconds/min/hour interval // date_period is day/week/month/year interval diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.test.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.test.ts index 98d2da8d78cc0..0078e0fac119c 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.test.ts @@ -7,7 +7,10 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { shouldBeQuotedSource } from './helpers'; +import { parse } from '@kbn/esql-ast'; +import { getExpressionType, shouldBeQuotedSource } from './helpers'; +import { SupportedDataType } from '../definitions/types'; +import { setTestFunctions } from './test_functions'; describe('shouldBeQuotedSource', () => { it('does not have to be quoted for sources with acceptable characters @-+$', () => { @@ -47,3 +50,295 @@ describe('shouldBeQuotedSource', () => { expect(shouldBeQuotedSource('index-[dd-mm]')).toBe(true); }); }); + +describe('getExpressionType', () => { + const getASTForExpression = (expression: string) => { + const { root } = parse(`FROM index | EVAL ${expression}`); + return root.commands[1].args[0]; + }; + + describe('literal expressions', () => { + const cases: Array<{ expression: string; expectedType: SupportedDataType }> = [ + { + expression: '1.0', + expectedType: 'double', + }, + { + expression: '1', + expectedType: 'integer', + }, + { + expression: 'true', + expectedType: 'boolean', + }, + { + expression: '"foobar"', + expectedType: 'keyword', + }, + { + expression: 'NULL', + expectedType: 'null', + }, + // TODO — consider whether we need to be worried about + // differentiating between time_duration, and date_period + // instead of just using time_literal + { + expression: '1 second', + expectedType: 'time_literal', + }, + { + expression: '1 day', + expectedType: 'time_literal', + }, + ]; + test.each(cases)('detects a literal of type $expectedType', ({ expression, expectedType }) => { + const ast = getASTForExpression(expression); + expect(getExpressionType(ast)).toBe(expectedType); + }); + }); + + describe('inline casting', () => { + const cases: Array<{ expression: string; expectedType: SupportedDataType }> = [ + { expectedType: 'boolean', expression: '"true"::bool' }, + { expectedType: 'boolean', expression: '"false"::boolean' }, + { expectedType: 'boolean', expression: '"false"::BooLEAN' }, + { expectedType: 'cartesian_point', expression: '""::cartesian_point' }, + { expectedType: 'cartesian_shape', expression: '""::cartesian_shape' }, + { expectedType: 'date_nanos', expression: '1::date_nanos' }, + { expectedType: 'date_period', expression: '1::date_period' }, + { expectedType: 'date', expression: '1::datetime' }, + { expectedType: 'double', expression: '1::double' }, + { expectedType: 'geo_point', expression: '""::geo_point' }, + { expectedType: 'geo_shape', expression: '""::geo_shape' }, + { expectedType: 'integer', expression: '1.2::int' }, + { expectedType: 'integer', expression: '1.2::integer' }, + { expectedType: 'ip', expression: '"123.12.12.2"::ip' }, + { expectedType: 'keyword', expression: '1::keyword' }, + { expectedType: 'long', expression: '1::long' }, + { expectedType: 'keyword', expression: '1::string' }, + { expectedType: 'keyword', expression: '1::text' }, + { expectedType: 'time_duration', expression: '1::time_duration' }, + { expectedType: 'unsigned_long', expression: '1::unsigned_long' }, + { expectedType: 'version', expression: '"1.2.3"::version' }, + { expectedType: 'version', expression: '"1.2.3"::VERSION' }, + ]; + test.each(cases)( + 'detects a casted literal of type $expectedType ($expression)', + ({ expression, expectedType }) => { + const ast = getASTForExpression(expression); + expect(getExpressionType(ast)).toBe(expectedType); + } + ); + }); + + describe('fields and variables', () => { + it('detects the type of fields and variables which exist', () => { + expect( + getExpressionType( + getASTForExpression('fieldName'), + new Map([ + [ + 'fieldName', + { + name: 'fieldName', + type: 'geo_shape', + }, + ], + ]), + new Map() + ) + ).toBe('geo_shape'); + + expect( + getExpressionType( + getASTForExpression('var0'), + new Map(), + new Map([ + [ + 'var0', + [ + { + name: 'var0', + type: 'long', + location: { min: 0, max: 0 }, + }, + ], + ], + ]) + ) + ).toBe('long'); + }); + + it('handles fields and variables which do not exist', () => { + expect(getExpressionType(getASTForExpression('fieldName'), new Map(), new Map())).toBe( + 'unknown' + ); + }); + }); + + describe('functions', () => { + beforeAll(() => { + setTestFunctions([ + { + type: 'eval', + name: 'test', + description: 'Test function', + supportedCommands: ['eval'], + signatures: [ + { params: [{ name: 'arg', type: 'keyword' }], returnType: 'keyword' }, + { params: [{ name: 'arg', type: 'double' }], returnType: 'double' }, + { + params: [ + { name: 'arg', type: 'double' }, + { name: 'arg', type: 'keyword' }, + ], + returnType: 'long', + }, + ], + }, + { + type: 'eval', + name: 'returns_keyword', + description: 'Test function', + supportedCommands: ['eval'], + signatures: [{ params: [], returnType: 'keyword' }], + }, + { + type: 'eval', + name: 'accepts_dates', + description: 'Test function', + supportedCommands: ['eval'], + signatures: [ + { + params: [ + { name: 'arg1', type: 'date' }, + { name: 'arg2', type: 'date_period' }, + ], + returnType: 'keyword', + }, + ], + }, + ]); + }); + afterAll(() => { + setTestFunctions([]); + }); + + it('detects the return type of a function', () => { + expect( + getExpressionType(getASTForExpression('returns_keyword()'), new Map(), new Map()) + ).toBe('keyword'); + }); + + it('selects the correct signature based on the arguments', () => { + expect(getExpressionType(getASTForExpression('test("foo")'), new Map(), new Map())).toBe( + 'keyword' + ); + expect(getExpressionType(getASTForExpression('test(1.)'), new Map(), new Map())).toBe( + 'double' + ); + expect(getExpressionType(getASTForExpression('test(1., "foo")'), new Map(), new Map())).toBe( + 'long' + ); + }); + + it('supports nested functions', () => { + expect( + getExpressionType( + getASTForExpression('test(1., test(test(test(returns_keyword()))))'), + new Map(), + new Map() + ) + ).toBe('long'); + }); + + it('supports functions with casted results', () => { + expect( + getExpressionType(getASTForExpression('test(1.)::keyword'), new Map(), new Map()) + ).toBe('keyword'); + }); + + it('handles nulls and string-date casting', () => { + expect(getExpressionType(getASTForExpression('test(NULL)'), new Map(), new Map())).toBe( + 'null' + ); + expect(getExpressionType(getASTForExpression('test(NULL, NULL)'), new Map(), new Map())).toBe( + 'null' + ); + expect( + getExpressionType(getASTForExpression('accepts_dates("", "")'), new Map(), new Map()) + ).toBe('keyword'); + }); + + it('deals with functions that do not exist', () => { + expect(getExpressionType(getASTForExpression('does_not_exist()'), new Map(), new Map())).toBe( + 'unknown' + ); + }); + + it('deals with bad function invocations', () => { + expect( + getExpressionType(getASTForExpression('test(1., "foo", "bar")'), new Map(), new Map()) + ).toBe('unknown'); + + expect(getExpressionType(getASTForExpression('test()'), new Map(), new Map())).toBe( + 'unknown' + ); + + expect(getExpressionType(getASTForExpression('test("foo", 1.)'), new Map(), new Map())).toBe( + 'unknown' + ); + }); + + it('deals with the CASE function', () => { + expect(getExpressionType(getASTForExpression('CASE(true, 1, 2)'), new Map(), new Map())).toBe( + 'integer' + ); + + expect( + getExpressionType(getASTForExpression('CASE(true, 1., true, 1., 2.)'), new Map(), new Map()) + ).toBe('double'); + + expect( + getExpressionType( + getASTForExpression('CASE(true, "", true, "", keywordField)'), + new Map([[`keywordField`, { name: 'keywordField', type: 'keyword' }]]), + new Map() + ) + ).toBe('keyword'); + }); + }); + + describe('lists', () => { + const cases: Array<{ expression: string; expectedType: SupportedDataType | 'unknown' }> = [ + { + expression: '["foo", "bar"]', + expectedType: 'keyword', + }, + { + expression: '[1, 2]', + expectedType: 'integer', + }, + { + expression: '[1., 2.]', + expectedType: 'double', + }, + { + expression: '[null, null, null]', + expectedType: 'null', + }, + { + expression: '[true, false]', + expectedType: 'boolean', + }, + ]; + + test.each(cases)( + 'reports the type of $expression as $expectedType', + ({ expression, expectedType }) => { + const ast = getASTForExpression(expression); + expect(getExpressionType(ast)).toBe(expectedType); + } + ); + }); +}); diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index e3e3da4277344..31c2c01a11404 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -43,10 +43,10 @@ import { FunctionParameterType, FunctionReturnType, ArrayType, + SupportedDataType, } from '../definitions/types'; import type { ESQLRealField, ESQLVariable, ReferenceMaps } from '../validation/types'; import { removeMarkerArgFromArgsList } from './context'; -import { compareTypesWithLiterals, isNumericDecimalType } from './esql_types'; import type { ReasonTypes } from './types'; import { DOUBLE_TICKS_REGEX, EDITOR_MARKER, SINGLE_BACKTICK } from './constants'; import type { EditorContext } from '../autocomplete/types'; @@ -225,27 +225,29 @@ export function getCommandOption(optionName: CommandOptionsDefinition['name']) { } function doesLiteralMatchParameterType(argType: FunctionParameterType, item: ESQLLiteral) { - if (item.literalType === 'null') { + if (item.literalType === argType) { return true; } - if (item.literalType === 'decimal' && isNumericDecimalType(argType)) { + if (item.literalType === 'null') { + // all parameters accept null, but this is not yet reflected + // in our function definitions so we let it through here return true; } - if (item.literalType === 'string' && (argType === 'text' || argType === 'keyword')) { + // some parameters accept string literals because of ES auto-casting + if ( + item.literalType === 'keyword' && + (argType === 'date' || + argType === 'date_period' || + argType === 'version' || + argType === 'ip' || + argType === 'boolean') + ) { return true; } - if (item.literalType !== 'string') { - if (argType === item.literalType) { - return true; - } - return false; - } - - // date-type parameters accept string literals because of ES auto-casting - return ['string', 'date', 'date_period'].includes(argType); + return false; } /** @@ -417,7 +419,7 @@ export function inKnownTimeInterval(item: ESQLTimeInterval): boolean { */ export function isValidLiteralOption(arg: ESQLLiteral, argDef: FunctionParameter) { return ( - arg.literalType === 'string' && + arg.literalType === 'keyword' && argDef.acceptedValues && !argDef.acceptedValues .map((option) => option.toLowerCase()) @@ -447,7 +449,7 @@ export function checkFunctionArgMatchesDefinition( if (isSupportedFunction(arg.name, parentCommand).supported) { const fnDef = buildFunctionLookup().get(arg.name)!; return fnDef.signatures.some( - (signature) => signature.returnType === 'any' || argType === signature.returnType + (signature) => signature.returnType === 'unknown' || argType === signature.returnType ); } } @@ -460,23 +462,15 @@ export function checkFunctionArgMatchesDefinition( if (!validHit) { return false; } - const wrappedTypes = Array.isArray(validHit.type) ? validHit.type : [validHit.type]; - // if final type is of type any make it pass for now - return wrappedTypes.some( - (ct) => - ['any', 'null'].includes(ct) || - argType === ct || - (ct === 'string' && ['text', 'keyword'].includes(argType as string)) - ); + const wrappedTypes: Array<(typeof validHit)['type']> = Array.isArray(validHit.type) + ? validHit.type + : [validHit.type]; + return wrappedTypes.some((ct) => ct === argType || ct === 'null' || ct === 'unknown'); } if (arg.type === 'inlineCast') { const lowerArgType = argType?.toLowerCase(); - const lowerArgCastType = arg.castType?.toLowerCase(); - return ( - compareTypesWithLiterals(lowerArgCastType, lowerArgType) || - // for valid shorthand casts like 321.12::int or "false"::bool - (['int', 'bool'].includes(lowerArgCastType) && argType.startsWith(lowerArgCastType)) - ); + const castedType = getExpressionType(arg); + return castedType === lowerArgType; } } @@ -725,3 +719,143 @@ export function correctQuerySyntax(_query: string, context: EditorContext) { return query; } + +/** + * Gets the signatures of a function that match the number of arguments + * provided in the AST. + */ +export function getSignaturesWithMatchingArity( + fnDef: FunctionDefinition, + astFunction: ESQLFunction +) { + return fnDef.signatures.filter((def) => { + if (def.minParams) { + return astFunction.args.length >= def.minParams; + } + return ( + astFunction.args.length >= def.params.filter(({ optional }) => !optional).length && + astFunction.args.length <= def.params.length + ); + }); +} + +/** + * Given a function signature, returns the parameter at the given position. + * + * Takes into account variadic functions (minParams), returning the last + * parameter if the position is greater than the number of parameters. + * + * @param signature + * @param position + * @returns + */ +export function getParamAtPosition( + { params, minParams }: FunctionDefinition['signatures'][number], + position: number +) { + return params.length > position ? params[position] : minParams ? params[params.length - 1] : null; +} + +/** + * Determines the type of the expression + */ +export function getExpressionType( + root: ESQLAstItem, + fields?: Map, + variables?: Map +): SupportedDataType | 'unknown' { + if (!isSingleItem(root)) { + if (root.length === 0) { + return 'unknown'; + } + return getExpressionType(root[0], fields, variables); + } + + if (isLiteralItem(root) && root.literalType !== 'param') { + return root.literalType; + } + + if (isTimeIntervalItem(root)) { + return 'time_literal'; + } + + // from https://github.com/elastic/elasticsearch/blob/122e7288200ee03e9087c98dff6cebbc94e774aa/docs/reference/esql/functions/kibana/inline_cast.json + if (isInlineCastItem(root)) { + switch (root.castType) { + case 'int': + return 'integer'; + case 'bool': + return 'boolean'; + case 'string': + return 'keyword'; + case 'text': + return 'keyword'; + case 'datetime': + return 'date'; + default: + return root.castType; + } + } + + if (isColumnItem(root) && fields && variables) { + const column = getColumnForASTNode(root, { fields, variables }); + if (!column) { + return 'unknown'; + } + return column.type; + } + + if (root.type === 'list') { + return getExpressionType(root.values[0], fields, variables); + } + + if (isFunctionItem(root)) { + const fnDefinition = getFunctionDefinition(root.name); + if (!fnDefinition) { + return 'unknown'; + } + + if (fnDefinition.name === 'case' && root.args.length) { + // The CASE function doesn't fit our system of function definitions + // and needs special handling. This is imperfect, but it's a start because + // at least we know that the final argument to case will never be a conditional + // expression, always a result expression. + // + // One problem with this is that if a false case is not provided, the return type + // will be null, which we aren't detecting. But this is ok because we consider + // variables and fields to be nullable anyways and account for that during validation. + return getExpressionType(root.args[root.args.length - 1], fields, variables); + } + + const signaturesWithCorrectArity = getSignaturesWithMatchingArity(fnDefinition, root); + + if (!signaturesWithCorrectArity.length) { + return 'unknown'; + } + + const argTypes = root.args.map((arg) => getExpressionType(arg, fields, variables)); + + // When functions are passed null for any argument, they generally return null + // This is a special case that is not reflected in our function definitions + if (argTypes.some((argType) => argType === 'null')) return 'null'; + + const matchingSignature = signaturesWithCorrectArity.find((signature) => { + return argTypes.every((argType, i) => { + const param = getParamAtPosition(signature, i); + return ( + param && + (param.type === argType || + (argType === 'keyword' && ['date', 'date_period'].includes(param.type))) + ); + }); + }); + + if (!matchingSignature) { + return 'unknown'; + } + + return matchingSignature.returnType === 'any' ? 'unknown' : matchingSignature.returnType; + } + + return 'unknown'; +} diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts index 29656d2f581ed..c2de407264a99 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/variables.ts @@ -11,7 +11,7 @@ import type { ESQLAst, ESQLAstItem, ESQLCommand, ESQLFunction } from '@kbn/esql- import { Visitor } from '@kbn/esql-ast/src/visitor'; import type { ESQLVariable, ESQLRealField } from '../validation/types'; import { EDITOR_MARKER } from './constants'; -import { isColumnItem, isFunctionItem, getFunctionDefinition } from './helpers'; +import { isColumnItem, isFunctionItem, getExpressionType } from './helpers'; function addToVariableOccurrences(variables: Map, instance: ESQLVariable) { if (!variables.has(instance.name)) { @@ -43,62 +43,6 @@ 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. See https://github.com/elastic/kibana/issues/195682 - */ -function getExpressionType( - root: ESQLAstItem, - fields: Map, - variables: Map -): string { - const fallback = 'double'; - - if (Array.isArray(root) || !root) { - return fallback; - } - if (root.type === 'literal') { - return root.literalType; - } - if (root.type === 'inlineCast') { - if (root.castType === 'int') { - return 'integer'; - } - if (root.castType === 'bool') { - return 'boolean'; - } - return root.castType; - } - if (isColumnItem(root)) { - const field = fields.get(root.parts.join('.')); - if (field) { - return field.type; - } - const variable = variables.get(root.parts.join('.')); - if (variable) { - return variable[0].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); - } -} - export function excludeVariablesFromCurrentCommand( commands: ESQLCommand[], currentCommand: ESQLCommand, @@ -122,14 +66,10 @@ function addVariableFromAssignment( fields: Map ) { if (isColumnItem(assignOperation.args[0])) { - const rightHandSideArgType = getAssignRightHandSideType( - assignOperation.args[1], - fields, - variables - ); + const rightHandSideArgType = getExpressionType(assignOperation.args[1], fields, variables); addToVariableOccurrences(variables, { name: assignOperation.args[0].parts.join('.'), - type: rightHandSideArgType as string /* fallback to number */, + type: rightHandSideArgType /* fallback to number */, location: assignOperation.args[0].location, }); } @@ -138,14 +78,15 @@ function addVariableFromAssignment( function addVariableFromExpression( expressionOperation: ESQLFunction, queryString: string, - variables: Map + variables: Map, + fields: Map ) { if (!expressionOperation.text.includes(EDITOR_MARKER)) { const expressionText = queryString.substring( expressionOperation.location.min, expressionOperation.location.max + 1 ); - const expressionType = 'double'; // TODO - use getExpressionType once it actually works + const expressionType = getExpressionType(expressionOperation, fields, variables); addToVariableOccurrences(variables, { name: expressionText, type: expressionType, @@ -174,7 +115,7 @@ export function collectVariables( if (ctx.node.name === '=') { addVariableFromAssignment(ctx.node, variables, fields); } else { - addVariableFromExpression(ctx.node, queryString, variables); + addVariableFromExpression(ctx.node, queryString, variables, fields); } }) .on('visitCommandOption', (ctx) => { 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 85a6fadcc8f5c..a3934c1a35627 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 @@ -100,12 +100,12 @@ describe('function validation', () => { // straight call await expectErrors('FROM a_index | EVAL TEST(1.1)', [ - 'Argument of [test] must be [integer], found value [1.1] type [decimal]', + 'Argument of [test] must be [integer], found value [1.1] type [double]', ]); // assignment await expectErrors('FROM a_index | EVAL var = TEST(1.1)', [ - 'Argument of [test] must be [integer], found value [1.1] type [decimal]', + 'Argument of [test] must be [integer], found value [1.1] type [double]', ]); // nested function @@ -115,7 +115,7 @@ describe('function validation', () => { // inline cast await expectErrors('FROM a_index | EVAL TEST(1::DOUBLE)', [ - 'Argument of [test] must be [integer], found value [1::DOUBLE] type [DOUBLE]', + 'Argument of [test] must be [integer], found value [1::DOUBLE] type [double]', ]); // field @@ -125,13 +125,13 @@ describe('function validation', () => { // variables await expectErrors('FROM a_index | EVAL var1 = 1. | EVAL TEST(var1)', [ - 'Argument of [test] must be [integer], found value [var1] type [decimal]', + 'Argument of [test] must be [integer], found value [var1] type [double]', ]); // multiple instances await expectErrors('FROM a_index | EVAL TEST(1.1) | EVAL TEST(1.1)', [ - 'Argument of [test] must be [integer], found value [1.1] type [decimal]', - 'Argument of [test] must be [integer], found value [1.1] type [decimal]', + 'Argument of [test] must be [integer], found value [1.1] type [double]', + 'Argument of [test] must be [integer], found value [1.1] type [double]', ]); }); @@ -190,7 +190,7 @@ describe('function validation', () => { await expectErrors('ROW "a" IN ("a", "b", "c")', []); await expectErrors('ROW "a" IN (1, "b", "c")', [ - 'Argument of [in] must be [keyword[]], found value [(1, "b", "c")] type [(integer, string, string)]', + 'Argument of [in] must be [keyword[]], found value [(1, "b", "c")] type [(integer, keyword, keyword)]', ]); }); }); @@ -238,9 +238,9 @@ describe('function validation', () => { // double, double, double await expectErrors('FROM a_index | EVAL TEST(1., 1., 1.)', []); await expectErrors('FROM a_index | EVAL TEST("", "", "")', [ - 'Argument of [test] must be [double], found value [""] type [string]', - 'Argument of [test] must be [double], found value [""] type [string]', - 'Argument of [test] must be [double], found value [""] type [string]', + 'Argument of [test] must be [double], found value [""] type [keyword]', + 'Argument of [test] must be [double], found value [""] type [keyword]', + 'Argument of [test] must be [double], found value [""] type [keyword]', ]); // int, int @@ -260,7 +260,7 @@ describe('function validation', () => { // date await expectErrors('FROM a_index | EVAL TEST(NOW())', []); await expectErrors('FROM a_index | EVAL TEST(1.)', [ - 'Argument of [test] must be [date], found value [1.] type [decimal]', + 'Argument of [test] must be [date], found value [1.] type [double]', ]); }); }); @@ -721,5 +721,7 @@ describe('function validation', () => { // 'No nested aggregation functions.', // ]); }); + + // @TODO — test function aliases }); }); 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 a646c0323a76f..51ada18f02252 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 @@ -6654,14 +6654,14 @@ { "query": "from a_index | eval 1 * \"1\"", "error": [ - "Argument of [*] must be [double], found value [\"1\"] type [string]" + "Argument of [*] must be [double], found value [\"1\"] type [keyword]" ], "warning": [] }, { "query": "from a_index | eval \"1\" * 1", "error": [ - "Argument of [*] must be [double], found value [\"1\"] type [string]" + "Argument of [*] must be [double], found value [\"1\"] type [keyword]" ], "warning": [] }, @@ -6691,14 +6691,14 @@ { "query": "from a_index | eval 1 / \"1\"", "error": [ - "Argument of [/] must be [double], found value [\"1\"] type [string]" + "Argument of [/] must be [double], found value [\"1\"] type [keyword]" ], "warning": [] }, { "query": "from a_index | eval \"1\" / 1", "error": [ - "Argument of [/] must be [double], found value [\"1\"] type [string]" + "Argument of [/] must be [double], found value [\"1\"] type [keyword]" ], "warning": [] }, @@ -6728,14 +6728,14 @@ { "query": "from a_index | eval 1 % \"1\"", "error": [ - "Argument of [%] must be [double], found value [\"1\"] type [string]" + "Argument of [%] must be [double], found value [\"1\"] type [keyword]" ], "warning": [] }, { "query": "from a_index | eval \"1\" % 1", "error": [ - "Argument of [%] must be [double], found value [\"1\"] type [string]" + "Argument of [%] must be [double], found value [\"1\"] type [keyword]" ], "warning": [] }, @@ -9513,7 +9513,7 @@ "query": "from a_index | eval doubleField = \"5\"", "error": [], "warning": [ - "Column [doubleField] of type double has been overwritten as new type: string" + "Column [doubleField] of type double has been overwritten as new type: keyword" ] }, { @@ -9655,19 +9655,19 @@ "warning": [] }, { - "query": "from a_index | eval true AND \"false\"::boolean", + "query": "from a_index | eval true AND 0::boolean", "error": [], "warning": [] }, { - "query": "from a_index | eval true AND \"false\"::bool", + "query": "from a_index | eval true AND 0::bool", "error": [], "warning": [] }, { - "query": "from a_index | eval true AND \"false\"", + "query": "from a_index | eval true AND 0", "error": [ - "Argument of [and] must be [boolean], found value [\"false\"] type [string]" + "Argument of [and] must be [boolean], found value [0] type [integer]" ], "warning": [] }, diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/types.ts b/packages/kbn-esql-validation-autocomplete/src/validation/types.ts index 99ce0f8ac5196..7aac9f16ad032 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/types.ts @@ -8,12 +8,15 @@ */ import type { ESQLMessage, ESQLLocation } from '@kbn/esql-ast'; -import { FieldType } from '../definitions/types'; +import { FieldType, SupportedDataType } from '../definitions/types'; import type { EditorError } from '../types'; export interface ESQLVariable { name: string; - type: string; + // invalid expressions produce columns of type "unknown" + // also, there are some cases where we can't yet infer the type of + // a valid expression as with `CASE` which can return union types + type: SupportedDataType | 'unknown'; location: ESQLLocation; } 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 dd04f0e506fe8..5e636a941a86c 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts @@ -1129,13 +1129,13 @@ describe('validation logic', () => { `from a_index | eval 1 ${op} "1"`, ['+', '-'].includes(op) ? [`Argument of [${op}] must be [date_period], found value [1] type [integer]`] - : [`Argument of [${op}] must be [double], found value [\"1\"] type [string]`] + : [`Argument of [${op}] must be [double], found value [\"1\"] type [keyword]`] ); testErrorsAndWarnings( `from a_index | eval "1" ${op} 1`, ['+', '-'].includes(op) ? [`Argument of [${op}] must be [date_period], found value [1] type [integer]`] - : [`Argument of [${op}] must be [double], found value [\"1\"] type [string]`] + : [`Argument of [${op}] must be [double], found value [\"1\"] type [keyword]`] ); // TODO: enable when https://github.com/elastic/elasticsearch/issues/108432 is complete // testErrorsAndWarnings(`from a_index | eval "2022" ${op} 1 day`, []); @@ -1478,7 +1478,7 @@ describe('validation logic', () => { testErrorsAndWarnings( 'from a_index | eval doubleField = "5"', [], - ['Column [doubleField] of type double has been overwritten as new type: string'] + ['Column [doubleField] of type double has been overwritten as new type: keyword'] ); }); @@ -1674,11 +1674,11 @@ describe('validation logic', () => { testErrorsAndWarnings('from a_index | eval TRIM(23::text)', []); testErrorsAndWarnings('from a_index | eval TRIM(23::keyword)', []); - testErrorsAndWarnings('from a_index | eval true AND "false"::boolean', []); - testErrorsAndWarnings('from a_index | eval true AND "false"::bool', []); - testErrorsAndWarnings('from a_index | eval true AND "false"', [ + testErrorsAndWarnings('from a_index | eval true AND 0::boolean', []); + testErrorsAndWarnings('from a_index | eval true AND 0::bool', []); + testErrorsAndWarnings('from a_index | eval true AND 0', [ // just a counter-case to make sure the previous tests are meaningful - 'Argument of [and] must be [boolean], found value ["false"] type [string]', + 'Argument of [and] must be [boolean], found value [0] type [integer]', ]); // enforces strings for cartesian_point conversion diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index 2790a8a14a3b1..9605da8460eed 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -26,7 +26,6 @@ import { CommandModeDefinition, CommandOptionsDefinition, FunctionParameter, - FunctionDefinition, } from '../definitions/types'; import { areFieldAndVariableTypesCompatible, @@ -54,6 +53,7 @@ import { isAggFunction, getQuotedColumnName, isInlineCastItem, + getSignaturesWithMatchingArity, } from '../shared/helpers'; import { collectVariables } from '../shared/variables'; import { getMessageFromId, errors } from './errors'; @@ -74,7 +74,7 @@ import { retrieveFieldsFromStringSources, } from './resources'; import { collapseWrongArgumentTypeMessages, getMaxMinNumberOfParams } from './helpers'; -import { getParamAtPosition } from '../autocomplete/helper'; +import { getParamAtPosition } from '../shared/helpers'; import { METADATA_FIELDS } from '../shared/constants'; import { compareTypesWithLiterals } from '../shared/esql_types'; @@ -88,7 +88,7 @@ function validateFunctionLiteralArg( const messages: ESQLMessage[] = []; if (isLiteralItem(actualArg)) { if ( - actualArg.literalType === 'string' && + actualArg.literalType === 'keyword' && argDef.acceptedValues && isValidLiteralOption(actualArg, argDef) ) { @@ -309,21 +309,6 @@ function validateFunctionColumnArg( return messages; } -function extractCompatibleSignaturesForFunction( - fnDef: FunctionDefinition, - astFunction: ESQLFunction -) { - return fnDef.signatures.filter((def) => { - if (def.minParams) { - return astFunction.args.length >= def.minParams; - } - return ( - astFunction.args.length >= def.params.filter(({ optional }) => !optional).length && - astFunction.args.length <= def.params.length - ); - }); -} - function removeInlineCasts(arg: ESQLAstItem): ESQLAstItem { if (isInlineCastItem(arg)) { return removeInlineCasts(arg.value); @@ -376,7 +361,7 @@ function validateFunction( return messages; } } - const matchingSignatures = extractCompatibleSignaturesForFunction(fnDefinition, astFunction); + const matchingSignatures = getSignaturesWithMatchingArity(fnDefinition, astFunction); if (!matchingSignatures.length) { const { max, min } = getMaxMinNumberOfParams(fnDefinition); if (max === min) { diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index 474407581dc8a..b286289aa9367 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -5221,7 +5221,6 @@ "kbn-esql-validation-autocomplete.esql.definition.assignDoc": "Affecter (=)", "kbn-esql-validation-autocomplete.esql.definition.divideDoc": "Diviser (/)", "kbn-esql-validation-autocomplete.esql.definition.equalToDoc": "Égal à", - "kbn-esql-validation-autocomplete.esql.definition.functionsDoc": "Afficher les fonctions ES|QL disponibles avec signatures", "kbn-esql-validation-autocomplete.esql.definition.greaterThanDoc": "Supérieur à", "kbn-esql-validation-autocomplete.esql.definition.greaterThanOrEqualToDoc": "Supérieur ou égal à", "kbn-esql-validation-autocomplete.esql.definition.inDoc": "Teste si la valeur d'une expression est contenue dans une liste d'autres expressions", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 599cec87129fd..af71297358ede 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -5214,7 +5214,6 @@ "kbn-esql-validation-autocomplete.esql.definition.assignDoc": "割り当て（=）", "kbn-esql-validation-autocomplete.esql.definition.divideDoc": "除算（/）", "kbn-esql-validation-autocomplete.esql.definition.equalToDoc": "等しい", - "kbn-esql-validation-autocomplete.esql.definition.functionsDoc": "ES|QLで使用可能な関数と署名を表示", "kbn-esql-validation-autocomplete.esql.definition.greaterThanDoc": "より大きい", "kbn-esql-validation-autocomplete.esql.definition.greaterThanOrEqualToDoc": "よりも大きいまたは等しい", "kbn-esql-validation-autocomplete.esql.definition.inDoc": "ある式が取る値が、他の式のリストに含まれているかどうかをテストします", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 52dcc31d527b8..fdaa8ab629aa0 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -5225,7 +5225,6 @@ "kbn-esql-validation-autocomplete.esql.definition.assignDoc": "分配 (=)", "kbn-esql-validation-autocomplete.esql.definition.divideDoc": "除 (/)", "kbn-esql-validation-autocomplete.esql.definition.equalToDoc": "等于", - "kbn-esql-validation-autocomplete.esql.definition.functionsDoc": "显示带签名的 ES|QL 可用函数", "kbn-esql-validation-autocomplete.esql.definition.greaterThanDoc": "大于", "kbn-esql-validation-autocomplete.esql.definition.greaterThanOrEqualToDoc": "大于或等于", "kbn-esql-validation-autocomplete.esql.definition.inDoc": "测试某表达式接受的值是否包含在其他表达式列表中",