[ES|QL] Sorting accepts expressions (#181916)

## Summary

`SORT` accepts expressions as of
https://github.com/elastic/elasticsearch/pull/107158.

This PR updates the validator and autocompletion engine to account for
this improvement.

### Checklist

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added — looks like docs haven't been added for this feature yet. I
opened the discussion with ES team
([ref](https://github.com/elastic/elasticsearch/pull/107158#issuecomment-2080063533))
- [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 <efstratia.kalafateli@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Drew Tate 2024-05-01 11:32:37 -06:00 committed by GitHub
parent a1b990ddd6
commit 5ba6a399f2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 574 additions and 11 deletions

View file

@ -559,7 +559,10 @@ describe('autocomplete', () => {
}
describe('sort', () => {
testSuggestions('from a | sort ', getFieldNamesByType('any'));
testSuggestions('from a | sort ', [
...getFieldNamesByType('any'),
...getFunctionSignaturesByReturnType('sort', 'any', { evalMath: true }),
]);
testSuggestions('from a | sort stringField ', ['asc', 'desc', ',', '|']);
testSuggestions('from a | sort stringField desc ', ['nulls first', 'nulls last', ',', '|']);
// @TODO: improve here

View file

@ -594,7 +594,11 @@ async function getExpressionSuggestionsByType(
option?.name,
getFieldsByType,
{
functions: canHaveAssignments,
// TODO instead of relying on canHaveAssignments and other command name checks
// we should have a more generic way to determine if a command can have functions.
// I think it comes down to the definition of 'column' since 'any' should always
// include functions.
functions: canHaveAssignments || command.name === 'sort',
fields: !argDef.constantOnly,
variables: anyVariables,
literals: argDef.constantOnly,

View file

@ -19,7 +19,7 @@ function createMathDefinition(
type: 'builtin',
name,
description,
supportedCommands: ['eval', 'where', 'row', 'stats'],
supportedCommands: ['eval', 'where', 'row', 'stats', 'sort'],
supportedOptions: ['by'],
signatures: types.map((type) => {
if (Array.isArray(type)) {
@ -59,7 +59,7 @@ function createComparisonDefinition(
type: 'builtin' as const,
name,
description,
supportedCommands: ['eval', 'where', 'row'],
supportedCommands: ['eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
validate,
signatures: [
@ -296,7 +296,7 @@ export const builtinFunctions: FunctionDefinition[] = [
ignoreAsSuggestion: /not/.test(name),
name,
description,
supportedCommands: ['eval', 'where', 'row'],
supportedCommands: ['eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
signatures: [
{
@ -322,7 +322,7 @@ export const builtinFunctions: FunctionDefinition[] = [
ignoreAsSuggestion: /not/.test(name),
name,
description,
supportedCommands: ['eval', 'where', 'row'],
supportedCommands: ['eval', 'where', 'row', 'sort'],
signatures: [
{
params: [
@ -371,7 +371,7 @@ export const builtinFunctions: FunctionDefinition[] = [
type: 'builtin' as const,
name,
description,
supportedCommands: ['eval', 'where', 'row'],
supportedCommands: ['eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
signatures: [
{
@ -410,7 +410,7 @@ export const builtinFunctions: FunctionDefinition[] = [
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.notDoc', {
defaultMessage: 'Not',
}),
supportedCommands: ['eval', 'where', 'row'],
supportedCommands: ['eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
signatures: [
{
@ -436,7 +436,7 @@ export const builtinFunctions: FunctionDefinition[] = [
type: 'builtin',
name,
description,
supportedCommands: ['eval', 'where', 'row'],
supportedCommands: ['eval', 'where', 'row', 'sort'],
signatures: [
{
params: [{ name: 'left', type: 'any' }],

View file

@ -330,13 +330,14 @@ export const commandDefinitions: CommandDefinition[] = [
'… | sort a desc, b nulls last, c asc nulls first',
'… | sort b nulls last',
'… | sort c asc nulls first',
'… | sort a - abs(b)',
],
options: [],
modes: [],
signature: {
multipleParams: true,
params: [
{ name: 'column', type: 'column' },
{ name: 'expression', type: 'any' },
{ name: 'direction', type: 'string', optional: true, values: ['asc', 'desc'] },
{ name: 'nulls', type: 'string', optional: true, values: ['nulls first', 'nulls last'] },
],

View file

@ -1692,7 +1692,7 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [
.sort(({ name: a }, { name: b }) => a.localeCompare(b))
.map((def) => ({
...def,
supportedCommands: ['stats', 'eval', 'where', 'row'],
supportedCommands: ['stats', 'eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
type: 'eval',
}));

View file

@ -16092,6 +16092,509 @@
"error": [],
"warning": []
},
{
"query": "from a_index | sort abs(numberField) - to_long(stringField) desc nulls first",
"error": [],
"warning": []
},
{
"query": "from a_index | sort avg(numberField)",
"error": [
"SORT does not support function avg"
],
"warning": []
},
{
"query": "from a_index | sort sum(numberField)",
"error": [
"SORT does not support function sum"
],
"warning": []
},
{
"query": "from a_index | sort median(numberField)",
"error": [
"SORT does not support function median"
],
"warning": []
},
{
"query": "from a_index | sort median_absolute_deviation(numberField)",
"error": [
"SORT does not support function median_absolute_deviation"
],
"warning": []
},
{
"query": "from a_index | sort percentile(numberField, 5)",
"error": [
"SORT does not support function percentile"
],
"warning": []
},
{
"query": "from a_index | sort max(numberField)",
"error": [
"SORT does not support function max"
],
"warning": []
},
{
"query": "from a_index | sort min(numberField)",
"error": [
"SORT does not support function min"
],
"warning": []
},
{
"query": "from a_index | sort count(stringField)",
"error": [
"SORT does not support function count"
],
"warning": []
},
{
"query": "from a_index | sort count_distinct(stringField, numberField)",
"error": [
"SORT does not support function count_distinct"
],
"warning": []
},
{
"query": "from a_index | sort st_centroid_agg(cartesianPointField)",
"error": [
"SORT does not support function st_centroid_agg"
],
"warning": []
},
{
"query": "from a_index | sort values(stringField)",
"error": [
"SORT does not support function values"
],
"warning": []
},
{
"query": "from a_index | sort bucket(dateField, 1 year)",
"error": [
"SORT does not support function bucket"
],
"warning": []
},
{
"query": "from a_index | sort abs(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort acos(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort asin(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort atan(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort atan2(numberField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort case(booleanField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort ceil(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort cidr_match(ipField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort coalesce(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort concat(stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort cos(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort cosh(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort date_extract(\"ALIGNED_DAY_OF_WEEK_IN_MONTH\", dateField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort date_format(dateField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort date_parse(stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort date_trunc(1 year, dateField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort e()",
"error": [],
"warning": []
},
{
"query": "from a_index | sort ends_with(stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort floor(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort greatest(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort least(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort left(stringField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort length(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort log(numberField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort log10(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort ltrim(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_avg(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_concat(stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_count(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_dedupe(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_first(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_last(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_max(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_median(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_min(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_slice(stringField, numberField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_sort(stringField, \"asc\")",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_sum(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort mv_zip(stringField, stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort now()",
"error": [],
"warning": []
},
{
"query": "from a_index | sort pi()",
"error": [],
"warning": []
},
{
"query": "from a_index | sort pow(numberField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort replace(stringField, stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort right(stringField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort round(numberField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort rtrim(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort signum(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort sin(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort sinh(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort split(stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort sqrt(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort st_contains(geoPointField, geoPointField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort st_disjoint(geoPointField, geoPointField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort st_intersects(geoPointField, geoPointField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort st_within(geoPointField, geoPointField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort st_x(geoPointField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort st_y(geoPointField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort starts_with(stringField, stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort substring(stringField, numberField, numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort tan(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort tanh(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort tau()",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_boolean(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_cartesianpoint(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_cartesianshape(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_datetime(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_degrees(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_double(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_geopoint(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_geoshape(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_integer(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_ip(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_long(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_lower(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_radians(numberField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_string(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_unsigned_long(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_upper(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort to_version(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort trim(stringField)",
"error": [],
"warning": []
},
{
"query": "from a_index | sort sin(stringField)",
"error": [
"Argument of [sin] must be [number], found value [stringField] type [string]"
],
"warning": []
},
{
"query": "from a_index | sort numberField + stringField",
"error": [
"Argument of [+] must be [number], found value [stringField] type [string]"
],
"warning": []
},
{
"query": "from a_index | enrich",
"error": [

View file

@ -2455,6 +2455,58 @@ describe('validation logic', () => {
}
testErrorsAndWarnings(`row a = 1 | stats COUNT(*) | sort \`COUNT(*)\``, []);
testErrorsAndWarnings(`ROW a = 1 | STATS couNt(*) | SORT \`couNt(*)\``, []);
describe('sorting by expressions', () => {
// SORT accepts complex expressions
testErrorsAndWarnings(
'from a_index | sort abs(numberField) - to_long(stringField) desc nulls first',
[]
);
// SORT doesn't accept agg or grouping functions
for (const definition of [
...statsAggregationFunctionDefinitions,
...groupingFunctionDefinitions,
]) {
const {
name,
signatures: [firstSignature],
} = definition;
const fieldMapping = getFieldMapping(firstSignature.params);
const printedInvocation = getFunctionSignatures(
{ ...definition, signatures: [{ ...firstSignature, params: fieldMapping }] },
{ withTypes: false }
)[0].declaration;
testErrorsAndWarnings(`from a_index | sort ${printedInvocation}`, [
`SORT does not support function ${name}`,
]);
}
// But does accept eval functions
for (const definition of evalFunctionsDefinitions) {
const {
signatures: [firstSignature],
} = definition;
const fieldMapping = getFieldMapping(firstSignature.params);
const printedInvocation = getFunctionSignatures(
{ ...definition, signatures: [{ ...firstSignature, params: fieldMapping }] },
{ withTypes: false }
)[0].declaration;
testErrorsAndWarnings(`from a_index | sort ${printedInvocation}`, []);
}
// Expression parts are also validated
testErrorsAndWarnings('from a_index | sort sin(stringField)', [
'Argument of [sin] must be [number], found value [stringField] type [string]',
]);
// Expression parts are also validated
testErrorsAndWarnings('from a_index | sort numberField + stringField', [
'Argument of [+] must be [number], found value [stringField] type [string]',
]);
});
});
describe('enrich', () => {