Skip to content

Commit

Permalink
[8.x] [ES|QL] separate `WHERE` autocomplete routine (#198832)…
Browse files Browse the repository at this point in the history
… (#199595)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] separate `WHERE` autocomplete routine
(#198832)](#198832)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-11T08:41:38Z","message":"[ES|QL]
separate `WHERE` autocomplete routine (#198832)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/195418\r\n\r\n**NOTES**\r\n-
need to make sure these don't regress\r\n -
https://github.com/elastic/kibana/pull/195771\r\n -
https://github.com/elastic/kibana/pull/197139\r\n - suggesting variables
after binary operator (e.g. `field + <suggest>`\r\n- I've noticed that
incomplete null statements such as `is n` are\r\ncorrected in our syntax
tree. This sends the autocomplete down a\r\n\"completed operator
expression\" route as opposed to an unknown operator\r\nor \"to right of
column\" route. So, `... | EVAL foo IS N/` is interpreted\r\nas `... |
EVAL foo IS NULL /`.<br><br>It accidentally works (lol)\r\nbecause the
logic for `<operator-expression> <suggest>` suggests\r\noperators that
accept the return type of the existing operator\r\nexpression as their
left-hand argument. Since `foo IS NULL` is of type\r\n`boolean` and `IS
NULL` accepts boolean values, it gets included in the\r\nsuggestion list
which Monaco then filters by the actual prefix (`IS N`).\r\n🤣
<br><br>([issue](https://github.com/elastic/kibana/issues/199401))\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<[email protected]>","sha":"2466a172af66452bdd939dddc56506faba7ffb7a","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Feature:ES|QL","Team:ESQL","backport:version","v8.17.0"],"title":"[ES|QL]
separate `WHERE` autocomplete
routine","number":198832,"url":"https://github.com/elastic/kibana/pull/198832","mergeCommit":{"message":"[ES|QL]
separate `WHERE` autocomplete routine (#198832)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/195418\r\n\r\n**NOTES**\r\n-
need to make sure these don't regress\r\n -
https://github.com/elastic/kibana/pull/195771\r\n -
https://github.com/elastic/kibana/pull/197139\r\n - suggesting variables
after binary operator (e.g. `field + <suggest>`\r\n- I've noticed that
incomplete null statements such as `is n` are\r\ncorrected in our syntax
tree. This sends the autocomplete down a\r\n\"completed operator
expression\" route as opposed to an unknown operator\r\nor \"to right of
column\" route. So, `... | EVAL foo IS N/` is interpreted\r\nas `... |
EVAL foo IS NULL /`.<br><br>It accidentally works (lol)\r\nbecause the
logic for `<operator-expression> <suggest>` suggests\r\noperators that
accept the return type of the existing operator\r\nexpression as their
left-hand argument. Since `foo IS NULL` is of type\r\n`boolean` and `IS
NULL` accepts boolean values, it gets included in the\r\nsuggestion list
which Monaco then filters by the actual prefix (`IS N`).\r\n🤣
<br><br>([issue](https://github.com/elastic/kibana/issues/199401))\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<[email protected]>","sha":"2466a172af66452bdd939dddc56506faba7ffb7a"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198832","number":198832,"mergeCommit":{"message":"[ES|QL]
separate `WHERE` autocomplete routine (#198832)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/195418\r\n\r\n**NOTES**\r\n-
need to make sure these don't regress\r\n -
https://github.com/elastic/kibana/pull/195771\r\n -
https://github.com/elastic/kibana/pull/197139\r\n - suggesting variables
after binary operator (e.g. `field + <suggest>`\r\n- I've noticed that
incomplete null statements such as `is n` are\r\ncorrected in our syntax
tree. This sends the autocomplete down a\r\n\"completed operator
expression\" route as opposed to an unknown operator\r\nor \"to right of
column\" route. So, `... | EVAL foo IS N/` is interpreted\r\nas `... |
EVAL foo IS NULL /`.<br><br>It accidentally works (lol)\r\nbecause the
logic for `<operator-expression> <suggest>` suggests\r\noperators that
accept the return type of the existing operator\r\nexpression as their
left-hand argument. Since `foo IS NULL` is of type\r\n`boolean` and `IS
NULL` accepts boolean values, it gets included in the\r\nsuggestion list
which Monaco then filters by the actual prefix (`IS N`).\r\n🤣
<br><br>([issue](https://github.com/elastic/kibana/issues/199401))\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<[email protected]>","sha":"2466a172af66452bdd939dddc56506faba7ffb7a"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Drew Tate <[email protected]>
  • Loading branch information
kibanamachine and drewdaemon authored Nov 11, 2024
1 parent 1854055 commit 78c43a7
Show file tree
Hide file tree
Showing 17 changed files with 978 additions and 495 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,334 @@
/*
* 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 { ESQL_COMMON_NUMERIC_TYPES } from '../../shared/esql_types';
import { pipeCompleteItem } from '../complete_items';
import { getDateLiterals } from '../factories';
import { log10ParameterTypes, powParameterTypes } from './constants';
import {
attachTriggerCommand,
fields,
getFieldNamesByType,
getFunctionSignaturesByReturnType,
setup,
} from './helpers';

describe('WHERE <expression>', () => {
const allEvalFns = getFunctionSignaturesByReturnType('where', 'any', {
scalar: true,
});
test('beginning an expression', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a | where /', [
...getFieldNamesByType('any')
.map((field) => `${field} `)
.map(attachTriggerCommand),
...allEvalFns,
]);
await assertSuggestions(
'from a | eval var0 = 1 | where /',
[
...getFieldNamesByType('any')
.map((name) => `${name} `)
.map(attachTriggerCommand),
attachTriggerCommand('var0 '),
...allEvalFns,
],
{
callbacks: {
getColumnsFor: () => Promise.resolve([...fields, { name: 'var0', type: 'integer' }]),
},
}
);
});

describe('within the expression', () => {
test('after a field name', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a | where keywordField /', [
// all functions compatible with a keywordField type
...getFunctionSignaturesByReturnType(
'where',
'boolean',
{
builtin: true,
},
undefined,
['and', 'or', 'not']
),
]);
});

test('suggests dates after a comparison with a date', async () => {
const { assertSuggestions } = await setup();

const expectedComparisonWithDateSuggestions = [
...getDateLiterals(),
...getFieldNamesByType(['date']),
// all functions compatible with a keywordField type
...getFunctionSignaturesByReturnType('where', ['date'], { scalar: true }),
];
await assertSuggestions(
'from a | where dateField == /',
expectedComparisonWithDateSuggestions
);

await assertSuggestions(
'from a | where dateField < /',
expectedComparisonWithDateSuggestions
);

await assertSuggestions(
'from a | where dateField >= /',
expectedComparisonWithDateSuggestions
);
});

test('after a comparison with a string field', async () => {
const { assertSuggestions } = await setup();

const expectedComparisonWithTextFieldSuggestions = [
...getFieldNamesByType(['text', 'keyword', 'ip', 'version']),
...getFunctionSignaturesByReturnType('where', ['text', 'keyword', 'ip', 'version'], {
scalar: true,
}),
];

await assertSuggestions(
'from a | where textField >= /',
expectedComparisonWithTextFieldSuggestions
);
await assertSuggestions(
'from a | where textField >= textField/',
expectedComparisonWithTextFieldSuggestions
);
});

test('after a logical operator', async () => {
const { assertSuggestions } = await setup();

for (const op of ['and', 'or']) {
await assertSuggestions(`from a | where keywordField >= keywordField ${op} /`, [
...getFieldNamesByType('any'),
...getFunctionSignaturesByReturnType('where', 'any', { scalar: true }),
]);
await assertSuggestions(`from a | where keywordField >= keywordField ${op} doubleField /`, [
...getFunctionSignaturesByReturnType('where', 'boolean', { builtin: true }, ['double']),
]);
await assertSuggestions(
`from a | where keywordField >= keywordField ${op} doubleField == /`,
[
...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES),
...getFunctionSignaturesByReturnType('where', ESQL_COMMON_NUMERIC_TYPES, {
scalar: true,
}),
]
);
}
});

test('suggests operators after a field name', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a | stats a=avg(doubleField) | where a /', [
...getFunctionSignaturesByReturnType('where', 'any', { builtin: true, skipAssign: true }, [
'double',
]),
]);
});

test('accounts for fields lost in previous commands', async () => {
const { assertSuggestions } = await setup();

// Mind this test: suggestion is aware of previous commands when checking for fields
// in this case the doubleField has been wiped by the STATS command and suggest cannot find it's type
await assertSuggestions('from a | stats a=avg(doubleField) | where doubleField /', [], {
callbacks: { getColumnsFor: () => Promise.resolve([{ name: 'a', type: 'double' }]) },
});
});

test('suggests function arguments', async () => {
const { assertSuggestions } = await setup();

// The editor automatically inject the final bracket, so it is not useful to test with just open bracket
await assertSuggestions(
'from a | where log10(/)',
[
...getFieldNamesByType(log10ParameterTypes),
...getFunctionSignaturesByReturnType(
'where',
log10ParameterTypes,
{ scalar: true },
undefined,
['log10']
),
],
{ triggerCharacter: '(' }
);
await assertSuggestions(
'from a | WHERE pow(doubleField, /)',
[
...getFieldNamesByType(powParameterTypes),
...getFunctionSignaturesByReturnType(
'where',
powParameterTypes,
{ scalar: true },
undefined,
['pow']
),
],
{ triggerCharacter: ',' }
);
});

test('suggests boolean and numeric operators after a numeric function result', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a | where log10(doubleField) /', [
...getFunctionSignaturesByReturnType('where', 'double', { builtin: true }, ['double']),
...getFunctionSignaturesByReturnType('where', 'boolean', { builtin: true }, ['double']),
]);
});

test('suggestions after NOT', async () => {
const { assertSuggestions } = await setup();
await assertSuggestions('from index | WHERE keywordField not /', [
'LIKE $0',
'RLIKE $0',
'IN $0',
]);
await assertSuggestions('from index | WHERE keywordField NOT /', [
'LIKE $0',
'RLIKE $0',
'IN $0',
]);
await assertSuggestions('from index | WHERE not /', [
...getFieldNamesByType('boolean').map((name) => attachTriggerCommand(`${name} `)),
...getFunctionSignaturesByReturnType('where', 'boolean', { scalar: true }),
]);
await assertSuggestions('FROM index | WHERE NOT ENDS_WITH(keywordField, "foo") /', [
...getFunctionSignaturesByReturnType('where', 'boolean', { builtin: true }, ['boolean']),
pipeCompleteItem,
]);
await assertSuggestions('from index | WHERE keywordField IS NOT/', [
'!= $0',
'== $0',
'AND $0',
'IN $0',
'IS NOT NULL',
'IS NULL',
'NOT',
'OR $0',
'| ',
]);

await assertSuggestions('from index | WHERE keywordField IS NOT /', [
'!= $0',
'== $0',
'AND $0',
'IN $0',
'IS NOT NULL',
'IS NULL',
'NOT',
'OR $0',
'| ',
]);
});

test('suggestions after IN', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from index | WHERE doubleField in /', ['( $0 )']);
await assertSuggestions('from index | WHERE doubleField not in /', ['( $0 )']);
await assertSuggestions(
'from index | WHERE doubleField not in (/)',
[
...getFieldNamesByType('double').filter((name) => name !== 'doubleField'),
...getFunctionSignaturesByReturnType('where', 'double', { scalar: true }),
],
{ triggerCharacter: '(' }
);
await assertSuggestions('from index | WHERE doubleField in ( `any#Char$Field`, /)', [
...getFieldNamesByType('double').filter(
(name) => name !== '`any#Char$Field`' && name !== 'doubleField'
),
...getFunctionSignaturesByReturnType('where', 'double', { scalar: true }),
]);
await assertSuggestions('from index | WHERE doubleField not in ( `any#Char$Field`, /)', [
...getFieldNamesByType('double').filter(
(name) => name !== '`any#Char$Field`' && name !== 'doubleField'
),
...getFunctionSignaturesByReturnType('where', 'double', { scalar: true }),
]);
});

test('suggestions after IS (NOT) NULL', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('FROM index | WHERE tags.keyword IS NULL /', [
'AND $0',
'OR $0',
'| ',
]);

await assertSuggestions('FROM index | WHERE tags.keyword IS NOT NULL /', [
'AND $0',
'OR $0',
'| ',
]);
});

