Skip to content

Commit

Permalink
[ES|QL] Improve support for Invoke completion trigger kind (#188877)
Browse files Browse the repository at this point in the history
## Summary

Resolve #188677

This PR extends our support for Monaco's "Invoke" completion trigger
kind
([ref](https://microsoft.github.io/monaco-editor/typedoc/enums/languages.CompletionTriggerKind.html)).

What this means for the user is that suggestions are now shown when the
user types a character in a word even if the suggestion menu isn't
already open. This situation often occurs when the user is editing a
query. For example, they may follow a flow like this:

1. `FROM my-index | LIMIT 10<cursor-here>`
2. `FROM <cursor-here> | LIMIT 10` (deleted the index name)
3. `FROM k<cursor-here> | LIMIT 10` (types the first character of the
index name)

Previously, they wouldn't get any help. Now, we show the list of
suggestions.

The following table shows the cases that I considered as well as the
ones that are now fixed. I also added test coverage, even for cases that
worked before to prevent subtle regressions.

| | Before | After |

|------------------------------|------------------------------------------|-----------------------|
| Source command | ✅ | ✅ |
| Pipe command | ❌ | ✅ |
| Function argument | ✅ | ✅ |
| FROM source | ❌ | ✅ |
| FROM source METADATA | ✅ | ✅ |
| FROM source METADATA field | ❌ (for `_`) | ✅ |
| EVAL argument | ✅ | ✅ |
| DISSECT field | ✅ | ✅ |
| DISSECT field pattern | ❌ | ❌ |
| DROP field | ✅ | ✅ |
| DROP field1, field2 | ❌ | ✅ |
| ENRICH policy | ✅ | ✅ |
| ENRICH policy ON | ✅ | ✅ |
| ENRICH policy ON field | ✅ | ✅ |
| ENRICH policy WITH | ✅ | ✅ |
| ENRICH policy WITH field | ✅ | ✅ |
| GROK field | ✅ | ✅ |
| GROK field pattern | ❌ | ❌ |
| KEEP field | ✅ | ✅ |
| KEEP field1, field2 | ❌ | ✅ |
| LIMIT number | ❌ | ❌ |
| MV_EXPAND field | ✅ | ✅ |
| RENAME field | ✅ | ✅ |
| RENAME field AS | ❌ | ✅ |
| RENAME field AS var0 | ✅ | ✅ |
| SORT field | ✅ | ✅ |
| SORT field order | ✅ | ✅ |
| SORT field order nulls-order | ✅ | ✅ |
| STATS argument | ✅ | ✅ |
| STATS argument BY | ✅ | ✅ |
| STATS argument BY expression | ✅ | ✅ |
| WHERE argument | ✅ | ✅ |
| WHERE argument comparison    | ✅  | ✅                     |

As I worked, I encountered a couple small bugs which I also fixed.

### Escaping field values in `ENRICH`

**Before**


https://github.com/user-attachments/assets/88f2bf35-a703-4cf4-8d45-a24452a1b59d

**After**


https://github.com/user-attachments/assets/69ad0770-f158-44fb-be8d-66b497c7f7f7

### Suggesting variable assignment in `ENRICH ... WITH`
**Before**


https://github.com/user-attachments/assets/fec47b6d-eae5-44e8-b739-9b2eecc7458b



**After**


https://github.com/user-attachments/assets/dff5afe1-37e7-470b-bb9c-edb5ac87e76d


### 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: kibanamachine <[email protected]>
  • Loading branch information
drewdaemon and kibanamachine authored Jul 24, 2024
1 parent 6cc7444 commit c77cd80
Show file tree
Hide file tree
Showing 8 changed files with 361 additions and 96 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/ast_walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ export function visitRenameClauses(clausesCtx: RenameClauseContext[]): ESQLAstIt
return clausesCtx
.map((clause) => {
const asToken = clause.getToken(esql_parser.AS, 0);
if (asToken) {
if (asToken && textExistsAndIsValid(asToken.getText())) {
const fn = createOption(asToken.getText().toLowerCase(), clause);
for (const arg of [clause._oldName, clause._newName]) {
if (textExistsAndIsValid(arg.getText())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const allEvaFunctions = getFunctionSignaturesByReturnType(
'stats',
'any',
{
evalMath: true,
scalar: true,
grouping: false,
},
undefined,
Expand Down Expand Up @@ -77,37 +77,37 @@ describe('autocomplete.suggest', () => {
'from a | stats by bucket(/',
[
...getFieldNamesByType(['number', 'date']),
...getFunctionSignaturesByReturnType('eval', ['date', 'number'], { evalMath: true }),
...getFunctionSignaturesByReturnType('eval', ['date', 'number'], { scalar: true }),
].map((field) => `${field},`)
);

await assertSuggestions('from a | stats round(/', [
...getFunctionSignaturesByReturnType('stats', 'number', { agg: true, grouping: true }),
...getFieldNamesByType('number'),
...getFunctionSignaturesByReturnType('eval', 'number', { evalMath: true }, undefined, [
...getFunctionSignaturesByReturnType('eval', 'number', { scalar: true }, undefined, [
'round',
]),
]);
await assertSuggestions('from a | stats round(round(/', [
...getFunctionSignaturesByReturnType('stats', 'number', { agg: true }),
...getFieldNamesByType('number'),
...getFunctionSignaturesByReturnType('eval', 'number', { evalMath: true }, undefined, [
...getFunctionSignaturesByReturnType('eval', 'number', { scalar: true }, undefined, [
'round',
]),
]);
await assertSuggestions('from a | stats avg(round(/', [
...getFieldNamesByType('number'),
...getFunctionSignaturesByReturnType('eval', 'number', { evalMath: true }, undefined, [
...getFunctionSignaturesByReturnType('eval', 'number', { scalar: true }, undefined, [
'round',
]),
]);
await assertSuggestions('from a | stats avg(/', [
...getFieldNamesByType('number'),
...getFunctionSignaturesByReturnType('eval', 'number', { evalMath: true }),
...getFunctionSignaturesByReturnType('eval', 'number', { scalar: true }),
]);
await assertSuggestions('from a | stats round(avg(/', [
...getFieldNamesByType('number'),
...getFunctionSignaturesByReturnType('eval', 'number', { evalMath: true }, undefined, [
...getFunctionSignaturesByReturnType('eval', 'number', { scalar: true }, undefined, [
'round',
]),
]);
Expand All @@ -118,7 +118,7 @@ describe('autocomplete.suggest', () => {
const expected = [
...getFieldNamesByType(['number', 'date', 'boolean', 'ip']),
...getFunctionSignaturesByReturnType('stats', ['number', 'date', 'boolean', 'ip'], {
evalMath: true,
scalar: true,
}),
];

Expand All @@ -132,7 +132,7 @@ describe('autocomplete.suggest', () => {

await assertSuggestions('from a | stats avg(b/) by stringField', [
...getFieldNamesByType('number'),
...getFunctionSignaturesByReturnType('eval', 'number', { evalMath: true }),
...getFunctionSignaturesByReturnType('eval', 'number', { scalar: true }),
]);
});

Expand Down Expand Up @@ -198,7 +198,7 @@ describe('autocomplete.suggest', () => {
await assertSuggestions('from a | stats avg(b) by c, /', [
'var0 =',
...getFieldNamesByType('any'),
...getFunctionSignaturesByReturnType('eval', 'any', { evalMath: true }),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }),
...allGroupingFunctions,
]);
});
Expand All @@ -209,7 +209,7 @@ describe('autocomplete.suggest', () => {
await assertSuggestions('from a | stats avg(b) by numberField % /', [
...getFieldNamesByType('number'),
'`avg(b)`',
...getFunctionSignaturesByReturnType('eval', 'number', { evalMath: true }),
...getFunctionSignaturesByReturnType('eval', 'number', { scalar: true }),
...allGroupingFunctions,
]);
await assertSuggestions('from a | stats avg(b) by var0 = /', [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@ export function getFunctionSignaturesByReturnType(
{
agg,
grouping,
evalMath,
scalar,
builtin,
// 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,
}: {
agg?: boolean;
grouping?: boolean;
evalMath?: boolean;
scalar?: boolean;
builtin?: boolean;
skipAssign?: boolean;
} = {},
Expand All @@ -157,7 +157,7 @@ export function getFunctionSignaturesByReturnType(
list.push(...groupingFunctionDefinitions);
}
// eval functions (eval is a special keyword in JS)
if (evalMath) {
if (scalar) {
list.push(...evalFunctionDefinitions);
}
if (builtin) {
Expand Down
Loading

0 comments on commit c77cd80

Please sign in to comment.