Skip to content

Commit

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

## Summary

This PR begins the refactor described in
elastic#195418.

The autocomplete engine now delegates to command-specific routines
attached to the command definitions for `KEEP`, `DROP`, and `SORT`.

The naming of `getFieldsFor` has been broadened to `getColumnsFor`
because the response from Elasticsearch can contain variables as well as
fields, depending on the query that is used to fetch the columns.

No user-facing behavior should have changed.

### Checklist

- [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: Elastic Machine <[email protected]>
  • Loading branch information
drewdaemon and elasticmachine authored Oct 29, 2024
1 parent 669dc38 commit 11ae6a5
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 11ae6a5

Please sign in to comment.