Skip to content

Commit

Permalink
[ES|QL] Batch of fixes and new features for autocomplete and validati…
Browse files Browse the repository at this point in the history
…on (elastic#175986)

## Summary

This PR is a batch of fixes and (tiny) new features for validation and
autocomplete items in ES|QL client side area.
Content of the PR:
* 🐛 remove array/multi-value definition annotation for `mv_*`
functions - fix elastic#174709
<img width="305" alt="Screenshot 2024-01-31 at 13 43 37"
src="https://github.com/elastic/kibana/assets/924948/2a458e74-9c87-4a67-8b42-0071e7a04f48">
<img width="287" alt="Screenshot 2024-01-31 at 13 43 25"
src="https://github.com/elastic/kibana/assets/924948/7205d984-d0c9-410b-8b39-c1d530c3f54f">
* while the type check has been relaxed for lists, the actual
non-multivalued argument type is retained and used for validation
<img width="758" alt="Screenshot 2024-01-31 at 13 44 06"
src="https://github.com/elastic/kibana/assets/924948/384e9dfa-aaf8-4e0f-953e-a2d656776ea7">

* 🐛 Remove array/multi-value definition annotation for `mv_expand`
command - fix elastic#174709

<img width="556" alt="Screenshot 2024-01-31 at 14 36 19"
src="https://github.com/elastic/kibana/assets/924948/0debbccc-da9a-4e6a-bfc9-3fded61300b5">
<img width="277" alt="Screenshot 2024-01-31 at 14 36 13"
src="https://github.com/elastic/kibana/assets/924948/9438832b-d8d5-48c5-a030-b8983057546e">


* ✨ add metadata fields validation + autocomplete (fields are
retrieved via Kibana `uiSettings`) ( part of elastic#172646 )
  

![metadata_fix](https://github.com/elastic/kibana/assets/924948/958e6609-ab73-4949-8652-f368c6604a09)
  
<img width="760" alt="Screenshot 2024-01-31 at 13 45 43"
src="https://github.com/elastic/kibana/assets/924948/b3f13d99-3e12-4586-b39c-f4ea3691a2f1">
  * ✨ Quick fixes now available also for metadata fields
  

![metadata_fixes](https://github.com/elastic/kibana/assets/924948/b273418f-a754-4c50-9539-a128f35fc4cd)

* 🐛 fixed autocomplete for `NOT` operation

<img width="549" alt="Screenshot 2024-01-31 at 13 46 43"
src="https://github.com/elastic/kibana/assets/924948/29b7aa7c-db66-485a-a5b0-329e66df842f">

* fixed autocomplete for `in` operation, with or without `not`:
* ✨ now a missing list bracket is correctly detected and
suggested after `in`
* ✨ the content for the list is suggested and takes into
account both the left side of the `in`/`not in` operator, but also the
current content of the list


![in_list_fix_2](https://github.com/elastic/kibana/assets/924948/50ea8db5-23da-411c-b81e-5ba3397b65c1)

![in_list_fix](https://github.com/elastic/kibana/assets/924948/cbb73ad2-4073-4eb6-9c12-61835858fd5e)

![not_in_list_fix](https://github.com/elastic/kibana/assets/924948/9d7fa756-3d34-49fe-ac7a-28e2edbe7fee)

* fixed `grok` and `dissect` in various ways
*:sparkles: the pattern provided by autocomplete for both was not valid
in `dissect` nor useful at all, so I've changed to something more useful
like `'"%{WORD:firstWord}"'` for `grok` and `'"%{firstWord}"'` for
`dissect` to match the first word in the text field.
* :bug: there was a bug in the validation engine as both `grok` and
`dissect` could generate new columns based on matches, so now a new
field query is fired (only when either a `GROK` or a `DISSECT` command
is detected) which enriches the set of fields available (similar to the
enrich fields)
  * ✅ Added tons of tests here


![grok_fix](https://github.com/elastic/kibana/assets/924948/a23c8edc-0702-4531-aaa5-6126e375913b)

![dissect_fix](https://github.com/elastic/kibana/assets/924948/d5591753-775e-4768-be00-31c4371be330)

* 🐛 fixed an issue with proposing an assign (`=`) operator when it
should not
  * `... EVAL var0 = round( ...) <here>` not within another assignment
  * `... EVAL var0 = 1 <here>` not within another assignment
  * `... EVAL var0 = 1 year + 2 <here>` not within another assignment
  * `... | WHERE field` should not shadow a field name in this case

* 🐛 fix an annoying auto trigger suggestion on field selection, and
kept it only for functions

![auto_trigger_fix](https://github.com/elastic/kibana/assets/924948/31d0c296-6338-450d-8f81-e08cd1c7526d)

* 🐛 fixed an error in console with autocomplete when a typed
function does not exists
* ✅  Add tests for the hover feature
* 🔥 Removed some unused functions detected via code coverage
analysis


### 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
2 people authored and fkanout committed Mar 4, 2024
1 parent 2a26691 commit aefbf7e
Show file tree
Hide file tree
Showing 26 changed files with 1,023 additions and 326 deletions.
3 changes: 1 addition & 2 deletions packages/kbn-monaco/src/esql/lib/ast/ast_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ export class AstListener implements ESQLParserListener {
if (metadataContext) {
const option = createOption(metadataContext.METADATA().text.toLowerCase(), metadataContext);
commandAst.args.push(option);
// skip for the moment as there's no easy way to get meta fields right now
// option.args.push(...collectAllColumnIdentifiers(metadataContext));
option.args.push(...collectAllColumnIdentifiers(metadataContext));
}
}

Expand Down
9 changes: 8 additions & 1 deletion packages/kbn-monaco/src/esql/lib/ast/ast_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export function createLiteralString(token: Token): ESQLLiteral {
};
}

function isMissingText(text: string) {
export function isMissingText(text: string) {
return /<missing /.test(text);
}

Expand Down Expand Up @@ -180,6 +180,13 @@ export function computeLocationExtends(fn: ESQLFunction) {
location.min = walkFunctionStructure(fn.args, location, 'min', () => 0);
// get max location navigating in depth keeping the right/last arg
location.max = walkFunctionStructure(fn.args, location, 'max', (args) => args.length - 1);
// in case of empty array as last arg, bump the max location by 3 chars (empty brackets)
if (
Array.isArray(fn.args[fn.args.length - 1]) &&
!(fn.args[fn.args.length - 1] as ESQLAstItem[]).length
) {
location.max += 3;
}
}
return location;
}
Expand Down
31 changes: 21 additions & 10 deletions packages/kbn-monaco/src/esql/lib/ast/ast_walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import {
createPolicy,
createSettingTuple,
createLiteralString,
isMissingText,
} from './ast_helpers';
import { getPosition } from './ast_position_utils';
import type {
Expand Down Expand Up @@ -206,11 +207,16 @@ function visitLogicalAndsOrs(ctx: LogicalBinaryContext) {
function visitLogicalIns(ctx: LogicalInContext) {
const fn = createFunction(ctx.NOT() ? 'not_in' : 'in', ctx);
const [left, ...list] = ctx.valueExpression();
const values = [visitValueExpression(left), list.map((ve) => visitValueExpression(ve))];
for (const arg of values) {
if (arg) {
const filteredArgs = Array.isArray(arg) ? arg.filter(nonNullable) : [arg];
fn.args.push(filteredArgs);
const leftArg = visitValueExpression(left);
if (leftArg) {
fn.args.push(...(Array.isArray(leftArg) ? leftArg : [leftArg]));
const values = list.map((ve) => visitValueExpression(ve));
const listArgs = values
.filter(nonNullable)
.flatMap((arg) => (Array.isArray(arg) ? arg.filter(nonNullable) : arg));
// distinguish between missing brackets (missing text error) and an empty list
if (!isMissingText(ctx.text)) {
fn.args.push(listArgs);
}
}
// update the location of the assign based on arguments
Expand Down Expand Up @@ -244,6 +250,9 @@ function getComparisonName(ctx: ComparisonOperatorContext) {
}

function visitValueExpression(ctx: ValueExpressionContext) {
if (isMissingText(ctx.text)) {
return [];
}
if (ctx instanceof ValueExpressionDefaultContext) {
return visitOperatorExpression(ctx.operatorExpression());
}
Expand Down Expand Up @@ -538,16 +547,18 @@ export function visitDissect(ctx: DissectCommandContext) {
const pattern = ctx.string().tryGetToken(esql_parser.STRING, 0);
return [
visitPrimaryExpression(ctx.primaryExpression()),
createLiteral('string', pattern),
...visitDissectOptions(ctx.commandOptions()),
...(pattern && !isMissingText(pattern.text)
? [createLiteral('string', pattern), ...visitDissectOptions(ctx.commandOptions())]
: []),
].filter(nonNullable);
}

export function visitGrok(ctx: GrokCommandContext) {
const pattern = ctx.string().tryGetToken(esql_parser.STRING, 0);
return [visitPrimaryExpression(ctx.primaryExpression()), createLiteral('string', pattern)].filter(
nonNullable
);
return [
visitPrimaryExpression(ctx.primaryExpression()),
...(pattern && !isMissingText(pattern.text) ? [createLiteral('string', pattern)] : []),
].filter(nonNullable);
}

function visitDissectOptions(ctx: CommandOptionsContext | undefined) {
Expand Down
Loading

0 comments on commit aefbf7e

Please sign in to comment.