Skip to content

Commit

Permalink
[ES|QL] Separate FROM autocomplete routine (elastic#210465)
Browse files Browse the repository at this point in the history
## Summary

Part of elastic#195418

Gives `FROM` and `METADATA` autocomplete logic its own home 🏡

### 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

### Identify risks

- [ ] As with any refactor, there's a possibility this will introduce a
regression in the behavior of FROM. However, all automated tests are
passing and I have tested the behavior manually and can detect no
regression.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 201dfdd)
  • Loading branch information
drewdaemon committed Feb 13, 2025
1 parent 532304a commit d5e3e74
Show file tree
Hide file tree
Showing 16 changed files with 477 additions and 276 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@ describe('autocomplete.suggest', () => {
await assertSuggestions('from /index', visibleIndices);
});

test('suggests visible indices on comma', async () => {
test("doesn't create suggestions after an open quote", async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('FROM a,/', visibleIndices);
await assertSuggestions('FROM a, /', visibleIndices);
await assertSuggestions('from *,/', visibleIndices);
await assertSuggestions('FROM " /"', []);
});

test('can suggest integration data sources', async () => {
Expand All @@ -72,7 +70,7 @@ describe('autocomplete.suggest', () => {
describe('... METADATA <fields>', () => {
const metadataFieldsAndIndex = metadataFields.filter((field) => field !== '_index');

test('on <kbd>SPACE</kbd> without comma ",", suggests adding metadata', async () => {
test('on <// FROM something METADATA field1, /kbd>SPACE</kbd> without comma ",", suggests adding metadata', async () => {
const recommendedQueries = getRecommendedQueries({
fromCommand: '',
timeField: 'dateField',
Expand All @@ -88,12 +86,31 @@ describe('autocomplete.suggest', () => {
await assertSuggestions('from a, b /', expected);
});

test('partially-typed METADATA keyword', async () => {
const { assertSuggestions } = await setup();

assertSuggestions('FROM index1 MET/', ['METADATA $0']);
});

test('not before first index', async () => {
const { assertSuggestions } = await setup();

assertSuggestions('FROM MET/', visibleIndices);
});

test('on <kbd>SPACE</kbd> after "METADATA" keyword suggests all metadata fields', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a, b METADATA /', metadataFields);
});

test('metadata field prefixes', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a, b METADATA _/', metadataFields);
await assertSuggestions('from a, b METADATA _sour/', metadataFields);
});

test('on <kbd>SPACE</kbd> after "METADATA" column suggests command and pipe operators', async () => {
const { assertSuggestions } = await setup();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,10 @@ describe('autocomplete', () => {

// @TODO: get updated eval block from main
describe('values suggestions', () => {
testSuggestions('FROM "i/"', ['index'], undefined, [, [{ name: 'index', hidden: false }]]);
testSuggestions('FROM "index/"', ['index'], undefined, [, [{ name: 'index', hidden: false }]]);
// TODO — re-enable these tests when we can support this case
testSuggestions.skip('FROM " a/"', []);
testSuggestions.skip('FROM "foo b/"', []);
testSuggestions('FROM "i/"', []);
testSuggestions('FROM "index/"', []);
testSuggestions('FROM " a/"', []);
testSuggestions('FROM "foo b/"', []);
testSuggestions('FROM a | WHERE tags == " /"', [], ' ');
testSuggestions('FROM a | WHERE tags == """ /"""', [], ' ');
testSuggestions('FROM a | WHERE tags == "a/"', []);
Expand Down Expand Up @@ -497,12 +496,7 @@ describe('autocomplete', () => {

// FROM source METADATA
recommendedQuerySuggestions = getRecommendedQueriesSuggestions('', 'dateField');
testSuggestions('FROM index1 M/', [
',',
'METADATA $0',
'| ',
...recommendedQuerySuggestions.map((q) => q.queryString),
]);
testSuggestions('FROM index1 M/', ['METADATA $0']);

// FROM source METADATA field
testSuggestions('FROM index1 METADATA _/', METADATA_FIELDS);
Expand Down Expand Up @@ -890,12 +884,7 @@ describe('autocomplete', () => {

recommendedQuerySuggestions = getRecommendedQueriesSuggestions('', 'dateField');
// FROM source METADATA
testSuggestions('FROM index1 M/', [
',',
attachAsSnippet(attachTriggerCommand('METADATA $0')),
'| ',
...recommendedQuerySuggestions.map((q) => q.queryString),
]);
testSuggestions('FROM index1 M/', [attachAsSnippet(attachTriggerCommand('METADATA $0'))]);

describe('ENRICH', () => {
testSuggestions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
*/

import { uniq, uniqBy } from 'lodash';
import type {
AstProviderFn,
ESQLAstItem,
ESQLCommand,
ESQLCommandOption,
ESQLFunction,
ESQLSingleAstItem,
import {
type AstProviderFn,
type ESQLAstItem,
type ESQLCommand,
type ESQLCommandOption,
type ESQLFunction,
type ESQLSingleAstItem,
} from '@kbn/esql-ast';
import { ESQL_NUMBER_TYPES, isNumericType } from '../shared/esql_types';
import type { EditorContext, ItemKind, SuggestionRawDefinition, GetColumnsByTypeFn } from './types';
Expand Down Expand Up @@ -44,7 +44,6 @@ import {
noCaseCompare,
correctQuerySyntax,
getColumnByName,
sourceExists,
findFinalWord,
getAllCommands,
getExpressionType,
Expand All @@ -63,7 +62,6 @@ import {
import {
buildFieldsDefinitions,
buildPoliciesDefinitions,
buildSourcesDefinitions,
getNewVariableSuggestion,
buildNoPoliciesAvailableDefinition,
getFunctionSuggestions,
Expand All @@ -80,7 +78,7 @@ import {
getOperatorSuggestions,
getSuggestionsAfterNot,
} from './factories';
import { EDITOR_MARKER, FULL_TEXT_SEARCH_FUNCTIONS, METADATA_FIELDS } from '../shared/constants';
import { EDITOR_MARKER, FULL_TEXT_SEARCH_FUNCTIONS } from '../shared/constants';
import { getAstContext, removeMarkerArgFromArgsList } from '../shared/context';
import {
buildQueryUntilPreviousCommand,
Expand All @@ -99,7 +97,6 @@ import {
getQueryForFields,
getSourcesFromCommands,
isAggFunctionUsedAlready,
removeQuoteForSuggestedSources,
getValidSignaturesAndTypesToSuggestNext,
handleFragment,
getFieldsOrFunctionsSuggestions,
Expand All @@ -109,7 +106,6 @@ import {
checkFunctionInvocationComplete,
} from './helper';
import { FunctionParameter, isParameterType } from '../definitions/types';
import { metadataOption } from '../definitions/options';
import { comparisonFunctions } from '../definitions/builtin';
import { getRecommendedQueriesSuggestions } from './recommended_queries/suggestions';

Expand Down Expand Up @@ -213,7 +209,8 @@ export async function suggest(

if (
astContext.type === 'expression' ||
(astContext.type === 'option' && astContext.command?.name === 'join')
(astContext.type === 'option' && astContext.command?.name === 'join') ||
(astContext.type === 'option' && astContext.command?.name === 'from')
) {
return getSuggestionsWithinCommandExpression(
innerText,
Expand Down Expand Up @@ -313,17 +310,6 @@ function getPolicyRetriever(resourceRetriever?: ESQLCallbacks) {
};
}

function getSourceSuggestions(sources: ESQLSourceResult[]) {
// hide indexes that start with .
return buildSourcesDefinitions(
sources
.filter(({ hidden }) => !hidden)
.map(({ name, dataStreams, title, type }) => {
return { name, isIntegration: Boolean(dataStreams && dataStreams.length), title, type };
})
);
}

function findNewVariable(variables: Map<string, ESQLVariable[]>) {
let autoGeneratedVariableCounter = 0;
let name = `var${autoGeneratedVariableCounter++}`;
Expand Down Expand Up @@ -422,19 +408,23 @@ async function getSuggestionsWithinCommandExpression(
const references = { fields: fieldsMap, variables: anyVariables };
if (commandDef.suggest) {
// The new path.
return commandDef.suggest(
return commandDef.suggest({
innerText,
command,
getColumnsByType,
(col: string) => Boolean(getColumnByName(col, references)),
() => findNewVariable(anyVariables),
(expression: ESQLAstItem | undefined) =>
columnExists: (col: string) => Boolean(getColumnByName(col, references)),
getSuggestedVariableName: () => findNewVariable(anyVariables),
getExpressionType: (expression: ESQLAstItem | undefined) =>
getExpressionType(expression, references.fields, references.variables),
getPreferences,
commands,
commandDef,
callbacks
);
definition: commandDef,
getSources,
getRecommendedQueriesSuggestions: (prefix) =>
getRecommendedQueriesSuggestions(getColumnsByType, prefix),
getSourcesFromQuery: (type) => getSourcesFromCommands(commands, type),
previousCommands: commands,
callbacks,
});
} else {
// The deprecated path.
return getExpressionSuggestionsByType(
Expand Down Expand Up @@ -484,12 +474,6 @@ async function getExpressionSuggestionsByType(
return [];
}

// TODO - this is a workaround because it was too difficult to handle this case in a generic way :(
if (commandDef.name === 'from' && node && isSourceItem(node) && /\s/.test(node.name)) {
// FROM " <suggest>"
return [];
}

// A new expression is considered either
// * just after a command name => i.e. ... | STATS <here>
// * or after a comma => i.e. STATS fieldA, <here>
Expand Down Expand Up @@ -522,7 +506,6 @@ async function getExpressionSuggestionsByType(
const optArg = optionsAlreadyDeclared.find(({ name: optionName }) => optionName === name);
return (!optArg && !optionsAlreadyDeclared.length) || (optArg && index > optArg.index);
});
const hasRecommendedQueries = Boolean(commandDef?.hasRecommendedQueries);
// get the next definition for the given command
let argDef = commandDef.signature.params[argIndex];
// tune it for the variadic case
Expand Down Expand Up @@ -903,82 +886,6 @@ async function getExpressionSuggestionsByType(
});
}
suggestions.push(...(policies.length ? policies : [buildNoPoliciesAvailableDefinition()]));
} else {
const indexes = getSourcesFromCommands(commands, 'index');
const lastIndex = indexes[indexes.length - 1];
const canRemoveQuote = isNewExpression && innerText.includes('"');
// Function to add suggestions based on canRemoveQuote
const addSuggestionsBasedOnQuote = async (definitions: SuggestionRawDefinition[]) => {
suggestions.push(
...(canRemoveQuote ? removeQuoteForSuggestedSources(definitions) : definitions)
);
};

if (lastIndex && lastIndex.text && lastIndex.text !== EDITOR_MARKER) {
const sources = await getSources();

const recommendedQueriesSuggestions = hasRecommendedQueries
? await getRecommendedQueriesSuggestions(getFieldsByType)
: [];

const suggestionsToAdd = await handleFragment(
innerText,
(fragment) =>
sourceExists(fragment, new Set(sources.map(({ name: sourceName }) => sourceName))),
(_fragment, rangeToReplace) => {
return getSourceSuggestions(sources).map((suggestion) => ({
...suggestion,
rangeToReplace,
}));
},
(fragment, rangeToReplace) => {
const exactMatch = sources.find(({ name: _name }) => _name === fragment);
if (exactMatch?.dataStreams) {
// this is an integration name, suggest the datastreams
const definitions = buildSourcesDefinitions(
exactMatch.dataStreams.map(({ name }) => ({ name, isIntegration: false }))
);

return canRemoveQuote ? removeQuoteForSuggestedSources(definitions) : definitions;
} else {
const _suggestions: SuggestionRawDefinition[] = [
{
...pipeCompleteItem,
filterText: fragment,
text: fragment + ' | ',
command: TRIGGER_SUGGESTION_COMMAND,
rangeToReplace,
},
{
...commaCompleteItem,
filterText: fragment,
text: fragment + ', ',
command: TRIGGER_SUGGESTION_COMMAND,
rangeToReplace,
},
{
...buildOptionDefinition(metadataOption),
filterText: fragment,
text: fragment + ' METADATA ',
asSnippet: false, // turn this off because $ could be contained within the source name
rangeToReplace,
},
...recommendedQueriesSuggestions.map((suggestion) => ({
...suggestion,
rangeToReplace,
filterText: fragment,
text: fragment + suggestion.text,
})),
];
return _suggestions;
}
}
);
addSuggestionsBasedOnQuote(suggestionsToAdd);
} else {
// FROM <suggest> or no index/text
await addSuggestionsBasedOnQuote(getSourceSuggestions(await getSources()));
}
}
}
}
Expand Down Expand Up @@ -1021,11 +928,6 @@ async function getExpressionSuggestionsByType(
}));
suggestions.push(...finalSuggestions);
}

// handle recommended queries for from
if (hasRecommendedQueries) {
suggestions.push(...(await getRecommendedQueriesSuggestions(getFieldsByType)));
}
}
// Due to some logic overlapping functions can be repeated
// so dedupe here based on text string (it can differ from name)
Expand Down Expand Up @@ -1508,53 +1410,6 @@ async function getOptionArgsSuggestions(
}
}

if (option.name === 'metadata') {
const existingFields = new Set(option.args.filter(isColumnItem).map(({ name }) => name));
const filteredMetaFields = METADATA_FIELDS.filter((name) => !existingFields.has(name));
if (isNewExpression) {
suggestions.push(
...(await handleFragment(
innerText,
(fragment) => METADATA_FIELDS.includes(fragment),
(_fragment, rangeToReplace) =>
buildFieldsDefinitions(filteredMetaFields).map((suggestion) => ({
...suggestion,
rangeToReplace,
})),
(fragment, rangeToReplace) => {
const _suggestions = [
{
...pipeCompleteItem,
text: fragment + ' | ',
filterText: fragment,
command: TRIGGER_SUGGESTION_COMMAND,
rangeToReplace,
},
];
if (filteredMetaFields.length > 1) {
_suggestions.push({
...commaCompleteItem,
text: fragment + ', ',
filterText: fragment,
command: TRIGGER_SUGGESTION_COMMAND,
rangeToReplace,
});
}
return _suggestions;
}
))
);
} else {
if (existingFields.size > 0) {
// METADATA field <suggest>
if (filteredMetaFields.length > 0) {
suggestions.push(commaCompleteItem);
}
suggestions.push(pipeCompleteItem);
}
}
}

if (optionDef) {
if (!suggestions.length) {
const argDefIndex = optionDef.signature.multipleParams
Expand Down
Loading

0 comments on commit d5e3e74

Please sign in to comment.