Skip to content

Commit b477afb

Browse files
authored
[ES|QL] Distinguish the functions/fields vs the values on the query level (elastic#213916)
## Summary Closes elastic#209731 This PR is based on the change made here elastic/elasticsearch#122459 The main difference is that: - Functions and fields should now be added as ?? (instead of ?) - The payload to ES is the same regardless if you send a value or a field/function In order to accommodate this the following changes were made: - Now the variable name in the control form displays the ? or ?? (it didnt display them before) <img width="428" alt="image" src="https://github.com/user-attachments/assets/1381ba4a-591c-47f2-af93-30d54fe7a639" /> - The previous created charts with the old format are bwc (this means that they should load correctly when you checkout in this PR (a helper function has been created to ensure it) ![meow](https://github.com/user-attachments/assets/a1863b5b-e113-494a-9231-e16386876e91) ### Release notes Now the fields / functions variables are being described with ?? in the query. The values variables use ? as before. ### Checklist - [ ] [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 - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
1 parent 51932b6 commit b477afb

File tree

28 files changed

+451
-217
lines changed

28 files changed

+451
-217
lines changed

src/platform/packages/private/kbn-esql-editor/src/esql_editor.tsx

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { useKibana } from '@kbn/kibana-react-plugin/public';
2929
import { ESQLLang, ESQL_LANG_ID, monaco, type ESQLCallbacks } from '@kbn/monaco';
3030
import memoize from 'lodash/memoize';
3131
import React, { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react';
32+
import { fixESQLQueryWithVariables } from '@kbn/esql-utils';
3233
import { createPortal } from 'react-dom';
3334
import { css } from '@emotion/react';
3435
import { ESQLVariableType, type ESQLControlVariable } from '@kbn/esql-types';
@@ -123,9 +124,14 @@ export const ESQLEditor = memo(function ESQLEditor({
123124
data,
124125
} = kibana.services;
125126

127+
const fixedQuery = useMemo(
128+
() => fixESQLQueryWithVariables(query.esql, esqlVariables),
129+
[esqlVariables, query.esql]
130+
);
131+
126132
const variablesService = kibana.services?.esql?.variablesService;
127133
const histogramBarTarget = uiSettings?.get('histogram:barTarget') ?? 50;
128-
const [code, setCode] = useState<string>(query.esql ?? '');
134+
const [code, setCode] = useState<string>(fixedQuery ?? '');
129135
// To make server side errors less "sticky", register the state of the code when submitting
130136
const [codeWhenSubmitted, setCodeStateOnSubmission] = useState(code);
131137
const [editorHeight, setEditorHeight] = useState(
@@ -212,11 +218,11 @@ export const ESQLEditor = memo(function ESQLEditor({
212218

213219
useEffect(() => {
214220
if (editor1.current) {
215-
if (code !== query.esql) {
216-
setCode(query.esql);
221+
if (code !== fixedQuery) {
222+
setCode(fixedQuery);
217223
}
218224
}
219-
}, [code, query.esql]);
225+
}, [code, fixedQuery]);
220226

221227
// Enable the variables service if the feature is supported in the consumer app
222228
useEffect(() => {
@@ -285,7 +291,7 @@ export const ESQLEditor = memo(function ESQLEditor({
285291
monaco.editor.registerCommand('esql.control.time_literal.create', async (...args) => {
286292
const position = editor1.current?.getPosition();
287293
await triggerControl(
288-
query.esql,
294+
fixedQuery,
289295
ESQLVariableType.TIME_LITERAL,
290296
position,
291297
uiActions,
@@ -298,7 +304,7 @@ export const ESQLEditor = memo(function ESQLEditor({
298304
monaco.editor.registerCommand('esql.control.fields.create', async (...args) => {
299305
const position = editor1.current?.getPosition();
300306
await triggerControl(
301-
query.esql,
307+
fixedQuery,
302308
ESQLVariableType.FIELDS,
303309
position,
304310
uiActions,
@@ -311,7 +317,7 @@ export const ESQLEditor = memo(function ESQLEditor({
311317
monaco.editor.registerCommand('esql.control.values.create', async (...args) => {
312318
const position = editor1.current?.getPosition();
313319
await triggerControl(
314-
query.esql,
320+
fixedQuery,
315321
ESQLVariableType.VALUES,
316322
position,
317323
uiActions,
@@ -324,7 +330,7 @@ export const ESQLEditor = memo(function ESQLEditor({
324330
monaco.editor.registerCommand('esql.control.functions.create', async (...args) => {
325331
const position = editor1.current?.getPosition();
326332
await triggerControl(
327-
query.esql,
333+
fixedQuery,
328334
ESQLVariableType.FUNCTIONS,
329335
position,
330336
uiActions,
@@ -440,7 +446,7 @@ export const ESQLEditor = memo(function ESQLEditor({
440446
const esqlCallbacks: ESQLCallbacks = useMemo(() => {
441447
const callbacks: ESQLCallbacks = {
442448
getSources: async () => {
443-
clearCacheWhenOld(dataSourcesCache, query.esql);
449+
clearCacheWhenOld(dataSourcesCache, fixedQuery);
444450
const sources = await memoizedSources(dataViews, core).result;
445451
return sources;
446452
},
@@ -505,7 +511,7 @@ export const ESQLEditor = memo(function ESQLEditor({
505511
fieldsMetadata,
506512
kibana.services?.esql?.getJoinIndicesAutocomplete,
507513
dataSourcesCache,
508-
query.esql,
514+
fixedQuery,
509515
memoizedSources,
510516
dataViews,
511517
core,

src/platform/packages/shared/kbn-esql-utils/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export {
3939
mapVariableToColumn,
4040
getValuesFromQueryField,
4141
getESQLQueryVariables,
42+
fixESQLQueryWithVariables,
4243
} from './src';
4344

4445
export { ENABLE_ESQL, FEEDBACK_LINK } from './constants';

src/platform/packages/shared/kbn-esql-utils/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export {
2323
mapVariableToColumn,
2424
getValuesFromQueryField,
2525
getESQLQueryVariables,
26+
fixESQLQueryWithVariables,
2627
} from './utils/query_parsing_helpers';
2728
export { queryCannotBeSampled } from './utils/query_cannot_be_sampled';
2829
export {

src/platform/packages/shared/kbn-esql-utils/src/utils/query_parsing_helpers.test.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
getQueryColumnsFromESQLQuery,
2121
mapVariableToColumn,
2222
getValuesFromQueryField,
23+
fixESQLQueryWithVariables,
2324
} from './query_parsing_helpers';
2425
import { monaco } from '@kbn/monaco';
2526

@@ -581,4 +582,104 @@ describe('esql query helpers', () => {
581582
expect(values).toEqual('my_field');
582583
});
583584
});
585+
586+
describe('fixESQLQueryWithVariables', () => {
587+
it('should return the query as is if no variables are given', () => {
588+
const esql = 'FROM my_index | STATS COUNT(?field)';
589+
const variables: ESQLControlVariable[] = [];
590+
expect(fixESQLQueryWithVariables(esql, variables)).toEqual(esql);
591+
});
592+
593+
it('should return the query as is if no fields or functions variables are given', () => {
594+
const esql = 'FROM my_index | WHERE field == ?value';
595+
const variables: ESQLControlVariable[] = [
596+
{
597+
key: 'interval',
598+
value: '5 minutes',
599+
type: ESQLVariableType.TIME_LITERAL,
600+
},
601+
{
602+
key: 'agent_name',
603+
value: 'go',
604+
type: ESQLVariableType.VALUES,
605+
},
606+
];
607+
expect(fixESQLQueryWithVariables(esql, variables)).toEqual(esql);
608+
});
609+
610+
it('should return the query as is if fields or functions variables are given but they are already used with ??', () => {
611+
const esql = 'FROM my_index | STATS COUNT(??field)';
612+
const variables: ESQLControlVariable[] = [
613+
{
614+
key: 'interval',
615+
value: '5 minutes',
616+
type: ESQLVariableType.TIME_LITERAL,
617+
},
618+
{
619+
key: 'agent_name',
620+
value: 'go',
621+
type: ESQLVariableType.VALUES,
622+
},
623+
{
624+
key: 'field',
625+
value: 'bytes',
626+
type: ESQLVariableType.FIELDS,
627+
},
628+
];
629+
expect(fixESQLQueryWithVariables(esql, variables)).toEqual(esql);
630+
});
631+
632+
it('should fix the query if fields or functions variables are given and they are already used with ?', () => {
633+
const esql = 'FROM my_index | STATS COUNT(?field)';
634+
const expected = 'FROM my_index | STATS COUNT(??field)';
635+
const variables: ESQLControlVariable[] = [
636+
{
637+
key: 'interval',
638+
value: '5 minutes',
639+
type: ESQLVariableType.TIME_LITERAL,
640+
},
641+
{
642+
key: 'agent_name',
643+
value: 'go',
644+
type: ESQLVariableType.VALUES,
645+
},
646+
{
647+
key: 'field',
648+
value: 'bytes',
649+
type: ESQLVariableType.FIELDS,
650+
},
651+
];
652+
expect(fixESQLQueryWithVariables(esql, variables)).toEqual(expected);
653+
});
654+
655+
it('should fix a query with multiple variables', () => {
656+
const esql =
657+
'FROM my_index | STATS COUNT(?field) by ?breakdownField | WHERE agent.name == ?agent_name';
658+
const expected =
659+
'FROM my_index | STATS COUNT(??field) by ??breakdownField | WHERE agent.name == ?agent_name';
660+
const variables: ESQLControlVariable[] = [
661+
{
662+
key: 'interval',
663+
value: '5 minutes',
664+
type: ESQLVariableType.TIME_LITERAL,
665+
},
666+
{
667+
key: 'agent_name',
668+
value: 'go',
669+
type: ESQLVariableType.VALUES,
670+
},
671+
{
672+
key: 'field',
673+
value: 'bytes',
674+
type: ESQLVariableType.FIELDS,
675+
},
676+
{
677+
key: 'breakdownField',
678+
value: 'clientip',
679+
type: ESQLVariableType.FIELDS,
680+
},
681+
];
682+
expect(fixESQLQueryWithVariables(esql, variables)).toEqual(expected);
683+
});
684+
});
584685
});

src/platform/packages/shared/kbn-esql-utils/src/utils/query_parsing_helpers.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type {
1616
ESQLInlineCast,
1717
ESQLCommandOption,
1818
} from '@kbn/esql-ast';
19-
import type { ESQLControlVariable } from '@kbn/esql-types';
19+
import { type ESQLControlVariable, ESQLVariableType } from '@kbn/esql-types';
2020
import type { DatatableColumn } from '@kbn/expressions-plugin/common';
2121
import { monaco } from '@kbn/monaco';
2222

@@ -161,7 +161,7 @@ export const getQueryColumnsFromESQLQuery = (esql: string): string[] => {
161161
export const getESQLQueryVariables = (esql: string): string[] => {
162162
const { root } = parse(esql);
163163
const usedVariablesInQuery = Walker.params(root);
164-
return usedVariablesInQuery.map((v) => v.text.replace('?', ''));
164+
return usedVariablesInQuery.map((v) => v.text.replace(/^\?+/, ''));
165165
};
166166

167167
/**
@@ -228,3 +228,36 @@ export const getValuesFromQueryField = (queryString: string, cursorPosition?: mo
228228
return `${column.name}`;
229229
}
230230
};
231+
232+
// this is for backward compatibility, if the query is of fields or functions type
233+
// and the query is not set with ?? in the query, we should set it
234+
// https://github.com/elastic/elasticsearch/pull/122459
235+
export const fixESQLQueryWithVariables = (
236+
queryString: string,
237+
esqlVariables?: ESQLControlVariable[]
238+
) => {
239+
const currentVariables = getESQLQueryVariables(queryString);
240+
if (!currentVariables.length) {
241+
return queryString;
242+
}
243+
244+
// filter out the variables that are not used in the query
245+
// and that they are not of type FIELDS or FUNCTIONS
246+
const identifierTypeVariables = esqlVariables?.filter(
247+
(variable) =>
248+
currentVariables.includes(variable.key) &&
249+
(variable.type === ESQLVariableType.FIELDS || variable.type === ESQLVariableType.FUNCTIONS)
250+
);
251+
252+
// check if they are set with ?? or ? in the query
253+
// replace only if there is only one ? in front of the variable
254+
if (identifierTypeVariables?.length) {
255+
identifierTypeVariables.forEach((variable) => {
256+
const regex = new RegExp(`(?<!\\?)\\?${variable.key}`);
257+
queryString = queryString.replace(regex, `??${variable.key}`);
258+
});
259+
return queryString;
260+
}
261+
262+
return queryString;
263+
};

src/platform/packages/shared/kbn-esql-utils/src/utils/run_query.test.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe('run query helpers', () => {
6565

6666
it('should return the variables if given', () => {
6767
const time = { from: 'Jul 5, 2024 @ 08:03:56.849', to: 'Jul 5, 2024 @ 10:03:56.849' };
68-
const query = 'FROM foo | KEEP ?field | WHERE agent.name = ?agent_name';
68+
const query = 'FROM foo | KEEP ??field | WHERE agent.name = ?agent_name';
6969
const variables = [
7070
{
7171
key: 'field',
@@ -91,9 +91,7 @@ describe('run query helpers', () => {
9191
const params = getNamedParams(query, time, variables);
9292
expect(params).toStrictEqual([
9393
{
94-
field: {
95-
identifier: 'clientip',
96-
},
94+
field: 'clientip',
9795
},
9896
{
9997
interval: '5 minutes',
@@ -102,17 +100,15 @@ describe('run query helpers', () => {
102100
agent_name: 'go',
103101
},
104102
{
105-
function: {
106-
identifier: 'count',
107-
},
103+
function: 'count',
108104
},
109105
]);
110106
});
111107

112108
it('should return the variables and named params if given', () => {
113109
const time = { from: 'Jul 5, 2024 @ 08:03:56.849', to: 'Jul 5, 2024 @ 10:03:56.849' };
114110
const query =
115-
'FROM foo | KEEP ?field | WHERE agent.name = ?agent_name AND time < ?_tend amd time > ?_tstart';
111+
'FROM foo | KEEP ??field | WHERE agent.name = ?agent_name AND time < ?_tend amd time > ?_tstart';
116112
const variables = [
117113
{
118114
key: 'field',
@@ -140,9 +136,7 @@ describe('run query helpers', () => {
140136
expect(params[0]).toHaveProperty('_tstart');
141137
expect(params[1]).toHaveProperty('_tend');
142138
expect(params[2]).toStrictEqual({
143-
field: {
144-
identifier: 'clientip',
145-
},
139+
field: 'clientip',
146140
});
147141
expect(params[3]).toStrictEqual({
148142
interval: '5 minutes',
@@ -152,9 +146,7 @@ describe('run query helpers', () => {
152146
});
153147

154148
expect(params[5]).toStrictEqual({
155-
function: {
156-
identifier: 'count',
157-
},
149+
function: 'count',
158150
});
159151
});
160152
});

src/platform/packages/shared/kbn-esql-utils/src/utils/run_query.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { esFieldTypeToKibanaFieldType } from '@kbn/field-types';
1616
import type { ESQLColumn, ESQLSearchResponse, ESQLSearchParams } from '@kbn/es-types';
1717
import { lastValueFrom } from 'rxjs';
1818
import { type ESQLControlVariable } from '@kbn/esql-types';
19-
import { ESQLVariableType } from '@kbn/esql-types';
2019

2120
export const hasStartEndParams = (query: string) => /\?_tstart|\?_tend/i.test(query);
2221

@@ -47,12 +46,8 @@ export const getNamedParams = (
4746
) => {
4847
const namedParams: ESQLSearchParams['params'] = getStartEndParams(query, timeRange);
4948
if (variables?.length) {
50-
variables?.forEach(({ key, value, type }) => {
51-
if (type === ESQLVariableType.FIELDS || type === ESQLVariableType.FUNCTIONS) {
52-
namedParams.push({ [key]: { identifier: value } });
53-
} else {
54-
namedParams.push({ [key]: value });
55-
}
49+
variables?.forEach(({ key, value }) => {
50+
namedParams.push({ [key]: value });
5651
});
5752
}
5853
return namedParams;

0 commit comments

Comments
 (0)