[ES|QL] Sorting accepts expressions (#181916)

## Summary `SORT` accepts expressions as of elastic/elasticsearch#107158. This PR updates the validator and autocompletion engine to account for this improvement. ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added — looks like docs haven't been added for this feature yet. I opened the discussion with ES team ([ref](elastic/elasticsearch#107158 (comment))) - [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: Stratoula Kalafateli <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
elastic · May 1, 2024 · 5ba6a39 · 5ba6a39
1 parent a1b990d
commit 5ba6a39
Show file tree

Hide file tree

Showing 7 changed files with 574 additions and 11 deletions.
diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
@@ -559,7 +559,10 @@ describe('autocomplete', () => {
   }
 
   describe('sort', () => {
-    testSuggestions('from a | sort ', getFieldNamesByType('any'));
+    testSuggestions('from a | sort ', [
+      ...getFieldNamesByType('any'),
+      ...getFunctionSignaturesByReturnType('sort', 'any', { evalMath: true }),
+    ]);
     testSuggestions('from a | sort stringField ', ['asc', 'desc', ',', '|']);
     testSuggestions('from a | sort stringField desc ', ['nulls first', 'nulls last', ',', '|']);
     // @TODO: improve here

diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts
@@ -594,7 +594,11 @@ async function getExpressionSuggestionsByType(
             option?.name,
             getFieldsByType,
             {
-              functions: canHaveAssignments,
+              // TODO instead of relying on canHaveAssignments and other command name checks
+              // we should have a more generic way to determine if a command can have functions.
+              // I think it comes down to the definition of 'column' since 'any' should always
+              // include functions.
+              functions: canHaveAssignments || command.name === 'sort',
               fields: !argDef.constantOnly,
               variables: anyVariables,
               literals: argDef.constantOnly,

diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts
@@ -19,7 +19,7 @@ function createMathDefinition(
     type: 'builtin',
     name,
     description,
-    supportedCommands: ['eval', 'where', 'row', 'stats'],
+    supportedCommands: ['eval', 'where', 'row', 'stats', 'sort'],
     supportedOptions: ['by'],
     signatures: types.map((type) => {
       if (Array.isArray(type)) {
@@ -59,7 +59,7 @@ function createComparisonDefinition(
     type: 'builtin' as const,
     name,
     description,
-    supportedCommands: ['eval', 'where', 'row'],
+    supportedCommands: ['eval', 'where', 'row', 'sort'],
     supportedOptions: ['by'],
     validate,
     signatures: [
@@ -296,7 +296,7 @@ export const builtinFunctions: FunctionDefinition[] = [
     ignoreAsSuggestion: /not/.test(name),
     name,
     description,
-    supportedCommands: ['eval', 'where', 'row'],
+    supportedCommands: ['eval', 'where', 'row', 'sort'],
     supportedOptions: ['by'],
     signatures: [
       {
@@ -322,7 +322,7 @@ export const builtinFunctions: FunctionDefinition[] = [
     ignoreAsSuggestion: /not/.test(name),
     name,
     description,
-    supportedCommands: ['eval', 'where', 'row'],
+    supportedCommands: ['eval', 'where', 'row', 'sort'],
     signatures: [
       {
         params: [
@@ -371,7 +371,7 @@ export const builtinFunctions: FunctionDefinition[] = [
     type: 'builtin' as const,
     name,
     description,
-    supportedCommands: ['eval', 'where', 'row'],
+    supportedCommands: ['eval', 'where', 'row', 'sort'],
     supportedOptions: ['by'],
     signatures: [
       {
@@ -410,7 +410,7 @@ export const builtinFunctions: FunctionDefinition[] = [
     description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.notDoc', {
       defaultMessage: 'Not',
     }),
-    supportedCommands: ['eval', 'where', 'row'],
+    supportedCommands: ['eval', 'where', 'row', 'sort'],
     supportedOptions: ['by'],
     signatures: [
       {
@@ -436,7 +436,7 @@ export const builtinFunctions: FunctionDefinition[] = [
     type: 'builtin',
     name,
     description,
-    supportedCommands: ['eval', 'where', 'row'],
+    supportedCommands: ['eval', 'where', 'row', 'sort'],
     signatures: [
       {
         params: [{ name: 'left', type: 'any' }],

diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts
@@ -330,13 +330,14 @@ export const commandDefinitions: CommandDefinition[] = [
       '… | sort a desc, b nulls last, c asc nulls first',
       '… | sort b nulls last',
       '… | sort c asc nulls first',
+      '… | sort a - abs(b)',
     ],
     options: [],
     modes: [],
     signature: {
       multipleParams: true,
       params: [
-        { name: 'column', type: 'column' },
+        { name: 'expression', type: 'any' },
         { name: 'direction', type: 'string', optional: true, values: ['asc', 'desc'] },
         { name: 'nulls', type: 'string', optional: true, values: ['nulls first', 'nulls last'] },
       ],

diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts
@@ -1692,7 +1692,7 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [
   .sort(({ name: a }, { name: b }) => a.localeCompare(b))
   .map((def) => ({
     ...def,
-    supportedCommands: ['stats', 'eval', 'where', 'row'],
+    supportedCommands: ['stats', 'eval', 'where', 'row', 'sort'],
     supportedOptions: ['by'],
     type: 'eval',
   }));