Skip to content

Commit

Permalink
[ES|QL] separate STATS autocomplete routine (elastic#198224)
Browse files Browse the repository at this point in the history 
## Summary

Part of elastic#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]>
  • Loading branch information
@drewdaemon @stratoula
drewdaemon and stratoula authored Nov 1, 2024
1 parent ec391f7 commit 7f2b56f
Show file tree
Hide file tree
Showing 14 changed files with 334 additions and 204 deletions.
55 changes: 36 additions & 19 deletions ...sql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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,/', [
Expand All @@ -210,7 +209,7 @@ describe('autocomplete.suggest', () => {
const { assertSuggestions } = await setup();
const expected = [
'var0 = ',
getAddDateHistogramSnippet(),
getDateHistogramCompletionItem(),
...getFieldNamesByType('any').map((field) => `${field} `),
...allEvaFunctions,
...allGroupingFunctions,
Expand All @@ -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 () => {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)', [
Expand Down Expand Up @@ -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);
});
});
});
});
});
2 changes: 1 addition & 1 deletion packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 7 additions & 7 deletions packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);

Expand Down Expand Up @@ -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} `),
]);
Expand Down Expand Up @@ -1072,8 +1072,8 @@ describe('autocomplete', () => {

// STATS argument BY
testSuggestions('FROM a | STATS AVG(numberField) /', [
',',
attachAsSnippet(attachTriggerCommand('BY $0')),
', ',
attachTriggerCommand('BY '),
attachTriggerCommand('| '),
]);

Expand All @@ -1090,7 +1090,7 @@ describe('autocomplete', () => {
'by'
);
testSuggestions('FROM a | STATS AVG(numberField) BY /', [
getAddDateHistogramSnippet(),
getDateHistogramCompletionItem(),
attachTriggerCommand('var0 = '),
...getFieldNamesByType('any')
.map((field) => `${field} `)
Expand All @@ -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),
Expand Down
Loading

0 comments on commit 7f2b56f

Please sign in to comment.