Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ES|QL] Add autocomplete and validation to support MATCH and QSRT #199032

Merged
merged 21 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { join } from 'path';
import _ from 'lodash';
import type { RecursivePartial } from '@kbn/utility-types';
import { FunctionDefinition } from '../src/definitions/types';

import { FULL_TEXT_SEARCH_FUNCTIONS } from '../src/shared/constants';
const aliasTable: Record<string, string[]> = {
to_version: ['to_ver'],
to_unsigned_long: ['to_ul', 'to_ulong'],
Expand Down Expand Up @@ -246,23 +246,40 @@ const convertDateTime = (s: string) => (s === 'datetime' ? 'date' : s);
* @returns
*/
function getFunctionDefinition(ESFunctionDefinition: Record<string, any>): FunctionDefinition {
let supportedCommandsAndOptions: Pick<
FunctionDefinition,
'supportedCommands' | 'supportedOptions'
> =
ESFunctionDefinition.type === 'eval'
? scalarSupportedCommandsAndOptions
: aggregationSupportedCommandsAndOptions;

// MATCH and QSRT has limited supported for where commands only
if (FULL_TEXT_SEARCH_FUNCTIONS.includes(ESFunctionDefinition.name)) {
supportedCommandsAndOptions = {
supportedCommands: ['where'],
supportedOptions: [],
};
}
const ret = {
type: ESFunctionDefinition.type,
name: ESFunctionDefinition.name,
...(ESFunctionDefinition.type === 'eval'
? scalarSupportedCommandsAndOptions
: aggregationSupportedCommandsAndOptions),
...supportedCommandsAndOptions,
description: ESFunctionDefinition.description,
alias: aliasTable[ESFunctionDefinition.name],
ignoreAsSuggestion: ESFunctionDefinition.snapshot_only,
preview: ESFunctionDefinition.preview,
signatures: _.uniqBy(
ESFunctionDefinition.signatures.map((signature: any) => ({
...signature,
params: signature.params.map((param: any) => ({
params: signature.params.map((param: any, idx: number) => ({
...param,
type: convertDateTime(param.type),
description: undefined,
...(idx === 0 && FULL_TEXT_SEARCH_FUNCTIONS.includes(ESFunctionDefinition.name)
? // Default to false. If set to true, this parameter does not accept a function or literal, only fields.
{ fieldsOnly: true }
: {}),
})),
returnType: convertDateTime(signature.returnType),
variadic: undefined, // we don't support variadic property
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('WHERE <expression>', () => {
.map((name) => `${name} `)
.map(attachTriggerCommand),
attachTriggerCommand('var0 '),
...allEvalFns,
...allEvalFns.filter((fn) => fn.label !== 'QSTR'),
],
{
callbacks: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ describe('autocomplete.suggest', () => {
for (const fn of scalarFunctionDefinitions) {
// skip this fn for the moment as it's quite hard to test
// Add match in the text when the autocomplete is ready https://github.com/elastic/kibana/issues/196995
if (!['bucket', 'date_extract', 'date_diff', 'case', 'match'].includes(fn.name)) {
if (!['bucket', 'date_extract', 'date_diff', 'case', 'match', 'qstr'].includes(fn.name)) {
test(`${fn.name}`, async () => {
const testedCases = new Set<string>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export const policies = [
* @returns
*/
export function getFunctionSignaturesByReturnType(
command: string,
command: string | string[],
_expectedReturnType: Readonly<FunctionReturnType | 'any' | Array<FunctionReturnType | 'any'>>,
{
agg,
Expand Down Expand Up @@ -165,12 +165,16 @@ export function getFunctionSignaturesByReturnType(

const deduped = Array.from(new Set(list));

const commands = Array.isArray(command) ? command : [command];
return deduped
.filter(({ signatures, ignoreAsSuggestion, supportedCommands, supportedOptions, name }) => {
if (ignoreAsSuggestion) {
return false;
}
if (!supportedCommands.includes(command) && !supportedOptions?.includes(option || '')) {
if (
!commands.some((c) => supportedCommands.includes(c)) &&
!supportedOptions?.includes(option || '')
) {
return false;
}
const filteredByReturnType = signatures.filter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import { uniq, uniqBy } from 'lodash';
import type {
AstProviderFn,
ESQLAst,
ESQLAstItem,
ESQLCommand,
ESQLCommandOption,
Expand Down Expand Up @@ -151,14 +152,16 @@ export async function suggest(
astProvider: AstProviderFn,
resourceRetriever?: ESQLCallbacks
): Promise<SuggestionRawDefinition[]> {
// Partition out to inner ast / ast context for the latest command
const innerText = fullText.substring(0, offset);

const correctedQuery = correctQuerySyntax(innerText, context);

const { ast } = await astProvider(correctedQuery);

const astContext = getAstContext(innerText, ast, offset);

// But we also need the full ast for the full query
const correctedFullQuery = correctQuerySyntax(fullText, context);
const { ast: fullAst } = await astProvider(correctedFullQuery);

if (astContext.type === 'comment') {
return [];
}
Expand Down Expand Up @@ -216,7 +219,8 @@ export async function suggest(
getFieldsMap,
getPolicies,
getPolicyMetadata,
resourceRetriever?.getPreferences
resourceRetriever?.getPreferences,
fullAst
);
}
if (astContext.type === 'setting') {
Expand Down Expand Up @@ -394,7 +398,8 @@ async function getSuggestionsWithinCommandExpression(
getFieldsMap: GetFieldsMapFn,
getPolicies: GetPoliciesFn,
getPolicyMetadata: GetPolicyMetadataFn,
getPreferences?: () => Promise<{ histogramBarTarget: number } | undefined>
getPreferences?: () => Promise<{ histogramBarTarget: number } | undefined>,
fullAst?: ESQLAst
) {
const commandDef = getCommandDefinition(command.name);

Expand All @@ -413,7 +418,8 @@ async function getSuggestionsWithinCommandExpression(
() => findNewVariable(anyVariables),
(expression: ESQLAstItem | undefined) =>
getExpressionType(expression, references.fields, references.variables),
getPreferences
getPreferences,
fullAst
);
} else {
// The deprecated path.
Expand Down Expand Up @@ -1173,19 +1179,21 @@ async function getFunctionArgsSuggestions(
);

// Functions
suggestions.push(
...getFunctionSuggestions({
command: command.name,
option: option?.name,
returnTypes: canBeBooleanCondition
? ['any']
: (getTypesFromParamDefs(typesToSuggestNext) as string[]),
ignored: fnToIgnore,
}).map((suggestion) => ({
...suggestion,
text: addCommaIf(shouldAddComma, suggestion.text),
}))
);
if (typesToSuggestNext.every((d) => !d.fieldsOnly)) {
suggestions.push(
...getFunctionSuggestions({
command: command.name,
option: option?.name,
returnTypes: canBeBooleanCondition
? ['any']
: (getTypesFromParamDefs(typesToSuggestNext) as string[]),
ignored: fnToIgnore,
}).map((suggestion) => ({
...suggestion,
text: addCommaIf(shouldAddComma, suggestion.text),
}))
);
}
// could also be in stats (bucket) but our autocomplete is not great yet
if (
(getTypesFromParamDefs(typesToSuggestNext).includes('date') &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
type ESQLCommand,
type ESQLSingleAstItem,
type ESQLFunction,
ESQLAst,
} from '@kbn/esql-ast';
import { logicalOperators } from '../../../definitions/builtin';
import { isParameterType, type SupportedDataType } from '../../../definitions/types';
Expand All @@ -27,6 +28,10 @@ import {
import { getOverlapRange, getSuggestionsToRightOfOperatorExpression } from '../../helper';
import { getPosition } from './util';
import { pipeCompleteItem } from '../../complete_items';
import {
UNSUPPORTED_COMMANDS_BEFORE_MATCH,
UNSUPPORTED_COMMANDS_BEFORE_QSTR,
} from '../../../shared/constants';

export async function suggest(
innerText: string,
Expand All @@ -35,7 +40,8 @@ export async function suggest(
_columnExists: (column: string) => boolean,
_getSuggestedVariableName: () => string,
getExpressionType: (expression: ESQLAstItem | undefined) => SupportedDataType | 'unknown',
_getPreferences?: () => Promise<{ histogramBarTarget: number } | undefined>
_getPreferences?: () => Promise<{ histogramBarTarget: number } | undefined>,
fullTextAst?: ESQLAst
): Promise<SuggestionRawDefinition[]> {
const suggestions: SuggestionRawDefinition[] = [];

Expand Down Expand Up @@ -154,11 +160,25 @@ export async function suggest(
break;

case 'empty_expression':
// Don't suggest MATCH or QSTR after unsupported commands
const priorCommands = fullTextAst?.map((a) => a.name) ?? [];
const ignored = [];
if (priorCommands.some((c) => UNSUPPORTED_COMMANDS_BEFORE_MATCH.has(c))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very clean 👍

ignored.push('match');
}
if (priorCommands.some((c) => UNSUPPORTED_COMMANDS_BEFORE_QSTR.has(c))) {
ignored.push('qstr');
}

const columnSuggestions = await getColumnsByType('any', [], {
advanceCursor: true,
openSuggestions: true,
});
suggestions.push(...columnSuggestions, ...getFunctionSuggestions({ command: 'where' }));

suggestions.push(
...columnSuggestions,
...getFunctionSuggestions({ command: 'where', ignored })
);

break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type { CodeActionOptions } from './types';
import type { ESQLRealField } from '../validation/types';
import type { FieldType } from '../definitions/types';
import type { ESQLCallbacks, PartialFieldsMetadataClient } from '../shared/types';
import { FULL_TEXT_SEARCH_FUNCTIONS } from '../shared/constants';

function getCallbackMocks(): jest.Mocked<ESQLCallbacks> {
return {
Expand Down Expand Up @@ -285,6 +286,16 @@ describe('quick fixes logic', () => {
{ relaxOnMissingCallbacks: false },
]) {
for (const fn of getAllFunctions({ type: 'eval' })) {
if (FULL_TEXT_SEARCH_FUNCTIONS.includes(fn.name)) {
testQuickFixes(
`FROM index | WHERE ${BROKEN_PREFIX}${fn.name}()`,
[fn.name].map(toFunctionSignature),
{ equalityCheck: 'include', ...options }
);
}
}
for (const fn of getAllFunctions({ type: 'eval' })) {
if (FULL_TEXT_SEARCH_FUNCTIONS.includes(fn.name)) continue;
// add an A to the function name to make it invalid
testQuickFixes(
`FROM index | EVAL ${BROKEN_PREFIX}${fn.name}()`,
Expand Down Expand Up @@ -313,6 +324,8 @@ describe('quick fixes logic', () => {
);
}
for (const fn of getAllFunctions({ type: 'agg' })) {
if (FULL_TEXT_SEARCH_FUNCTIONS.includes(fn.name)) continue;

// add an A to the function name to make it invalid
testQuickFixes(
`FROM index | STATS ${BROKEN_PREFIX}${fn.name}()`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3259,6 +3259,7 @@ const matchDefinition: FunctionDefinition = {
name: 'field',
type: 'keyword',
optional: false,
fieldsOnly: true,
},
{
name: 'query',
Expand All @@ -3274,6 +3275,7 @@ const matchDefinition: FunctionDefinition = {
name: 'field',
type: 'keyword',
optional: false,
fieldsOnly: true,
},
{
name: 'query',
Expand All @@ -3289,6 +3291,7 @@ const matchDefinition: FunctionDefinition = {
name: 'field',
type: 'text',
optional: false,
fieldsOnly: true,
},
{
name: 'query',
Expand All @@ -3304,6 +3307,7 @@ const matchDefinition: FunctionDefinition = {
name: 'field',
type: 'text',
optional: false,
fieldsOnly: true,
},
{
name: 'query',
Expand All @@ -3314,8 +3318,8 @@ const matchDefinition: FunctionDefinition = {
returnType: 'boolean',
},
],
supportedCommands: ['stats', 'inlinestats', 'metrics', 'eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
supportedCommands: ['where'],
supportedOptions: [],
validate: undefined,
examples: [
'from books \n| where match(author, "Faulkner")\n| keep book_no, author \n| sort book_no \n| limit 5;',
Expand Down Expand Up @@ -5912,6 +5916,7 @@ const qstrDefinition: FunctionDefinition = {
name: 'query',
type: 'keyword',
optional: false,
fieldsOnly: true,
},
],
returnType: 'boolean',
Expand All @@ -5922,13 +5927,14 @@ const qstrDefinition: FunctionDefinition = {
name: 'query',
type: 'text',
optional: false,
fieldsOnly: true,
},
],
returnType: 'boolean',
},
],
supportedCommands: ['stats', 'inlinestats', 'metrics', 'eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
supportedCommands: ['where'],
supportedOptions: [],
validate: undefined,
examples: [
'from books \n| where qstr("author: Faulkner")\n| keep book_no, author \n| sort book_no \n| limit 5;',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type {
ESQLAst,
ESQLAstItem,
ESQLCommand,
ESQLCommandOption,
Expand Down Expand Up @@ -136,6 +137,10 @@ export interface FunctionDefinition {
* though a function can be used to create the value. (e.g. now() for dates or concat() for strings)
*/
constantOnly?: boolean;
/**
* Default to false. If set to true, this parameter does not accept a function or literal, only fields.
*/
fieldsOnly?: boolean;
/**
* if provided this means that the value must be one
* of the options in the array iff the value is a literal.
Expand Down Expand Up @@ -181,7 +186,8 @@ export interface CommandBaseDefinition<CommandName extends string> {
columnExists: (column: string) => boolean,
getSuggestedVariableName: () => string,
getExpressionType: (expression: ESQLAstItem | undefined) => SupportedDataType | 'unknown',
getPreferences?: () => Promise<{ histogramBarTarget: number } | undefined>
getPreferences?: () => Promise<{ histogramBarTarget: number } | undefined>,
fullTextAst?: ESQLAst
) => Promise<SuggestionRawDefinition[]>;
/** @deprecated this property will disappear in the future */
signature: {
Expand Down
16 changes: 16 additions & 0 deletions packages/kbn-esql-validation-autocomplete/src/shared/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,19 @@ export const DOUBLE_BACKTICK = '``';
export const SINGLE_BACKTICK = '`';

export const METADATA_FIELDS = ['_version', '_id', '_index', '_source', '_ignored', '_index_mode'];

export const FULL_TEXT_SEARCH_FUNCTIONS = ['match', 'qstr'];
export const UNSUPPORTED_COMMANDS_BEFORE_QSTR = new Set([
'show',
'row',
'dissect',
'enrich',
'eval',
'grok',
'keep',
'mv_expand',
'rename',
'stats',
'limit',
]);
export const UNSUPPORTED_COMMANDS_BEFORE_MATCH = new Set(['limit']);
Loading