Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ES|QL] separate STATS autocomplete routine #198224

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variables from previous EVAL commands are no longer suggested (e.g. ...| EVAL foo = 1 | STATS /).

I really think we want this. A user will create an eval variable to use it in stats. Is a very common scenario. Think about date buckets or variables with case.

image
  • If I write the variable on my own now, the suggestions seem wrong
image

All the other decisions seem ok to me 👍

Copy link
Contributor Author

@drewdaemon drewdaemon Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @stratoula

variables from previous EVAL commands are no longer suggested (e.g. ...| EVAL foo = 1 | STATS /).

I really think we want this. A user will create an eval variable to use it in stats. Is a very common scenario. Think about date buckets or variables with case.

Just to make sure I understand—

  • we want only variables (no fields) in STATS /
  • but we want variables and fields in STATS … BY /

Is that what you are saying?

If I write the variable on my own now, the suggestions seem wrong

From what I can see, this is no change in behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synced offline, I misunderstood the behavior described. We are good

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);
});
});
});
});
});
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
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