Skip to content

Commit

Permalink
[ES|QL] Do not load fields on every keystroke, when editing just the …
Browse files Browse the repository at this point in the history
…source command (elastic#184423)

## Summary

Fixes elastic#184093
Closes elastic#184417

- Fixes the problem on language tools layer.
- Prevents fields loading on every keystroke, when there is only a
single source command, which does not need the fields (FROM, SHOW, ROW).
It does it in two places:
  - Validation
  - Autocompletion


### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or ### For maintainers
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
vadimkibana and kibanamachine authored May 31, 2024
1 parent 839d611 commit 4397787
Show file tree
Hide file tree
Showing 8 changed files with 201 additions and 61 deletions.
73 changes: 73 additions & 0 deletions packages/kbn-esql-validation-autocomplete/src/__tests__/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* 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 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 or the Server
* Side Public License, v 1.
*/

import { camelCase } from 'lodash';
import { supportedFieldTypes } from '../definitions/types';

export const fields = [
...supportedFieldTypes.map((type) => ({ name: `${camelCase(type)}Field`, type })),
{ name: 'any#Char$Field', type: 'number' },
{ name: 'kubernetes.something.something', type: 'number' },
{ name: '@timestamp', type: 'date' },
];

export const enrichFields = [
{ name: 'otherField', type: 'string' },
{ name: 'yetAnotherField', type: 'number' },
];

// eslint-disable-next-line @typescript-eslint/naming-convention
export const unsupported_field = [{ name: 'unsupported_field', type: 'unsupported' }];

export const indexes = [
'a_index',
'index',
'other_index',
'.secret_index',
'my-index',
'unsupported_index',
];

export const policies = [
{
name: 'policy',
sourceIndices: ['enrich_index'],
matchField: 'otherStringField',
enrichFields: ['otherField', 'yetAnotherField'],
},
{
name: 'policy$',
sourceIndices: ['enrich_index'],
matchField: 'otherStringField',
enrichFields: ['otherField', 'yetAnotherField'],
},
];

export function getCallbackMocks() {
return {
getFieldsFor: jest.fn(async ({ query }) => {
if (/enrich/.test(query)) {
return enrichFields;
}
if (/unsupported_index/.test(query)) {
return unsupported_field;
}
if (/dissect|grok/.test(query)) {
return [{ name: 'firstWord', type: 'string' }];
}
return fields;
}),
getSources: jest.fn(async () =>
indexes.map((name) => ({
name,
hidden: name.startsWith('.'),
}))
),
getPolicies: jest.fn(async () => policies),
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* 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 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 or the Server
* Side Public License, v 1.
*/

import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
import { ESQLCallbacks } from '../shared/types';
import * as autocomplete from './autocomplete';
import { getCallbackMocks } from '../__tests__/helpers';
import { EditorContext } from './types';

const setup = async (caret = '?') => {
if (caret.length !== 1) throw new Error('Caret must be a single character');
const callbacks = getCallbackMocks();
const suggest = async (
query: string,
ctx: EditorContext = {
triggerKind: 0,
},
cb: ESQLCallbacks = callbacks
) => {
const pos = query.indexOf(caret);
if (pos < 0) throw new Error(`User cursor/caret "${caret}" not found in query: ${query}`);
const querySansCaret = query.slice(0, pos) + query.slice(pos + 1);
return await autocomplete.suggest(querySansCaret, pos, ctx, getAstAndSyntaxErrors, cb);
};

return {
callbacks,
suggest,
};
};

describe('autocomplete.suggest', () => {
test('does not load fields when suggesting within a single FROM, SHOW, ROW command', async () => {
const { suggest, callbacks } = await setup();

await suggest('FROM kib, ? |');
await suggest('FROM ?');
await suggest('FROM ? |');
await suggest('sHoW ?');
await suggest('row ? |');

expect(callbacks.getFieldsFor.mock.calls.length).toBe(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import { ESQLCallbacks } from '../shared/types';
import {
getFunctionsToIgnoreForStats,
getParamAtPosition,
getQueryForFields,
isAggFunctionUsedAlready,
} from './helper';
import { FunctionArgSignature } from '../definitions/types';
Expand Down Expand Up @@ -196,7 +197,7 @@ export async function suggest(

const astContext = getAstContext(innerText, ast, offset);
// build the correct query to fetch the list of fields
const queryForFields = buildQueryUntilPreviousCommand(ast, finalText);
const queryForFields = getQueryForFields(buildQueryUntilPreviousCommand(ast, finalText), ast);
const { getFieldsByType, getFieldsMap } = getFieldsByTypeRetriever(
queryForFields,
resourceRetriever
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,11 @@ export function getParamAtPosition(
) {
return params.length > position ? params[position] : minParams ? params[params.length - 1] : null;
}

export function getQueryForFields(queryString: string, commands: ESQLCommand[]) {
// If there is only one source command and it does not require fields, do not
// fetch fields, hence return an empty string.
return commands.length === 1 && ['from', 'row', 'show'].includes(commands[0].name)
? ''
: queryString;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function buildQueryUntilPreviousCommand(ast: ESQLAst, queryString: string
export function getFieldsByTypeHelper(queryText: string, resourceRetriever?: ESQLCallbacks) {
const cacheFields = new Map<string, ESQLRealField>();
const getFields = async () => {
if (!cacheFields.size) {
if (!cacheFields.size && queryText) {
const fieldsOfType = await resourceRetriever?.getFieldsFor?.({ query: queryText });
for (const field of fieldsOfType || []) {
cacheFields.set(field.name, field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ export async function retrieveFields(
if (!callbacks || commands.length < 1) {
return new Map();
}
// Do not fetch fields, if query has only one source command and that command
// does not require fields.
if (commands.length === 1) {
switch (commands[0].name) {
case 'from':
case 'show':
case 'row': {
return new Map();
}
}
}
if (commands[0].name === 'row') {
return new Map();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* 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 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 or the Server
* Side Public License, v 1.
*/

import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
import { ESQLCallbacks } from '../shared/types';
import { ValidationOptions } from './types';
import { validateQuery } from './validation';
import { getCallbackMocks } from '../__tests__/helpers';

const setup = async () => {
const callbacks = getCallbackMocks();
const validate = async (
query: string,
opts: ValidationOptions = {},
cb: ESQLCallbacks = callbacks
) => {
return await validateQuery(query, getAstAndSyntaxErrors, opts, cb);
};

return {
callbacks,
validate,
};
};

test('does not load fields when validating only a single FROM, SHOW, ROW command', async () => {
const { validate, callbacks } = await setup();

await validate('FROM kib');
await validate('FROM kibana_ecommerce METADATA _i');
await validate('FROM kibana_ecommerce METADATA _id | ');
await validate('SHOW');
await validate('ROW \t');

expect(callbacks.getFieldsFor.mock.calls.length).toBe(0);
});

test('loads fields with FROM source when commands after pipe present', async () => {
const { validate, callbacks } = await setup();

await validate('FROM kibana_ecommerce METADATA _id | eval');

expect(callbacks.getFieldsFor.mock.calls.length).toBe(1);
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,71 +20,20 @@ import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
import { nonNullable } from '../shared/helpers';
import { METADATA_FIELDS } from '../shared/constants';
import { FUNCTION_DESCRIBE_BLOCK_NAME } from './function_describe_block_name';

const fields = [
...supportedFieldTypes.map((type) => ({ name: `${camelCase(type)}Field`, type })),
{ name: 'any#Char$Field', type: 'number' },
{ name: 'kubernetes.something.something', type: 'number' },
{ name: '@timestamp', type: 'date' },
];
const enrichFields = [
{ name: 'otherField', type: 'string' },
{ name: 'yetAnotherField', type: 'number' },
];
// eslint-disable-next-line @typescript-eslint/naming-convention
const unsupported_field = [{ name: 'unsupported_field', type: 'unsupported' }];
const indexes = [
'a_index',
'index',
'other_index',
'.secret_index',
'my-index',
'unsupported_index',
];
const policies = [
{
name: 'policy',
sourceIndices: ['enrich_index'],
matchField: 'otherStringField',
enrichFields: ['otherField', 'yetAnotherField'],
},
{
name: 'policy$',
sourceIndices: ['enrich_index'],
matchField: 'otherStringField',
enrichFields: ['otherField', 'yetAnotherField'],
},
];
import {
fields,
enrichFields,
getCallbackMocks,
indexes,
policies,
unsupported_field,
} from '../__tests__/helpers';

const NESTING_LEVELS = 4;
const NESTED_DEPTHS = Array(NESTING_LEVELS)
.fill(0)
.map((_, i) => i + 1);

function getCallbackMocks() {
return {
getFieldsFor: jest.fn(async ({ query }) => {
if (/enrich/.test(query)) {
return enrichFields;
}
if (/unsupported_index/.test(query)) {
return unsupported_field;
}
if (/dissect|grok/.test(query)) {
return [{ name: 'firstWord', type: 'string' }];
}
return fields;
}),
getSources: jest.fn(async () =>
indexes.map((name) => ({
name,
hidden: name.startsWith('.'),
}))
),
getPolicies: jest.fn(async () => policies),
};
}

const toInteger = evalFunctionDefinitions.find(({ name }) => name === 'to_integer')!;
const toStringSignature = evalFunctionDefinitions.find(({ name }) => name === 'to_string')!;
const toDateSignature = evalFunctionDefinitions.find(({ name }) => name === 'to_datetime')!;
Expand Down

0 comments on commit 4397787

Please sign in to comment.