From 4100c79caf658059ee7aac3da357007008669a89 Mon Sep 17 00:00:00 2001 From: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com> Date: Thu, 19 Sep 2024 05:53:05 +0200 Subject: [PATCH] [ES|QL] Implement `OrderExpression` for `SORT` command arguments (#189959) ## Summary Closes https://github.com/elastic/kibana/issues/189491 - Adds *order expression* AST nodes, which are minted from `SORT` command. - Improves SORT command autocomplete suggestions. Shows fields on first space: image It now shows `NULLS FIRST` and `NULLS LAST`, even before `ASC` or `DESC` was entered, as `ASC` and `DESC` are optional: image Once `ASC` or `DESC` is entered, shows only nulls options: image It also now suggests partial modifier, if the in-progress text that user is typing matches it: image (However, we are not triggering autocomplete in those cases in UI, so no way to see it in UI right now.) ### Checklist Delete any items that are not applicable to this PR. - [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 ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Elastic Machine Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 2efd0f0d8bb9478aec0316d1760adc184feb6309) --- .../src/__tests__/ast_parser.sort.test.ts | 141 +++++++++++++++--- packages/kbn-esql-ast/src/ast_factory.ts | 4 +- packages/kbn-esql-ast/src/ast_helpers.ts | 21 +++ packages/kbn-esql-ast/src/ast_walker.ts | 63 ++++---- .../__tests__/basic_pretty_printer.test.ts | 16 +- .../src/pretty_print/basic_pretty_printer.ts | 23 ++- packages/kbn-esql-ast/src/types.ts | 18 ++- packages/kbn-esql-ast/src/visitor/contexts.ts | 6 + .../src/visitor/global_visitor_context.ts | 14 ++ packages/kbn-esql-ast/src/visitor/types.ts | 7 +- .../autocomplete.command.sort.test.ts | 106 +++++++++++++ .../src/autocomplete/autocomplete.test.ts | 70 --------- .../src/autocomplete/autocomplete.ts | 102 +++++++++++++ .../autocomplete/commands/sort/helper.test.ts | 55 +++++++ .../src/autocomplete/commands/sort/helper.ts | 84 +++++++++++ .../src/definitions/builtin.ts | 14 ++ .../src/definitions/commands.ts | 6 +- .../src/shared/context.ts | 2 +- 18 files changed, 616 insertions(+), 136 deletions(-) create mode 100644 packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts create mode 100644 packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts create mode 100644 packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts diff --git a/packages/kbn-esql-ast/src/__tests__/ast_parser.sort.test.ts b/packages/kbn-esql-ast/src/__tests__/ast_parser.sort.test.ts index c57a75644bcec..bb5e6aeb1e6b4 100644 --- a/packages/kbn-esql-ast/src/__tests__/ast_parser.sort.test.ts +++ b/packages/kbn-esql-ast/src/__tests__/ast_parser.sort.test.ts @@ -11,11 +11,49 @@ import { getAstAndSyntaxErrors as parse } from '../ast_parser'; describe('SORT', () => { describe('correctly formatted', () => { - // Un-skip one https://github.com/elastic/kibana/issues/189491 fixed. - it.skip('example from documentation', () => { + it('sorting order without modifiers', () => { + const text = `FROM employees | SORT height`; + const { ast, errors } = parse(text); + + expect(errors.length).toBe(0); + expect(ast).toMatchObject([ + {}, + { + type: 'command', + name: 'sort', + args: [ + { + type: 'column', + name: 'height', + }, + ], + }, + ]); + }); + + it('sort expression is a function call', () => { + const text = `from a_index | sort values(textField)`; + const { ast, errors } = parse(text); + + expect(errors.length).toBe(0); + expect(ast).toMatchObject([ + {}, + { + type: 'command', + name: 'sort', + args: [ + { + type: 'function', + name: 'values', + }, + ], + }, + ]); + }); + + it('with order modifier "DESC"', () => { const text = ` FROM employees - | KEEP first_name, last_name, height | SORT height DESC `; const { ast, errors } = parse(text); @@ -23,22 +61,57 @@ describe('SORT', () => { expect(errors.length).toBe(0); expect(ast).toMatchObject([ {}, + { + type: 'command', + name: 'sort', + args: [ + { + type: 'order', + order: 'DESC', + nulls: '', + args: [ + { + type: 'column', + name: 'height', + }, + ], + }, + ], + }, + ]); + }); + + it('with nulls modifier "NULLS LAST"', () => { + const text = ` + FROM employees + | SORT height NULLS LAST + `; + const { ast, errors } = parse(text); + + expect(errors.length).toBe(0); + expect(ast).toMatchObject([ {}, { type: 'command', name: 'sort', args: [ { - type: 'column', - name: 'height', + type: 'order', + order: '', + nulls: 'NULLS LAST', + args: [ + { + type: 'column', + name: 'height', + }, + ], }, ], }, ]); }); - // Un-skip once https://github.com/elastic/kibana/issues/189491 fixed. - it.skip('can parse various sorting columns with options', () => { + it('can parse various sorting columns with options', () => { const text = 'FROM a | SORT a, b ASC, c DESC, d NULLS FIRST, e NULLS LAST, f ASC NULLS FIRST, g DESC NULLS LAST'; const { ast, errors } = parse(text); @@ -55,28 +128,58 @@ describe('SORT', () => { name: 'a', }, { - type: 'column', - name: 'b', + order: 'ASC', + nulls: '', + args: [ + { + name: 'b', + }, + ], }, { - type: 'column', - name: 'c', + order: 'DESC', + nulls: '', + args: [ + { + name: 'c', + }, + ], }, { - type: 'column', - name: 'd', + order: '', + nulls: 'NULLS FIRST', + args: [ + { + name: 'd', + }, + ], }, { - type: 'column', - name: 'e', + order: '', + nulls: 'NULLS LAST', + args: [ + { + name: 'e', + }, + ], }, { - type: 'column', - name: 'f', + order: 'ASC', + nulls: 'NULLS FIRST', + args: [ + { + name: 'f', + }, + ], }, { - type: 'column', - name: 'g', + order: 'DESC', + nulls: 'NULLS LAST', + args: [ + { + name: 'g', + }, + ], }, ], }, diff --git a/packages/kbn-esql-ast/src/ast_factory.ts b/packages/kbn-esql-ast/src/ast_factory.ts index 44b8c03aa1e7f..f5c3ca7a3b621 100644 --- a/packages/kbn-esql-ast/src/ast_factory.ts +++ b/packages/kbn-esql-ast/src/ast_factory.ts @@ -53,7 +53,7 @@ import { visitDissect, visitGrok, collectBooleanExpression, - visitOrderExpression, + visitOrderExpressions, getPolicyName, getMatchField, getEnrichClauses, @@ -238,7 +238,7 @@ export class AstListener implements ESQLParserListener { exitSortCommand(ctx: SortCommandContext) { const command = createCommand('sort', ctx); this.ast.push(command); - command.args.push(...visitOrderExpression(ctx.orderExpression_list())); + command.args.push(...visitOrderExpressions(ctx.orderExpression_list())); } /** diff --git a/packages/kbn-esql-ast/src/ast_helpers.ts b/packages/kbn-esql-ast/src/ast_helpers.ts index 76f576f1ec019..7d4a94fde19a8 100644 --- a/packages/kbn-esql-ast/src/ast_helpers.ts +++ b/packages/kbn-esql-ast/src/ast_helpers.ts @@ -42,6 +42,7 @@ import type { ESQLNumericLiteralType, FunctionSubtype, ESQLNumericLiteral, + ESQLOrderExpression, } from './types'; import { parseIdentifier } from './parser/helpers'; @@ -222,6 +223,26 @@ export function createFunction( return node; } +export const createOrderExpression = ( + ctx: ParserRuleContext, + arg: ESQLAstItem, + order: ESQLOrderExpression['order'], + nulls: ESQLOrderExpression['nulls'] +) => { + const node: ESQLOrderExpression = { + type: 'order', + name: '', + order, + nulls, + args: [arg], + text: ctx.getText(), + location: getPosition(ctx.start, ctx.stop), + incomplete: Boolean(ctx.exception), + }; + + return node; +}; + function walkFunctionStructure( args: ESQLAstItem[], initialLocation: ESQLLocation, diff --git a/packages/kbn-esql-ast/src/ast_walker.ts b/packages/kbn-esql-ast/src/ast_walker.ts index 3599f2f5fabec..d57c4d1c64ae4 100644 --- a/packages/kbn-esql-ast/src/ast_walker.ts +++ b/packages/kbn-esql-ast/src/ast_walker.ts @@ -84,6 +84,7 @@ import { textExistsAndIsValid, createInlineCast, createUnknownItem, + createOrderExpression, } from './ast_helpers'; import { getPosition } from './ast_position_utils'; import { @@ -97,6 +98,7 @@ import { ESQLUnnamedParamLiteral, ESQLPositionalParamLiteral, ESQLNamedParamLiteral, + ESQLOrderExpression, } from './types'; export function collectAllSourceIdentifiers(ctx: FromCommandContext): ESQLAstItem[] { @@ -608,34 +610,43 @@ export function visitByOption( return [option]; } -export function visitOrderExpression(ctx: OrderExpressionContext[]) { - const ast: ESQLAstItem[] = []; - for (const orderCtx of ctx) { - const expression = collectBooleanExpression(orderCtx.booleanExpression()); - if (orderCtx._ordering) { - const terminalNode = - orderCtx.getToken(esql_parser.ASC, 0) || orderCtx.getToken(esql_parser.DESC, 0); - const literal = createLiteral('string', terminalNode); - if (literal) { - expression.push(literal); - } - } - if (orderCtx.NULLS()) { - expression.push(createLiteral('string', orderCtx.NULLS()!)!); - if (orderCtx._nullOrdering && orderCtx._nullOrdering.text !== '') { - const innerTerminalNode = - orderCtx.getToken(esql_parser.FIRST, 0) || orderCtx.getToken(esql_parser.LAST, 0); - const literal = createLiteral('string', innerTerminalNode); - if (literal) { - expression.push(literal); - } - } - } +const visitOrderExpression = (ctx: OrderExpressionContext): ESQLOrderExpression | ESQLAstItem => { + const arg = collectBooleanExpression(ctx.booleanExpression())[0]; - if (expression.length) { - ast.push(...expression); - } + let order: ESQLOrderExpression['order'] = ''; + let nulls: ESQLOrderExpression['nulls'] = ''; + + const ordering = ctx._ordering?.text?.toUpperCase(); + + if (ordering) order = ordering as ESQLOrderExpression['order']; + + const nullOrdering = ctx._nullOrdering?.text?.toUpperCase(); + + switch (nullOrdering) { + case 'LAST': + nulls = 'NULLS LAST'; + break; + case 'FIRST': + nulls = 'NULLS FIRST'; + break; } + + if (!order && !nulls) { + return arg; + } + + return createOrderExpression(ctx, arg, order, nulls); +}; + +export function visitOrderExpressions( + ctx: OrderExpressionContext[] +): Array { + const ast: Array = []; + + for (const orderCtx of ctx) { + ast.push(visitOrderExpression(orderCtx)); + } + return ast; } diff --git a/packages/kbn-esql-ast/src/pretty_print/__tests__/basic_pretty_printer.test.ts b/packages/kbn-esql-ast/src/pretty_print/__tests__/basic_pretty_printer.test.ts index 3b4734da9d45f..caf8c55dba3e0 100644 --- a/packages/kbn-esql-ast/src/pretty_print/__tests__/basic_pretty_printer.test.ts +++ b/packages/kbn-esql-ast/src/pretty_print/__tests__/basic_pretty_printer.test.ts @@ -50,18 +50,22 @@ describe('single line query', () => { expect(text).toBe('FROM a | SORT b'); }); - /** @todo Enable once order expressions are supported. */ - test.skip('order expression with ASC modifier', () => { + test('order expression with ASC modifier', () => { const { text } = reprint('FROM a | SORT b ASC'); expect(text).toBe('FROM a | SORT b ASC'); }); - /** @todo Enable once order expressions are supported. */ - test.skip('order expression with ASC and NULLS FIRST modifier', () => { - const { text } = reprint('FROM a | SORT b ASC NULLS FIRST'); + test('order expression with NULLS LAST modifier', () => { + const { text } = reprint('FROM a | SORT b NULLS LAST'); - expect(text).toBe('FROM a | SORT b ASC NULLS FIRST'); + expect(text).toBe('FROM a | SORT b NULLS LAST'); + }); + + test('order expression with DESC and NULLS FIRST modifier', () => { + const { text } = reprint('FROM a | SORT b DESC NULLS FIRST'); + + expect(text).toBe('FROM a | SORT b DESC NULLS FIRST'); }); }); diff --git a/packages/kbn-esql-ast/src/pretty_print/basic_pretty_printer.ts b/packages/kbn-esql-ast/src/pretty_print/basic_pretty_printer.ts index 6c190dcd3c5d9..1aa3d492c7583 100644 --- a/packages/kbn-esql-ast/src/pretty_print/basic_pretty_printer.ts +++ b/packages/kbn-esql-ast/src/pretty_print/basic_pretty_printer.ts @@ -133,7 +133,7 @@ export class BasicPrettyPrinter { : word.toUpperCase(); } - protected readonly visitor = new Visitor() + protected readonly visitor: Visitor = new Visitor() .on('visitExpression', (ctx) => { return ''; }) @@ -229,6 +229,21 @@ export class BasicPrettyPrinter { return `${ctx.visitArgument(0)} ${this.keyword('AS')} ${ctx.visitArgument(1)}`; }) + .on('visitOrderExpression', (ctx) => { + const node = ctx.node; + let text = ctx.visitArgument(0); + + if (node.order) { + text += ` ${node.order}`; + } + + if (node.nulls) { + text += ` ${node.nulls}`; + } + + return text; + }) + .on('visitCommandOption', (ctx) => { const opts = this.opts; const option = opts.lowercaseOptions ? ctx.node.name : ctx.node.name.toUpperCase(); @@ -281,14 +296,14 @@ export class BasicPrettyPrinter { }); public print(query: ESQLAstQueryNode) { - return this.visitor.visitQuery(query); + return this.visitor.visitQuery(query, undefined); } public printCommand(command: ESQLAstCommand) { - return this.visitor.visitCommand(command); + return this.visitor.visitCommand(command, undefined); } public printExpression(expression: ESQLAstExpressionNode) { - return this.visitor.visitExpression(expression); + return this.visitor.visitExpression(expression, undefined); } } diff --git a/packages/kbn-esql-ast/src/types.ts b/packages/kbn-esql-ast/src/types.ts index da42ec24bd69b..e98057258ee61 100644 --- a/packages/kbn-esql-ast/src/types.ts +++ b/packages/kbn-esql-ast/src/types.ts @@ -26,6 +26,7 @@ export type ESQLSingleAstItem = | ESQLLiteral // "literal expression" | ESQLCommandMode | ESQLInlineCast // "inline cast expression" + | ESQLOrderExpression | ESQLUnknownItem; export type ESQLAstField = ESQLFunction | ESQLColumn; @@ -135,11 +136,26 @@ export interface ESQLUnaryExpression extends ESQLFunction<'unary-expression'> { args: [ESQLAstItem]; } -export interface ESQLPostfixUnaryExpression extends ESQLFunction<'postfix-unary-expression'> { +export interface ESQLPostfixUnaryExpression + extends ESQLFunction<'postfix-unary-expression', Name> { subtype: 'postfix-unary-expression'; args: [ESQLAstItem]; } +/** + * Represents an order expression used in SORT commands. + * + * ``` + * ... | SORT field ASC NULLS FIRST + * ``` + */ +export interface ESQLOrderExpression extends ESQLAstBaseItem { + type: 'order'; + order: '' | 'ASC' | 'DESC'; + nulls: '' | 'NULLS FIRST' | 'NULLS LAST'; + args: [field: ESQLAstItem]; +} + export interface ESQLBinaryExpression extends ESQLFunction<'binary-expression', BinaryExpressionOperator> { subtype: 'binary-expression'; diff --git a/packages/kbn-esql-ast/src/visitor/contexts.ts b/packages/kbn-esql-ast/src/visitor/contexts.ts index 2cedf0d6ba8a3..c646b7f446227 100644 --- a/packages/kbn-esql-ast/src/visitor/contexts.ts +++ b/packages/kbn-esql-ast/src/visitor/contexts.ts @@ -26,6 +26,7 @@ import type { ESQLIntegerLiteral, ESQLList, ESQLLiteral, + ESQLOrderExpression, ESQLSingleAstItem, ESQLSource, ESQLTimeInterval, @@ -543,3 +544,8 @@ export class RenameExpressionVisitorContext< Methods extends VisitorMethods = VisitorMethods, Data extends SharedData = SharedData > extends VisitorContext {} + +export class OrderExpressionVisitorContext< + Methods extends VisitorMethods = VisitorMethods, + Data extends SharedData = SharedData +> extends VisitorContext {} diff --git a/packages/kbn-esql-ast/src/visitor/global_visitor_context.ts b/packages/kbn-esql-ast/src/visitor/global_visitor_context.ts index 8260776cca2f5..793803bc48f54 100644 --- a/packages/kbn-esql-ast/src/visitor/global_visitor_context.ts +++ b/packages/kbn-esql-ast/src/visitor/global_visitor_context.ts @@ -16,6 +16,7 @@ import type { ESQLInlineCast, ESQLList, ESQLLiteral, + ESQLOrderExpression, ESQLSource, ESQLTimeInterval, } from '../types'; @@ -400,6 +401,10 @@ export class GlobalVisitorContext< if (!this.methods.visitInlineCastExpression) break; return this.visitInlineCastExpression(parent, expressionNode, input as any); } + case 'order': { + if (!this.methods.visitOrderExpression) break; + return this.visitOrderExpression(parent, expressionNode, input as any); + } case 'option': { switch (expressionNode.name) { case 'as': { @@ -487,4 +492,13 @@ export class GlobalVisitorContext< const context = new contexts.RenameExpressionVisitorContext(this, node, parent); return this.visitWithSpecificContext('visitRenameExpression', context, input); } + + public visitOrderExpression( + parent: contexts.VisitorContext | null, + node: ESQLOrderExpression, + input: types.VisitorInput + ): types.VisitorOutput { + const context = new contexts.OrderExpressionVisitorContext(this, node, parent); + return this.visitWithSpecificContext('visitOrderExpression', context, input); + } } diff --git a/packages/kbn-esql-ast/src/visitor/types.ts b/packages/kbn-esql-ast/src/visitor/types.ts index 28259fb1cbaf4..c5b18a727bc3c 100644 --- a/packages/kbn-esql-ast/src/visitor/types.ts +++ b/packages/kbn-esql-ast/src/visitor/types.ts @@ -61,7 +61,8 @@ export type ExpressionVisitorInput = AnyToVoid< VisitorInput & VisitorInput & VisitorInput & - VisitorInput + VisitorInput & + VisitorInput >; /** @@ -76,7 +77,8 @@ export type ExpressionVisitorOutput = | VisitorOutput | VisitorOutput | VisitorOutput - | VisitorOutput; + | VisitorOutput + | VisitorOutput; /** * Input that satisfies any command visitor input constraints. @@ -203,6 +205,7 @@ export interface VisitorMethods< any, any >; + visitOrderExpression?: Visitor, any, any>; } /** diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts new file mode 100644 index 0000000000000..924790ed470f5 --- /dev/null +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts @@ -0,0 +1,106 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { setup, getFieldNamesByType } from './helpers'; + +describe('autocomplete.suggest', () => { + describe('SORT ( [ ASC / DESC ] [ NULLS FIST / NULLS LAST ] )+', () => { + describe('SORT ...', () => { + test('suggests command on first character', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions( + 'from a | sort /', + [...getFieldNamesByType('any')].map((field) => `${field} `) + ); + await assertSuggestions( + 'from a | sort column, /', + [...getFieldNamesByType('any')].map((field) => `${field} `) + ); + }); + }); + + describe('... [ ASC / DESC ] ...', () => { + test('suggests all modifiers on first space', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField /', [ + 'ASC ', + 'DESC ', + 'NULLS FIRST ', + 'NULLS LAST ', + ',', + '| ', + ]); + }); + + test('when user starts to type ASC modifier', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField A/', ['ASC ']); + }); + + test('when user starts to type DESC modifier', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField d/', ['DESC ']); + await assertSuggestions('from a | sort stringField des/', ['DESC ']); + await assertSuggestions('from a | sort stringField DES/', ['DESC ']); + }); + }); + + describe('... [ NULLS FIST / NULLS LAST ]', () => { + test('suggests command on first character', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField ASC /', [ + 'NULLS FIRST ', + 'NULLS LAST ', + ',', + '| ', + ]); + }); + + test('when user starts to type NULLS modifiers', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField N/', ['NULLS FIRST ', 'NULLS LAST ']); + await assertSuggestions('from a | sort stringField null/', ['NULLS FIRST ', 'NULLS LAST ']); + await assertSuggestions('from a | sort stringField nulls/', [ + 'NULLS FIRST ', + 'NULLS LAST ', + ]); + await assertSuggestions('from a | sort stringField nulls /', [ + 'NULLS FIRST ', + 'NULLS LAST ', + ]); + }); + + test('when user types NULLS FIRST', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField NULLS F/', ['NULLS FIRST ']); + await assertSuggestions('from a | sort stringField NULLS FI/', ['NULLS FIRST ']); + }); + + test('when user types NULLS LAST', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField NULLS L/', ['NULLS LAST ']); + await assertSuggestions('from a | sort stringField NULLS LAS/', ['NULLS LAST ']); + }); + + test('after nulls are entered, suggests comma or pipe', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort stringField NULLS LAST /', [',', '| ']); + }); + }); + }); +}); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index a58a55f124c4e..2a8c8e53fcab7 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -330,22 +330,6 @@ describe('autocomplete', () => { testSuggestions('from a | dissect keywordField/', []); }); - describe('sort', () => { - testSuggestions('from a | sort /', [ - ...getFieldNamesByType('any').map((name) => `${name} `), - ...getFunctionSignaturesByReturnType('sort', 'any', { scalar: true }), - ]); - testSuggestions('from a | sort keywordField /', ['ASC ', 'DESC ', ',', '| ']); - testSuggestions('from a | sort keywordField desc /', [ - 'NULLS FIRST ', - 'NULLS LAST ', - ',', - '| ', - ]); - // @TODO: improve here - // testSuggestions('from a | sort keywordField desc ', ['first', 'last']); - }); - describe('limit', () => { testSuggestions('from a | limit /', ['10 ', '100 ', '1000 ']); testSuggestions('from a | limit 4 /', ['| ']); @@ -672,23 +656,6 @@ describe('autocomplete', () => { // RENAME field AS var0 testSuggestions('FROM index1 | RENAME field AS v/', ['var0']); - // SORT field - testSuggestions('FROM index1 | SORT f/', [ - ...getFunctionSignaturesByReturnType('sort', 'any', { scalar: true }), - ...getFieldNamesByType('any').map((field) => `${field} `), - ]); - - // SORT field order - testSuggestions('FROM index1 | SORT keywordField a/', ['ASC ', 'DESC ', ',', '| ']); - - // SORT field order nulls - testSuggestions('FROM index1 | SORT keywordField ASC n/', [ - 'NULLS FIRST ', - 'NULLS LAST ', - ',', - '| ', - ]); - // STATS argument testSuggestions('FROM index1 | STATS f/', [ 'var0 = ', @@ -1015,27 +982,6 @@ describe('autocomplete', () => { // LIMIT number testSuggestions('FROM a | LIMIT /', ['10 ', '100 ', '1000 '].map(attachTriggerCommand)); - // SORT field - testSuggestions( - 'FROM a | SORT /', - [ - ...getFieldNamesByType('any').map((field) => `${field} `), - ...getFunctionSignaturesByReturnType('sort', 'any', { scalar: true }), - ].map(attachTriggerCommand) - ); - - // SORT field order - testSuggestions('FROM a | SORT field /', [ - ',', - ...['ASC ', 'DESC ', '| '].map(attachTriggerCommand), - ]); - - // SORT field order nulls - testSuggestions('FROM a | SORT field ASC /', [ - ',', - ...['NULLS FIRST ', 'NULLS LAST ', '| '].map(attachTriggerCommand), - ]); - // STATS argument testSuggestions( 'FROM a | STATS /', @@ -1266,22 +1212,6 @@ describe('autocomplete', () => { '>= $0', 'IN $0', ]); - testSuggestions('FROM a | SORT doubleField IS NOT N/', [ - { text: 'IS NOT NULL', rangeToReplace: { start: 27, end: 34 } }, - 'IS NULL', - '% $0', - '* $0', - '+ $0', - '- $0', - '/ $0', - '!= $0', - '< $0', - '<= $0', - '== $0', - '> $0', - '>= $0', - 'IN $0', - ]); describe('dot-separated field names', () => { testSuggestions( 'FROM a | KEEP field.nam/', diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index a1be88b0b1436..50937abbde9fc 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -102,6 +102,7 @@ import { removeQuoteForSuggestedSources, getValidSignaturesAndTypesToSuggestNext, } from './helper'; +import { getSortPos } from './commands/sort/helper'; import { FunctionParameter, FunctionReturnType, @@ -192,6 +193,10 @@ export async function suggest( } if (astContext.type === 'expression') { + if (astContext.command.name === 'sort') { + return await suggestForSortCmd(innerText, getFieldsByType); + } + // suggest next possible argument, or option // otherwise a variable return getExpressionSuggestionsByType( @@ -1833,3 +1838,100 @@ async function getOptionArgsSuggestions( } return suggestions; } + +const sortModifierSuggestions = { + ASC: { + label: 'ASC', + text: 'ASC ', + detail: '', + kind: 'Keyword', + sortText: '1-ASC', + command: TRIGGER_SUGGESTION_COMMAND, + } as SuggestionRawDefinition, + DESC: { + label: 'DESC', + text: 'DESC ', + detail: '', + kind: 'Keyword', + sortText: '1-DESC', + command: TRIGGER_SUGGESTION_COMMAND, + } as SuggestionRawDefinition, + NULLS_FIRST: { + label: 'NULLS FIRST', + text: 'NULLS FIRST ', + detail: '', + kind: 'Keyword', + sortText: '2-NULLS FIRST', + command: TRIGGER_SUGGESTION_COMMAND, + } as SuggestionRawDefinition, + NULLS_LAST: { + label: 'NULLS LAST', + text: 'NULLS LAST ', + detail: '', + kind: 'Keyword', + sortText: '2-NULLS LAST', + command: TRIGGER_SUGGESTION_COMMAND, + } as SuggestionRawDefinition, +}; + +export const suggestForSortCmd = async (innerText: string, getFieldsByType: GetFieldsByTypeFn) => { + const { pos, order, nulls } = getSortPos(innerText); + + switch (pos) { + case 'space2': { + return [ + sortModifierSuggestions.ASC, + sortModifierSuggestions.DESC, + sortModifierSuggestions.NULLS_FIRST, + sortModifierSuggestions.NULLS_LAST, + ...getFinalSuggestions({ + comma: true, + }), + ]; + } + case 'order': { + const suggestions: SuggestionRawDefinition[] = []; + for (const modifier of Object.values(sortModifierSuggestions)) { + if (modifier.label.startsWith(order)) { + suggestions.push(modifier); + } + } + return suggestions; + } + case 'space3': { + return [ + sortModifierSuggestions.NULLS_FIRST, + sortModifierSuggestions.NULLS_LAST, + ...getFinalSuggestions({ + comma: true, + }), + ]; + } + case 'nulls': { + const end = innerText.length + 1; + const start = end - nulls.length; + const suggestions: SuggestionRawDefinition[] = []; + for (const modifier of Object.values(sortModifierSuggestions)) { + if (modifier.label.startsWith(nulls)) { + suggestions.push({ + ...modifier, + rangeToReplace: { + start, + end, + }, + }); + } + } + return suggestions; + } + case 'space4': { + return [ + ...getFinalSuggestions({ + comma: true, + }), + ]; + } + } + + return (await getFieldsByType('any', [], { advanceCursor: true })) as SuggestionRawDefinition[]; +}; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts new file mode 100644 index 0000000000000..82059b6b7765c --- /dev/null +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts @@ -0,0 +1,55 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { getSortPos } from './helper'; + +test('returns correct position on complete modifier matches', () => { + expect(getSortPos('from a | ').pos).toBe('none'); + expect(getSortPos('from a | s').pos).toBe('pre-start'); + expect(getSortPos('from a | so').pos).toBe('pre-start'); + expect(getSortPos('from a | sor').pos).toBe('pre-start'); + expect(getSortPos('from a | sort').pos).toBe('start'); + expect(getSortPos('from a | sort ').pos).toBe('space1'); + expect(getSortPos('from a | sort col').pos).toBe('column'); + expect(getSortPos('from a | sort col ').pos).toBe('space2'); + expect(getSortPos('from a | sort col ASC').pos).toBe('order'); + expect(getSortPos('from a | sort col DESC ').pos).toBe('space3'); + expect(getSortPos('from a | sort col DESC NULLS FIRST').pos).toBe('nulls'); + expect(getSortPos('from a | sort col DESC NULLS LAST ').pos).toBe('space4'); + expect(getSortPos('from a | sort col DESC NULLS LAST, ').pos).toBe('space1'); + expect(getSortPos('from a | sort col DESC NULLS LAST, col2').pos).toBe('column'); + expect(getSortPos('from a | sort col DESC NULLS LAST, col2 DESC').pos).toBe('order'); + expect(getSortPos('from a | sort col DESC NULLS LAST, col2 NULLS LAST').pos).toBe('nulls'); + expect(getSortPos('from a | sort col DESC NULLS LAST, col2 NULLS LAST ').pos).toBe('space4'); +}); + +test('returns ASC/DESC matched text', () => { + expect(getSortPos('from a | sort col ASC').pos).toBe('order'); + expect(getSortPos('from a | sort col asc').order).toBe('ASC'); + + expect(getSortPos('from a | sort col as').pos).toBe('order'); + expect(getSortPos('from a | sort col as').order).toBe('AS'); + + expect(getSortPos('from a | sort col DE').pos).toBe('order'); + expect(getSortPos('from a | sort col DE').order).toBe('DE'); +}); + +test('returns NULLS FIRST/NULLS LAST matched text', () => { + expect(getSortPos('from a | sort col ASC NULLS FIRST').pos).toBe('nulls'); + expect(getSortPos('from a | sort col ASC NULLS FIRST').nulls).toBe('NULLS FIRST'); + + expect(getSortPos('from a | sort col ASC nulls fi').pos).toBe('nulls'); + expect(getSortPos('from a | sort col ASC nulls fi').nulls).toBe('NULLS FI'); + + expect(getSortPos('from a | sort col nul').pos).toBe('nulls'); + expect(getSortPos('from a | sort col nul').nulls).toBe('NUL'); + + expect(getSortPos('from a | sort col1, col2 NULLS LA').pos).toBe('nulls'); + expect(getSortPos('from a | sort col1, col2 NULLS LA').nulls).toBe('NULLS LA'); +}); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts new file mode 100644 index 0000000000000..dfa5ce0f4d5f7 --- /dev/null +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts @@ -0,0 +1,84 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +const regexStart = /.+\|\s*so?r?(?t?)(.+,)?(?\s+)?/i; +const regex = + /.+\|\s*sort(.+,)?((?\s+)(?[^\s]+)(?\s*)(?(AS?C?)|(DE?S?C?))?(?\s*)(?NU?L?L?S? ?(FI?R?S?T?|LA?S?T?)?)?(?\s*))?/i; + +export interface SortCaretPosition { + /** + * Position of the caret in the sort command: + * + * ``` + * SORT [ASC/DESC] [NULLS FIRST/NULLS LAST] + * | | | | | | | | + * | | | | | | | space4 + * | | | | | | nulls + * | | | | | space3 + * | | | | order + * | | | space 2 + * | | | + * | | column + * | start + * pre-start + * ``` + */ + pos: + | 'none' + | 'pre-start' + | 'start' + | 'space1' + | 'column' + | 'space2' + | 'order' + | 'space3' + | 'nulls' + | 'space4'; + order: string; + nulls: string; +} + +export const getSortPos = (query: string): SortCaretPosition => { + const match = query.match(regex); + let pos: SortCaretPosition['pos'] = 'none'; + let order: SortCaretPosition['order'] = ''; + let nulls: SortCaretPosition['nulls'] = ''; + + if (match?.groups?.space4) { + pos = 'space4'; + } else if (match?.groups?.nulls) { + pos = 'nulls'; + nulls = match.groups.nulls.toUpperCase(); + } else if (match?.groups?.space3) { + pos = 'space3'; + } else if (match?.groups?.order) { + pos = 'order'; + order = match.groups.order.toUpperCase(); + } else if (match?.groups?.space2) { + pos = 'space2'; + } else if (match?.groups?.column) { + pos = 'column'; + } else { + const match2 = query.match(regexStart); + + if (match2?.groups?.space1) { + pos = 'space1'; + } else if (match2?.groups?.start) { + pos = 'start'; + } else if (match2) { + pos = 'pre-start'; + } + } + + return { + pos, + order, + nulls, + }; +}; diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts index d2ff04e4d9baa..c59daa2130417 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts @@ -641,6 +641,20 @@ const otherDefinitions: FunctionDefinition[] = [ }, ], }, + { + name: 'order-expression', + type: 'builtin', + description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.infoDoc', { + defaultMessage: 'Specify column sorting modifiers', + }), + supportedCommands: ['sort'], + signatures: [ + { + params: [{ name: 'column', type: 'any' }], + returnType: 'void', + }, + ], + }, ]; export const builtinFunctions: FunctionDefinition[] = [ diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts index 349bfcf4a358a..979e718fb4174 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/commands.ts @@ -383,11 +383,7 @@ export const commandDefinitions: CommandDefinition[] = [ modes: [], signature: { multipleParams: true, - params: [ - { name: 'expression', type: 'any' }, - { name: 'direction', type: 'string', optional: true, values: ['ASC', 'DESC'] }, - { name: 'nulls', type: 'string', optional: true, values: ['NULLS FIRST', 'NULLS LAST'] }, - ], + params: [{ name: 'expression', type: 'any' }], }, }, { diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/context.ts b/packages/kbn-esql-validation-autocomplete/src/shared/context.ts index 22429e1ff9cb7..0f7f830c1417a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/context.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/context.ts @@ -35,7 +35,7 @@ function findNode(nodes: ESQLAstItem[], offset: number): ESQLSingleAstItem | und return ret; } } else { - if (node.location.min <= offset && node.location.max >= offset) { + if (node && node.location && node.location.min <= offset && node.location.max >= offset) { if ('args' in node) { const ret = findNode(node.args, offset); // if the found node is the marker, then return its parent