Skip to content

Commit

Permalink
[ES|QL] Auto open suggestions for field lists (#190466)
Browse files Browse the repository at this point in the history 
## Summary

Part of #189662

## Suggests comma and pipe


https://github.com/user-attachments/assets/c09bc6fd-20a6-4f42-a871-c70e68e6d81a

## Doesn't suggest comma when there are no more fields


https://github.com/user-attachments/assets/29fce13e-e58b-4d93-bce5-6b1f913b4d92

## Doesn't work for escaped columns :(


https://github.com/user-attachments/assets/3d65f3b9-923d-4c0e-9c50-51dd83115c8b

As part of this effort I discovered
#191100 and
#191105, as well as a problem
with column name validation (see
#177699)

I think we can revisit column escaping and probably resolve all of these
issues (issue [here](#191111)).


### 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
  • Loading branch information
@drewdaemon
drewdaemon authored Aug 23, 2024
1 parent bb7466e commit 15ef37f
Show file tree
Hide file tree
Showing 14 changed files with 240 additions and 77 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-esql-validation-autocomplete/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export {
getCommandDefinition,
getAllCommands,
getCommandOption,
lookupColumn,
getColumnForASTNode as lookupColumn,
shouldBeQuotedText,
printFunctionSignature,
checkFunctionArgMatchesDefinition as isEqualType,
Expand Down
5 changes: 3 additions & 2 deletions packages/kbn-esql-validation-autocomplete/scripts/generate_function_validation_tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
dataTypes,
fieldTypes,
isFieldType,
FunctionParameter,
} from '../src/definitions/types';
import { FUNCTION_DESCRIBE_BLOCK_NAME } from '../src/validation/function_describe_block_name';
import { getMaxMinNumberOfParams } from '../src/validation/helpers';
Expand Down Expand Up @@ -1155,7 +1156,7 @@ function generateIncorrectlyTypedParameters(
signatures: FunctionDefinition['signatures'],
currentParams: FunctionDefinition['signatures'][number]['params'],
availableFields: Array<{ name: string; type: SupportedDataType }>
) {
): { wrongFieldMapping: FunctionParameter[]; expectedErrors: string[] } {
const literalValues = {
string: `"a"`,
number: '5',
Expand Down Expand Up @@ -1260,7 +1261,7 @@ function generateIncorrectlyTypedParameters(
})
.filter(nonNullable);

return { wrongFieldMapping, expectedErrors };
return { wrongFieldMapping: wrongFieldMapping as FunctionParameter[], expectedErrors };
}

/**
Expand Down
101 changes: 88 additions & 13 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 @@ -20,7 +20,9 @@ import { camelCase, partition } from 'lodash';
import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
import {
FunctionParameter,
FunctionReturnType,
isFieldType,
isReturnType,
isSupportedDataType,
SupportedDataType,
} from '../definitions/types';
Expand Down Expand Up @@ -753,7 +755,9 @@ describe('autocomplete', () => {
);

const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] =>
Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType);
Array.from(new Set(paramDefs.map((p) => p.type))).filter(
isSupportedDataType
) as SupportedDataType[];

const suggestedConstants = param.literalSuggestions || param.literalOptions;

Expand All @@ -778,7 +782,9 @@ describe('autocomplete', () => {
),
...getFunctionSignaturesByReturnType(
'eval',
getTypesFromParamDefs(acceptsFieldParamDefs),
getTypesFromParamDefs(acceptsFieldParamDefs).filter(
isReturnType
) as FunctionReturnType[],
{ scalar: true },
undefined,
[fn.name]
Expand All @@ -802,7 +808,7 @@ describe('autocomplete', () => {
),
...getFunctionSignaturesByReturnType(
'eval',
getTypesFromParamDefs(acceptsFieldParamDefs),
getTypesFromParamDefs(acceptsFieldParamDefs) as FunctionReturnType[],
{ scalar: true },
undefined,
[fn.name]
Expand Down Expand Up @@ -851,13 +857,7 @@ describe('autocomplete', () => {
],
' '
);
testSuggestions('from a | eval a = 1 year /', [
',',
'| ',
...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [
'time_interval',
]),
]);
testSuggestions('from a | eval a = 1 year /', [',', '| ', 'IS NOT NULL', 'IS NULL']);
testSuggestions('from a | eval a = 1 day + 2 /', [',', '| ']);
testSuggestions(
'from a | eval 1 day + 2 /',
Expand Down Expand Up @@ -1357,6 +1357,81 @@ describe('autocomplete', () => {
['keyword']
).map((s) => (s.text.toLowerCase().includes('null') ? s : attachTriggerCommand(s)))
);
describe('field lists', () => {
// KEEP field
testSuggestions('FROM a | KEEP /', getFieldNamesByType('any').map(attachTriggerCommand));
testSuggestions(
'FROM a | KEEP d/',
getFieldNamesByType('any')
.map<PartialSuggestionWithText>((text) => ({
text,
rangeToReplace: { start: 15, end: 16 },
}))
.map(attachTriggerCommand)
);
testSuggestions(
'FROM a | KEEP doubleFiel/',
getFieldNamesByType('any').map(attachTriggerCommand)
);
testSuggestions(
'FROM a | KEEP doubleField/',
['doubleField, ', 'doubleField | ']
.map((text) => ({
text,
filterText: 'doubleField',
rangeToReplace: { start: 15, end: 26 },
}))
.map(attachTriggerCommand)
);
testSuggestions('FROM a | KEEP doubleField /', ['| ', ',']);

// Let's get funky with the field names
testSuggestions(
'FROM a | KEEP @timestamp/',
['@timestamp, ', '@timestamp | ']
.map((text) => ({
text,
filterText: '@timestamp',
rangeToReplace: { start: 15, end: 25 },
}))
.map(attachTriggerCommand),
undefined,
[[{ name: '@timestamp', type: 'date' }]]
);
testSuggestions(
'FROM a | KEEP foo.bar/',
['foo.bar, ', 'foo.bar | ']
.map((text) => ({
text,
filterText: 'foo.bar',
rangeToReplace: { start: 15, end: 22 },
}))
.map(attachTriggerCommand),
undefined,
[[{ name: 'foo.bar', type: 'double' }]]
);

// @todo re-enable these tests when we can use AST to support this case
testSuggestions.skip('FROM a | KEEP `foo.bar`/', ['foo.bar, ', 'foo.bar | '], undefined, [
[{ name: 'foo.bar', type: 'double' }],
]);
testSuggestions.skip('FROM a | KEEP `foo`.`bar`/', ['foo.bar, ', 'foo.bar | '], undefined, [
[{ name: 'foo.bar', type: 'double' }],
]);
testSuggestions.skip('FROM a | KEEP `any#Char$Field`/', [
'`any#Char$Field`, ',
'`any#Char$Field` | ',
]);

// Subsequent fields
testSuggestions(
'FROM a | KEEP doubleField, dateFiel/',
getFieldNamesByType('any')
.filter((s) => s !== 'doubleField')
.map(attachTriggerCommand)
);
testSuggestions('FROM a | KEEP doubleField, dateField/', ['dateField, ', 'dateField | ']);
});
});

describe('Replacement ranges are attached when needed', () => {
Expand Down Expand Up @@ -1417,21 +1492,21 @@ describe('autocomplete', () => {
describe('dot-separated field names', () => {
testSuggestions(
'FROM a | KEEP field.nam/',
[{ text: 'field.name', rangeToReplace: { start: 15, end: 23 } }],
[{ text: 'field.name', rangeToReplace: { start: 15, end: 24 } }],
undefined,
[[{ name: 'field.name', type: 'double' }]]
);
// multi-line
testSuggestions(
'FROM a\n| KEEP field.nam/',
[{ text: 'field.name', rangeToReplace: { start: 15, end: 23 } }],
[{ text: 'field.name', rangeToReplace: { start: 15, end: 24 } }],
undefined,
[[{ name: 'field.name', type: 'double' }]]
);
// triple separator
testSuggestions(
'FROM a\n| KEEP field.name.f/',
[{ text: 'field.name.foo', rangeToReplace: { start: 15, end: 26 } }],
[{ text: 'field.name.foo', rangeToReplace: { start: 15, end: 27 } }],
undefined,
[[{ name: 'field.name.foo', type: 'double' }]]
);
Expand Down
Loading

0 comments on commit 15ef37f

Please sign in to comment.