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.

Sign up for GitHub

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

Jump to bottom

[ML] Fix autocomplete suggestions for EVAL missing functions #197504

Closed
wants to merge 3 commits into from
Closed

[ML] Fix autocomplete suggestions for EVAL missing functions #197504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fb128d5
Update PR with new behavior
qn895 Oct 23, 2024
dc6539b
Merge remote-tracking branch 'upstream/main' into eval-funcs-suggestions
qn895 Oct 23, 2024
fb9d717
Update types
qn895 Oct 23, 2024
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
88 changes: 49 additions & 39 deletions ...esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,8 @@ describe('autocomplete.suggest', () => {
const { assertSuggestions } = await setup();
await assertSuggestions('from a | eval /', [
'var0 = ',
...getFieldNamesByType('any'),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
]);

await assertSuggestions('from a | eval doubleField/', [
'doubleField, ',
'doubleField | ',
'var0 = ',
...getFieldNamesByType('any', { advanceCursor: true }),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true, advanceCursor: true }),
]);

await assertSuggestions('from a | eval doubleField /', [
Expand Down Expand Up @@ -101,9 +95,9 @@ describe('autocomplete.suggest', () => {
);
await assertSuggestions('from a | eval a=doubleField, /', [
'var0 = ',
...getFieldNamesByType('any'),
'a',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
...getFieldNamesByType('any', { advanceCursor: true }),
'a ',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true, advanceCursor: true }),
]);
await assertSuggestions(
'from a | eval a=round(/)',
Expand Down Expand Up @@ -171,9 +165,9 @@ describe('autocomplete.suggest', () => {
);
await assertSuggestions('from a | eval a=round(doubleField),/', [
'var0 = ',
...getFieldNamesByType('any'),
'a',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
...getFieldNamesByType('any', { advanceCursor: true }),
'a ',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true, advanceCursor: true }),
]);
await assertSuggestions('from a | eval a=round(doubleField) + /', [
...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES),
Expand Down Expand Up @@ -203,8 +197,11 @@ describe('autocomplete.suggest', () => {
'from a | stats avg(doubleField) by keywordField | eval /',
[
'var0 = ',
'`avg(doubleField)`',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
'`avg(doubleField)` ',
...getFunctionSignaturesByReturnType('eval', 'any', {
scalar: true,
advanceCursor: true,
}),
],
{
triggerCharacter: ' ',
Expand All @@ -216,18 +213,24 @@ describe('autocomplete.suggest', () => {
'from a | eval abs(doubleField) + 1 | eval /',
[
'var0 = ',
...getFieldNamesByType('any'),
'`abs(doubleField) + 1`',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
...getFieldNamesByType('any', { advanceCursor: true }),
'`abs(doubleField) + 1` ',
...getFunctionSignaturesByReturnType('eval', 'any', {
scalar: true,
advanceCursor: true,
}),
],
{ triggerCharacter: ' ' }
);
await assertSuggestions(
'from a | stats avg(doubleField) by keywordField | eval /',
[
'var0 = ',
'`avg(doubleField)`',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
'`avg(doubleField)` ',
...getFunctionSignaturesByReturnType('eval', 'any', {
scalar: true,
advanceCursor: true,
}),
],
{
triggerCharacter: ' ',
Expand All @@ -243,9 +246,12 @@ describe('autocomplete.suggest', () => {
'from a | stats avg(doubleField), avg(kubernetes.something.something) by keywordField | eval /',
[
'var0 = ',
'`avg(doubleField)`',
'`avg(kubernetes.something.something)`',
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
'`avg(doubleField)` ',
'`avg(kubernetes.something.something)` ',
...getFunctionSignaturesByReturnType('eval', 'any', {
scalar: true,
advanceCursor: true,
}),
],
{
triggerCharacter: ' ',
Expand Down Expand Up @@ -610,29 +616,33 @@ describe('autocomplete.suggest', () => {
triggerCharacter: ' ',
}
);

const expectedNumericSuggestions = [
// Notice no extra space after field name
...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES).map((field) => `${field}`),
...getFunctionSignaturesByReturnType(
'eval',
ESQL_COMMON_NUMERIC_TYPES,
{ scalar: true },
undefined,
[]
),
];
await assertSuggestions(
'from a | eval case( integerField != /)',
expectedNumericSuggestions,
[
// Notice no extra space after field name
...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES).map((field) => `${field}`),
...getFunctionSignaturesByReturnType(
'eval',
ESQL_COMMON_NUMERIC_TYPES,
{ scalar: true, advanceCursor: false },
undefined,
[]
),
],
{
triggerCharacter: ' ',
}
);
await assertSuggestions('from a | eval case( integerField != /)', [
// Notice no extra space after field name
...getFieldNamesByType('any').map((field) => `${field}`),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }, undefined, []),
...getFieldNamesByType('any').map((field) => `${field} `),
...getFunctionSignaturesByReturnType(
'eval',
'any',
{ scalar: true, advanceCursor: false },
undefined,
[]
),
]);

// case( field > 0, >) suggests fields like normal
Expand Down
11 changes: 7 additions & 4 deletions packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,14 @@ export function getFunctionSignaturesByReturnType(
// skipAssign here is used to communicate to not propose an assignment if it's not possible
// within the current context (the actual logic has it, but here we want a shortcut)
skipAssign,
advanceCursor,
}: {
agg?: boolean;
grouping?: boolean;
scalar?: boolean;
builtin?: boolean;
skipAssign?: boolean;
advanceCursor?: boolean;
} = {},
paramsTypes?: Readonly<FunctionParameterType[]>,
ignored?: string[],
Expand Down Expand Up @@ -209,7 +211,7 @@ export function getFunctionSignaturesByReturnType(
if (type === 'builtin') {
return {
text: signatures.some(({ params }) => params.length > 1)
? `${name.toUpperCase()} $0`
? `${name.toUpperCase()} $0${advanceCursor ? ' ' : ''}`
: name.toUpperCase(),
label: name.toUpperCase(),
};
Expand All @@ -219,19 +221,20 @@ export function getFunctionSignaturesByReturnType(
capitalize: true,
});
return {
text: `${name.toUpperCase()}($0)`,
text: `${name.toUpperCase()}($0)${advanceCursor ? ' ' : ''}`,
label: printedSignatures[0].declaration,
};
});
}

export function getFieldNamesByType(
_requestedType: Readonly<FieldType | 'any' | Array<FieldType | 'any'>>
_requestedType: Readonly<FieldType | 'any' | Array<FieldType | 'any'>>,
opts?: { advanceCursor: boolean }
) {
const requestedType = Array.isArray(_requestedType) ? _requestedType : [_requestedType];
return fields
.filter(({ type }) => requestedType.includes('any') || requestedType.includes(type))
.map(({ name, suggestedAs }) => suggestedAs || name);
.map(({ name, suggestedAs }) => (suggestedAs || name) + (opts?.advanceCursor ? ' ' : ''));
}

export function getLiteralsByType(_type: SupportedDataType | SupportedDataType[]) {
Expand Down
4 changes: 2 additions & 2 deletions ...-validation-autocomplete/src/autocomplete/__tests__/hidden_functions_and_commands.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ describe('hidden functions', () => {

const { suggest } = await setup();
const suggestedFunctions = (await suggest('FROM index | EVAL /')).map((s) => s.text);
expect(suggestedFunctions).toContain('VISIBLE_FUNCTION($0)');
expect(suggestedFunctions).not.toContain('HIDDEN_FUNCTION($0)');
expect(suggestedFunctions).toContain('VISIBLE_FUNCTION($0) ');
expect(suggestedFunctions).not.toContain('HIDDEN_FUNCTION($0) ');
});
it('does not suggest hidden agg functions', async () => {
setTestFunctions([
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,8 @@ describe('autocomplete', () => {
// EVAL argument
testSuggestions('FROM index1 | EVAL b/', [
'var0 = ',
...getFieldNamesByType('any'),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
...getFieldNamesByType('any', { advanceCursor: true }),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true, advanceCursor: true }),
]);

testSuggestions('FROM index1 | EVAL var0 = f/', [
Expand Down
20 changes: 5 additions & 15 deletions packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,17 +596,6 @@ async function getExpressionSuggestionsByType(

const suggestions: SuggestionRawDefinition[] = [];

// When user types and accepts autocomplete suggestion, and cursor is placed at the end of a valid field
// we should not show irrelevant functions that might have words matching
const columnWithActiveCursor = commands.find(
(c) =>
c.name === command.name &&
command.name === 'eval' &&
c.args.some((arg) => isColumnItem(arg) && arg.name.includes(EDITOR_MARKER))
);

const shouldShowFunctions = !columnWithActiveCursor;

// in this flow there's a clear plan here from argument definitions so try to follow it
if (argDef) {
if (argDef.type === 'column' || argDef.type === 'any' || argDef.type === 'function') {
Expand Down Expand Up @@ -660,14 +649,15 @@ async function getExpressionSuggestionsByType(
// COMMAND fie<suggest>
return fieldSuggestions.map((suggestion) => ({
...suggestion,
text: suggestion.text + (['grok', 'dissect'].includes(command.name) ? ' ' : ''),
text:
suggestion.text + (['grok', 'dissect', 'eval'].includes(command.name) ? ' ' : ''),
command: TRIGGER_SUGGESTION_COMMAND,
rangeToReplace,
}));
},
(fragment: string, rangeToReplace: { start: number; end: number }) => {
// COMMAND field<suggest>
if (['grok', 'dissect'].includes(command.name)) {
if (['grok', 'dissect', 'eval'].includes(command.name)) {
return fieldSuggestions.map((suggestion) => ({
...suggestion,
text: suggestion.text + ' ',
Expand Down Expand Up @@ -733,7 +723,7 @@ async function getExpressionSuggestionsByType(
option?.name,
getFieldsByType,
{
functions: shouldShowFunctions,
functions: true,
fields: false,
variables: nodeArg ? undefined : anyVariables,
literals: argDef.constantOnly,
Expand Down Expand Up @@ -1064,7 +1054,7 @@ async function getExpressionSuggestionsByType(
}
// Due to some logic overlapping functions can be repeated
// so dedupe here based on text string (it can differ from name)
return uniqBy(suggestions, (suggestion) => suggestion.text);
return uniqBy(suggestions, (suggestion) => suggestion.text.trim());
}

async function getBuiltinFunctionNextArgument(
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9976,7 +9976,7 @@
{
"query": "from index [METADATA _id, _source2]",
"error": [
"Metadata field [_source2] is not available. Available metadata fields are: [_version, _id, _index, _source, _ignored]"
"Metadata field [_source2] is not available. Available metadata fields are: [_version, _id, _index, _source, _ignored, _index_mode]"
],
"warning": [
"Square brackets '[]' need to be removed from FROM METADATA declaration"
Expand Down Expand Up @@ -10014,7 +10014,7 @@
{
"query": "from index METADATA _id, _source2",
"error": [
"Metadata field [_source2] is not available. Available metadata fields are: [_version, _id, _index, _source, _ignored]"
"Metadata field [_source2] is not available. Available metadata fields are: [_version, _id, _index, _source, _ignored, _index_mode]"
],
"warning": []
},
Expand Down