test('suggestions after an arithmetic expression', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('FROM index | WHERE doubleField + doubleField /', [
...getFunctionSignaturesByReturnType('where', 'any', { builtin: true, skipAssign: true }, [
'double',
]),
]);
});

test('pipe suggestion after complete expression', async () => {
const { suggest } = await setup();
expect(await suggest('from index | WHERE doubleField != doubleField /')).toContainEqual(
expect.objectContaining({
label: '|',
})
);
});

test('attaches ranges', async () => {
const { suggest } = await setup();

const suggestions = await suggest('FROM index | WHERE doubleField IS N/');

expect(suggestions).toContainEqual(
expect.objectContaining({
text: 'IS NOT NULL',
rangeToReplace: {
start: 32,
end: 36,
},
})
);

expect(suggestions).toContainEqual(
expect.objectContaining({
text: 'IS NULL',
rangeToReplace: {
start: 32,
end: 36,
},
})
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -535,17 +535,6 @@ describe('autocomplete.suggest', () => {
{ triggerCharacter: ' ' }
);
await assertSuggestions('from a | eval a = 1 year /', [',', '| ', 'IS NOT NULL', 'IS NULL']);
await assertSuggestions('from a | eval a = 1 day + 2 /', [',', '| ']);
await assertSuggestions(
'from a | eval 1 day + 2 /',
[
...dateSuggestions,
...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [
'integer',
]),
],
{ triggerCharacter: ' ' }
);
await assertSuggestions(
'from a | eval var0=date_trunc(/)',
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe('hidden functions', () => {
expect(suggestedFunctions).toContain('VISIBLE_FUNCTION($0)');
expect(suggestedFunctions).not.toContain('HIDDEN_FUNCTION($0)');
});

it('does not suggest hidden agg functions', async () => {
setTestFunctions([
{
Expand Down
Loading

0 comments on commit 78c43a7

Please sign in to comment.