Skip to content

Commit

Permalink
[8.x] [ES|QL] Add autocomplete and validation to support MATCH and QS…
Browse files Browse the repository at this point in the history
…RT (#199032) (#200127)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Add autocomplete and validation to support MATCH and QSRT
(#199032)](#199032)

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

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

<!--BACKPORT [{"author":{"name":"Quynh Nguyen
(Quinn)","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-14T09:05:36Z","message":"[ES|QL]
Add autocomplete and validation to support MATCH and QSRT
(#199032)\n\n## Summary\r\n\r\nCloses
#196995. This PR
adds\r\nautocomplete and validation to support MATCH and
QSRT\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/f6be7108-cc6c-480f-b7cf-8c7953d06e7a\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0549e044-90d6-4619-a00b-c9a2c8f94c04\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Stratoula Kalafateli
<[email protected]>","sha":"f4af267e0e97348991b9103f325922fb71b07204","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","Feature:ES|QL","Team:ESQL","backport:version","v8.17.0"],"title":"[ES|QL]
Add autocomplete and validation to support MATCH and
QSRT","number":199032,"url":"https://github.com/elastic/kibana/pull/199032","mergeCommit":{"message":"[ES|QL]
Add autocomplete and validation to support MATCH and QSRT
(#199032)\n\n## Summary\r\n\r\nCloses
#196995. This PR
adds\r\nautocomplete and validation to support MATCH and
QSRT\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/f6be7108-cc6c-480f-b7cf-8c7953d06e7a\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0549e044-90d6-4619-a00b-c9a2c8f94c04\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Stratoula Kalafateli
<[email protected]>","sha":"f4af267e0e97348991b9103f325922fb71b07204"}},"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/199032","number":199032,"mergeCommit":{"message":"[ES|QL]
Add autocomplete and validation to support MATCH and QSRT
(#199032)\n\n## Summary\r\n\r\nCloses
#196995. This PR
adds\r\nautocomplete and validation to support MATCH and
QSRT\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/f6be7108-cc6c-480f-b7cf-8c7953d06e7a\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0549e044-90d6-4619-a00b-c9a2c8f94c04\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Stratoula Kalafateli
<[email protected]>","sha":"f4af267e0e97348991b9103f325922fb71b07204"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Quynh Nguyen (Quinn) <[email protected]>
  • Loading branch information
kibanamachine and qn895 authored Nov 14, 2024
1 parent d298669 commit 8085b63
Show file tree
Hide file tree
Showing 14 changed files with 422 additions and 58 deletions.
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))) {
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

0 comments on commit 8085b63

Please sign in to comment.