Skip to content

Commit

Permalink
[8.x] [ES|QL] separate `KEEP`, `DROP`, and `…
Browse files Browse the repository at this point in the history
…SORT` autocomplete routines (#197744) (#198159)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] separate `KEEP`, `DROP`, and
`SORT` autocomplete routines
(#197744)](#197744)

<!--- 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-10-29T15:50:30Z","message":"[ES|QL]
separate `KEEP`, `DROP`, and `SORT` autocomplete routines
(#197744)\n\n## Summary\r\n\r\nThis PR begins the refactor described
in\r\nhttps://github.com//issues/195418.\r\n\r\nThe
autocomplete engine now delegates to command-specific
routines\r\nattached to the command definitions for `KEEP`, `DROP`, and
`SORT`.\r\n\r\nThe naming of `getFieldsFor` has been broadened to
`getColumnsFor`\r\nbecause the response from Elasticsearch can contain
variables as well as\r\nfields, depending on the query that is used to
fetch the columns.\r\n\r\nNo user-facing behavior should have
changed.\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: Elastic Machine
<[email protected]>","sha":"11ae6a5bd9a06a4402e8af5173b0b0efcf5f52fc","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 `KEEP`, `DROP`, and `SORT` autocomplete
routines","number":197744,"url":"https://github.com/elastic/kibana/pull/197744","mergeCommit":{"message":"[ES|QL]
separate `KEEP`, `DROP`, and `SORT` autocomplete routines
(#197744)\n\n## Summary\r\n\r\nThis PR begins the refactor described
in\r\nhttps://github.com//issues/195418.\r\n\r\nThe
autocomplete engine now delegates to command-specific
routines\r\nattached to the command definitions for `KEEP`, `DROP`, and
`SORT`.\r\n\r\nThe naming of `getFieldsFor` has been broadened to
`getColumnsFor`\r\nbecause the response from Elasticsearch can contain
variables as well as\r\nfields, depending on the query that is used to
fetch the columns.\r\n\r\nNo user-facing behavior should have
changed.\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: Elastic Machine
<[email protected]>","sha":"11ae6a5bd9a06a4402e8af5173b0b0efcf5f52fc"}},"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/197744","number":197744,"mergeCommit":{"message":"[ES|QL]
separate `KEEP`, `DROP`, and `SORT` autocomplete routines
(#197744)\n\n## Summary\r\n\r\nThis PR begins the refactor described
in\r\nhttps://github.com//issues/195418.\r\n\r\nThe
autocomplete engine now delegates to command-specific
routines\r\nattached to the command definitions for `KEEP`, `DROP`, and
`SORT`.\r\n\r\nThe naming of `getFieldsFor` has been broadened to
`getColumnsFor`\r\nbecause the response from Elasticsearch can contain
variables as well as\r\nfields, depending on the query that is used to
fetch the columns.\r\n\r\nNo user-facing behavior should have
changed.\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: Elastic Machine
<[email protected]>","sha":"11ae6a5bd9a06a4402e8af5173b0b0efcf5f52fc"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Drew Tate <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
3 people authored Oct 30, 2024
1 parent dcf5007 commit d4b1e28
Show file tree
Hide file tree
Showing 27 changed files with 760 additions and 473 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-esql-editor/src/esql_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ export const ESQLEditor = memo(function ESQLEditor({
const sources = await memoizedSources(dataViews, core).result;
return sources;
},
getFieldsFor: async ({ query: queryToExecute }: { query?: string } | undefined = {}) => {
getColumnsFor: async ({ query: queryToExecute }: { query?: string } | undefined = {}) => {
if (queryToExecute) {
// ES|QL with limit 0 returns only the columns and is more performant
const esqlQuery = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const policies = [

export function getCallbackMocks() {
return {
getFieldsFor: jest.fn(async ({ query }) => {
getColumnsFor: jest.fn(async ({ query }) => {
if (/enrich/.test(query)) {
return enrichFields;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ describe('autocomplete.suggest', () => {
await suggest('sHoW ?');
await suggest('row ? |');

expect(callbacks.getFieldsFor.mock.calls.length).toBe(0);
expect(callbacks.getColumnsFor.mock.calls.length).toBe(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,17 @@ export function getDateLiteralsByFieldType(_requestedType: FieldType | FieldType
}

export function createCustomCallbackMocks(
customFields?: ESQLRealField[],
/**
* Columns that will come from Elasticsearch since the last command
* e.g. the test case may be `FROM index | EVAL foo = 1 | KEEP /`
*
* In this case, the columns available for the KEEP command will be the ones
* that were available after the EVAL command
*
* `FROM index | EVAL foo = 1 | LIMIT 0` will be used to fetch columns. The response
* will include "foo" as a column.
*/
customColumnsSinceLastCommand?: ESQLRealField[],
customSources?: Array<{ name: string; hidden: boolean }>,
customPolicies?: Array<{
name: string;
Expand All @@ -253,11 +263,11 @@ export function createCustomCallbackMocks(
enrichFields: string[];
}>
) {
const finalFields = customFields || fields;
const finalColumnsSinceLastCommand = customColumnsSinceLastCommand || fields;
const finalSources = customSources || indexes;
const finalPolicies = customPolicies || policies;
return {
getFieldsFor: jest.fn(async () => finalFields),
getColumnsFor: jest.fn(async () => finalColumnsSinceLastCommand),
getSources: jest.fn(async () => finalSources),
getPolicies: jest.fn(async () => finalPolicies),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import {
TIME_PICKER_SUGGESTION,
setup,
attachTriggerCommand,
SuggestOptions,
fields,
} from './__tests__/helpers';
import { METADATA_FIELDS } from '../shared/constants';
import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types';
Expand Down Expand Up @@ -385,24 +387,56 @@ describe('autocomplete', () => {
'```````round(doubleField) + 1```` + 1`` + 1`',
'```````````````round(doubleField) + 1```````` + 1```` + 1`` + 1`',
'```````````````````````````````round(doubleField) + 1```````````````` + 1```````` + 1```` + 1`` + 1`',
],
undefined,
[
[
...fields,
// the following non-field columns will come over the wire as part of the response
{
name: 'round(doubleField) + 1',
type: 'double',
},
{
name: '`round(doubleField) + 1` + 1',
type: 'double',
},
{
name: '```round(doubleField) + 1`` + 1` + 1',
type: 'double',
},
{
name: '```````round(doubleField) + 1```` + 1`` + 1` + 1',
type: 'double',
},
{
name: '```````````````round(doubleField) + 1```````` + 1```` + 1`` + 1` + 1',
type: 'double',
},
],
]
);

it('should not suggest already-used fields and variables', async () => {
const { suggest: suggestTest } = await setup();
const getSuggestions = async (query: string) =>
(await suggestTest(query)).map((value) => value.text);
const getSuggestions = async (query: string, opts?: SuggestOptions) =>
(await suggestTest(query, opts)).map((value) => value.text);

expect(await getSuggestions('from a_index | EVAL foo = 1 | KEEP /')).toContain('foo');
expect(await getSuggestions('from a_index | EVAL foo = 1 | KEEP foo, /')).not.toContain(
'foo'
);
expect(await getSuggestions('from a_index | EVAL foo = 1 | KEEP /')).toContain(
expect(
await getSuggestions('from a_index | EVAL foo = 1 | KEEP /', {
callbacks: { getColumnsFor: () => [...fields, { name: 'foo', type: 'integer' }] },
})
).toContain('foo');
expect(
await getSuggestions('from a_index | EVAL foo = 1 | KEEP foo, /', {
callbacks: { getColumnsFor: () => [...fields, { name: 'foo', type: 'integer' }] },
})
).not.toContain('foo');

expect(await getSuggestions('from a_index | KEEP /')).toContain('doubleField');
expect(await getSuggestions('from a_index | KEEP doubleField, /')).not.toContain(
'doubleField'
);
expect(
await getSuggestions('from a_index | EVAL foo = 1 | KEEP doubleField, /')
).not.toContain('doubleField');
});
});
}
Expand Down Expand Up @@ -504,7 +538,7 @@ describe('autocomplete', () => {
});

describe('callbacks', () => {
it('should send the fields query without the last command', async () => {
it('should send the columns query without the last command', async () => {
const callbackMocks = createCustomCallbackMocks(undefined, undefined, undefined);
const statement = 'from a | drop keywordField | eval var0 = abs(doubleField) ';
const triggerOffset = statement.lastIndexOf(' ');
Expand All @@ -516,7 +550,7 @@ describe('autocomplete', () => {
async (text) => (text ? getAstAndSyntaxErrors(text) : { ast: [], errors: [] }),
callbackMocks
);
expect(callbackMocks.getFieldsFor).toHaveBeenCalledWith({
expect(callbackMocks.getColumnsFor).toHaveBeenCalledWith({
query: 'from a | drop keywordField',
});
});
Expand All @@ -532,7 +566,7 @@ describe('autocomplete', () => {
async (text) => (text ? getAstAndSyntaxErrors(text) : { ast: [], errors: [] }),
callbackMocks
);
expect(callbackMocks.getFieldsFor).toHaveBeenCalledWith({ query: 'from a' });
expect(callbackMocks.getColumnsFor).toHaveBeenCalledWith({ query: 'from a' });
});
});

Expand Down
Loading

0 comments on commit d4b1e28

Please sign in to comment.