[ES|QL] Create expression type evaluator (elastic#195989)

## Summary Close elastic#195682 Close elastic#195430 Introduces `getExpressionType`, the ES|QL expression type evaluator to rule them all! Also, fixes several validation bugs related to the faulty logic that existed before with variable type detection (some noted in elastic#192255 (comment)). ### Checklist - [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 --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
kibanamachine · Oct 18, 2024 · 2173af7 · 2173af7
1 parent 09fe7bd
commit 2173af7
Show file tree

Hide file tree

Showing 30 changed files with 609 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<ValueType = ESQLAstItem> 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<T extends ESQLNumericLiteralType> 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
@@ -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
@@ -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
@@ -52,7 +52,7 @@ const extraFunctions: FunctionDefinition[] = [
           { name: 'value', type: 'any' },
         ],
         minParams: 2,
-        returnType: 'any',
+        returnType: 'unknown',
       },
     ],
     examples: [

diff --git a/...sql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts b/...sql-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
@@ -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
@@ -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
@@ -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
  *