[ES|QL] separate STATS autocomplete routine (#198224)

## Summary Part of #195418 This PR moves the `STATS` completion logic to its own home. There are also a few changes in behavior. I am open for feedback on any of these. - the cursor is automatically advanced after accepting a comma suggestion - variables from previous `EVAL` commands are no longer suggested (e.g. `...| EVAL foo = 1 | STATS /`). I'm not sure about this change, but it seemed potentially unintended to suggest variables but no other columns such as field names. - a new variable is suggested for new expressions in the `BY` clause. Formerly, new variables were only suggested in the `STATS` clause. - `+` and `-` are no longer suggested after a completed function call within an assignment in the `BY` clause (e.g. `... | STATS ... BY var1 = BUCKET(dateField, 1 day) /`. This behavior was encoded in our tests, but it feels unintended to me, especially since it only applied when the result of the function was assigned to a new variable in the `BY` clause. ### 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: Stratoula Kalafateli <[email protected]>
elastic · Nov 1, 2024 · 7f2b56f · 7f2b56f
1 parent ec391f7
commit 7f2b56f
Show file tree

Hide file tree

Showing 14 changed files with 334 additions and 204 deletions.
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
@@ -9,8 +9,8 @@
 
 import { FieldType, FunctionReturnType } from '../../definitions/types';
 import { ESQL_COMMON_NUMERIC_TYPES, ESQL_NUMBER_TYPES } from '../../shared/esql_types';
+import { getDateHistogramCompletionItem } from '../commands/stats/util';
 import { allStarConstant } from '../complete_items';
-import { getAddDateHistogramSnippet } from '../factories';
 import { roundParameterTypes } from './constants';
 import {
   setup,
@@ -71,7 +71,7 @@ describe('autocomplete.suggest', () => {
       test('on space after aggregate field', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | stats a=min(b) /', ['BY $0', ',', '| ']);
+        await assertSuggestions('from a | stats a=min(b) /', ['BY ', ', ', '| ']);
       });
 
       test('on space after aggregate field with comma', async () => {
@@ -184,17 +184,16 @@ describe('autocomplete.suggest', () => {
       test('when typing right paren', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | stats a = min(b)/ | sort b', ['BY $0', ',', '| ']);
+        await assertSuggestions('from a | stats a = min(b)/ | sort b', ['BY ', ', ', '| ']);
       });
 
       test('increments suggested variable name counter', async () => {
         const { assertSuggestions } = await setup();
 
         await assertSuggestions('from a | eval var0=round(b), var1=round(c) | stats /', [
           'var2 = ',
+          // TODO verify that this change is ok
           ...allAggFunctions,
-          'var0',
-          'var1',
           ...allEvaFunctions,
         ]);
         await assertSuggestions('from a | stats var0=min(b),var1=c,/', [
@@ -210,7 +209,7 @@ describe('autocomplete.suggest', () => {
         const { assertSuggestions } = await setup();
         const expected = [
           'var0 = ',
-          getAddDateHistogramSnippet(),
+          getDateHistogramCompletionItem(),
           ...getFieldNamesByType('any').map((field) => `${field} `),
           ...allEvaFunctions,
           ...allGroupingFunctions,
@@ -224,7 +223,7 @@ describe('autocomplete.suggest', () => {
       test('on space after grouping field', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | stats a=c by d /', [',', '| ']);
+        await assertSuggestions('from a | stats a=c by d /', [', ', '| ']);
       });
 
       test('after comma "," in grouping fields', async () => {
@@ -233,7 +232,7 @@ describe('autocomplete.suggest', () => {
         const fields = getFieldNamesByType('any').map((field) => `${field} `);
         await assertSuggestions('from a | stats a=c by d, /', [
           'var0 = ',
-          getAddDateHistogramSnippet(),
+          getDateHistogramCompletionItem(),
           ...fields,
           ...allEvaFunctions,
           ...allGroupingFunctions,
@@ -245,7 +244,7 @@ describe('autocomplete.suggest', () => {
         ]);
         await assertSuggestions('from a | stats avg(b) by c, /', [
           'var0 = ',
-          getAddDateHistogramSnippet(),
+          getDateHistogramCompletionItem(),
           ...fields,
           ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
           ...allGroupingFunctions,
@@ -262,17 +261,16 @@ describe('autocomplete.suggest', () => {
           ...getFunctionSignaturesByReturnType('eval', ['integer', 'double', 'long'], {
             scalar: true,
           }),
-
           ...allGroupingFunctions,
         ]);
         await assertSuggestions('from a | stats avg(b) by var0 = /', [
-          getAddDateHistogramSnippet(),
+          getDateHistogramCompletionItem(),
           ...getFieldNamesByType('any').map((field) => `${field} `),
           ...allEvaFunctions,
           ...allGroupingFunctions,
         ]);
         await assertSuggestions('from a | stats avg(b) by c, var0 = /', [
-          getAddDateHistogramSnippet(),
+          getDateHistogramCompletionItem(),
           ...getFieldNamesByType('any').map((field) => `${field} `),
           ...allEvaFunctions,
           ...allGroupingFunctions,
@@ -282,21 +280,17 @@ describe('autocomplete.suggest', () => {
       test('on space after expression right hand side operand', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | stats avg(b) by doubleField % 2 /', [',', '| ']);
-        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();
         await assertSuggestions('from a | stats avg(b) by BUCKET(/, 50, ?_tstart, ?_tend)', [
@@ -330,6 +324,29 @@ describe('autocomplete.suggest', () => {
         const suggestions = await suggest('from a | stats count(/)');
         expect(suggestions).toContain(allStarConstant);
       });
+
+      describe('date histogram snippet', () => {
+        test('uses histogramBarTarget preference when available', async () => {
+          const { suggest } = await setup();
+          const histogramBarTarget = Math.random() * 100;
+          const expectedCompletionItem = getDateHistogramCompletionItem(histogramBarTarget);
+
+          const suggestions = await suggest('FROM a | STATS BY /', {
+            callbacks: { getPreferences: () => Promise.resolve({ histogramBarTarget }) },
+          });
+
+          expect(suggestions).toContainEqual(expectedCompletionItem);
+        });
+
+        test('defaults gracefully', async () => {
+          const { suggest } = await setup();
+          const expectedCompletionItem = getDateHistogramCompletionItem();
+
+          const suggestions = await suggest('FROM a | STATS BY /');
+
+          expect(suggestions).toContainEqual(expectedCompletionItem);
+        });
+      });
     });
   });
 });
diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts
@@ -177,7 +177,7 @@ export function getFunctionSignaturesByReturnType(
         ({ returnType }) =>
           expectedReturnType.includes('any') || expectedReturnType.includes(returnType as string)
       );
-      if (!filteredByReturnType.length) {
+      if (!filteredByReturnType.length && !expectedReturnType.includes('any')) {
         return false;
       }
       if (paramsTypes?.length) {

diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
@@ -12,7 +12,6 @@ import { scalarFunctionDefinitions } from '../definitions/generated/scalar_funct
 import { timeUnitsToSuggest } from '../definitions/literals';
 import { commandDefinitions as unmodifiedCommandDefinitions } from '../definitions/commands';
 import {
-  getAddDateHistogramSnippet,
   getDateLiterals,
   getSafeInsertText,
   TIME_SYSTEM_PARAMS,
@@ -38,6 +37,7 @@ import { METADATA_FIELDS } from '../shared/constants';
 import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types';
 import { log10ParameterTypes, powParameterTypes } from './__tests__/constants';
 import { getRecommendedQueries } from './recommended_queries/templates';
+import { getDateHistogramCompletionItem } from './commands/stats/util';
 
 const commandDefinitions = unmodifiedCommandDefinitions.filter(({ hidden }) => !hidden);
 
@@ -737,12 +737,12 @@ describe('autocomplete', () => {
     ]);
 
     // STATS argument BY
-    testSuggestions('FROM index1 | STATS AVG(booleanField) B/', ['BY $0', ',', '| ']);
+    testSuggestions('FROM index1 | STATS AVG(booleanField) B/', ['BY ', ', ', '| ']);
 
     // STATS argument BY expression
     testSuggestions('FROM index1 | STATS field BY f/', [
       'var0 = ',
-      getAddDateHistogramSnippet(),
+      getDateHistogramCompletionItem(),
       ...getFunctionSignaturesByReturnType('stats', 'any', { grouping: true, scalar: true }),
       ...getFieldNamesByType('any').map((field) => `${field} `),
     ]);
@@ -1072,8 +1072,8 @@ describe('autocomplete', () => {
 
     // STATS argument BY
     testSuggestions('FROM a | STATS AVG(numberField) /', [
-      ',',
-      attachAsSnippet(attachTriggerCommand('BY $0')),
+      ', ',
+      attachTriggerCommand('BY '),
       attachTriggerCommand('| '),
     ]);
 
@@ -1090,7 +1090,7 @@ describe('autocomplete', () => {
       'by'
     );
     testSuggestions('FROM a | STATS AVG(numberField) BY /', [
-      getAddDateHistogramSnippet(),
+      getDateHistogramCompletionItem(),
       attachTriggerCommand('var0 = '),
       ...getFieldNamesByType('any')
         .map((field) => `${field} `)
@@ -1100,7 +1100,7 @@ describe('autocomplete', () => {
 
     // STATS argument BY assignment (checking field suggestions)
     testSuggestions('FROM a | STATS AVG(numberField) BY var0 = /', [
-      getAddDateHistogramSnippet(),
+      getDateHistogramCompletionItem(),
       ...getFieldNamesByType('any')
         .map((field) => `${field} `)
         .map(attachTriggerCommand),