Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [ES|QL] Implement `OrderExpression` for `SORT` command arguments (#189959) #193379

Merged
merged 2 commits into from
Sep 23, 2024
Merged

[8.x] [ES|QL] Implement `OrderExpression` for `SORT` command arguments (#189959) #193379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4100c79
[ES|QL] Implement `OrderExpression` for `SORT` command arguments (#18…
vadimkibana Sep 19, 2024
06bcd3e
Merge branch '8.x' into backport/8.x/pr-189959
elasticmachine Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 122 additions & 19 deletions packages/kbn-esql-ast/src/__tests__/ast_parser.sort.test.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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