Skip to content

Commit

Permalink
[ES|QL] Implement OrderExpression for SORT command arguments (ela…
Browse files Browse the repository at this point in the history 
…stic#189959)

## Summary

Closes elastic#189491

- Adds *order expression* AST nodes, which are minted from `SORT`
command.
- Improves SORT command autocomplete suggestions.

Shows fields on first space:

<img width="791" alt="image"
src="https://github.com/user-attachments/assets/3fec96b4-4e61-4212-a856-ace7a33d9755">


It now shows `NULLS FIRST` and `NULLS LAST`, even before `ASC` or `DESC`
was entered, as `ASC` and `DESC` are optional:

<img width="871" alt="image"
src="https://github.com/user-attachments/assets/4b6d6c28-a7b0-4ac0-bafc-133df1207d54">

Once `ASC` or `DESC` is entered, shows only nulls options:

<img width="911" alt="image"
src="https://github.com/user-attachments/assets/5b27bd3d-ccdc-4bd0-b09f-fe65e5975e28">

It also now suggests partial modifier, if the in-progress text that user
is typing matches it:

<img width="504" alt="image"
src="https://github.com/user-attachments/assets/9a047c40-b49b-4694-8477-7270cb9c0886">

(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 <elasticmachine@users.noreply.github.com>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@vadimkibana @elasticmachine @kibanamachine
3 people authored Sep 19, 2024
1 parent 6f4be61 commit 2efd0f0
Show file tree
Hide file tree
Showing 18 changed files with 616 additions and 136 deletions.
141 changes: 122 additions & 19 deletions packages/kbn-esql-ast/src/__tests__/ast_parser.sort.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,107 @@ 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);

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);
Expand All @@ -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',
},
],
},
],
},
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-esql-ast/src/ast_factory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import {
visitDissect,
visitGrok,
collectBooleanExpression,
visitOrderExpression,
visitOrderExpressions,
getPolicyName,
getMatchField,
getEnrichClauses,
Expand Down Expand Up @@ -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()));
}

/**
Expand Down
21 changes: 21 additions & 0 deletions packages/kbn-esql-ast/src/ast_helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import type {
ESQLNumericLiteralType,
FunctionSubtype,
ESQLNumericLiteral,
ESQLOrderExpression,
} from './types';
import { parseIdentifier } from './parser/helpers';

Expand Down Expand Up @@ -222,6 +223,26 @@ export function createFunction<Subtype extends FunctionSubtype>(
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,
Expand Down
63 changes: 37 additions & 26 deletions packages/kbn-esql-ast/src/ast_walker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import {
textExistsAndIsValid,
createInlineCast,
createUnknownItem,
createOrderExpression,
} from './ast_helpers';
import { getPosition } from './ast_position_utils';
import {
Expand All @@ -97,6 +98,7 @@ import {
ESQLUnnamedParamLiteral,
ESQLPositionalParamLiteral,
ESQLNamedParamLiteral,
ESQLOrderExpression,
} from './types';

export function collectAllSourceIdentifiers(ctx: FromCommandContext): ESQLAstItem[] {
Expand Down Expand Up @@ -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 !== '<first missing>') {
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<ESQLOrderExpression | ESQLAstItem> {
const ast: Array<ESQLOrderExpression | ESQLAstItem> = [];

for (const orderCtx of ctx) {
ast.push(visitOrderExpression(orderCtx));
}

return ast;
}

Expand Down
16 changes: 10 additions & 6 deletions packages/kbn-esql-ast/src/pretty_print/__tests__/basic_pretty_printer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});

Expand Down
Loading

0 comments on commit 2efd0f0

Please sign in to comment.