Skip to content

Commit

Permalink
[ES|QL] Change the params to start with _ (#192990)
Browse files Browse the repository at this point in the history
## Summary

Now that the named params support underscore in the beginning
elastic/elasticsearch#111950 we decided that we
want all the kibana named params to start with it. So I had to change
it. Again. But naming is hard 🤷‍♀️ and this time I think is the final
change 🥲

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

(cherry picked from commit 9ac525c)
  • Loading branch information
stratoula committed Sep 17, 2024
1 parent c9c3e89 commit 172780f
Show file tree
Hide file tree
Showing 15 changed files with 40 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
export const query1 = `
from kibana_sample_data_logs
| EVAL timestamp=DATE_TRUNC(3 hour, @timestamp), status = CASE( to_integer(response.keyword) >= 200 and to_integer(response.keyword) < 400, "HTTP 2xx and 3xx", to_integer(response.keyword) >= 400 and to_integer(response.keyword) < 500, "HTTP 4xx", "HTTP 5xx")
| stats results = count(*) by \`Over time\` = BUCKET(timestamp, 50, ?t_start, ?t_end), status
| stats results = count(*) by \`Over time\` = BUCKET(timestamp, 50, ?_tstart, ?_tend), status
`;

export const query2 = `
Expand Down
6 changes: 3 additions & 3 deletions packages/kbn-esql-ast/src/walker/walker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ describe('Walker.params()', () => {

test('can collect all params from grouping functions', () => {
const query =
'ROW x=1, time=2024-07-10 | stats z = avg(x) by bucket(time, 20, ?t_start,?t_end)';
'ROW x=1, time=2024-07-10 | stats z = avg(x) by bucket(time, 20, ?_tstart,?_tend)';
const { ast } = getAstAndSyntaxErrors(query);
const params = Walker.params(ast);

Expand All @@ -802,13 +802,13 @@ describe('Walker.params()', () => {
type: 'literal',
literalType: 'param',
paramType: 'named',
value: 't_start',
value: '_tstart',
},
{
type: 'literal',
literalType: 'param',
paramType: 'named',
value: 't_end',
value: '_tend',
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe('getInitialESQLQuery', () => {
] as DataView['fields'];
const dataView = getDataView('logs*', fields, '@custom_timestamp');
expect(getInitialESQLQuery(dataView)).toBe(
'FROM logs* | WHERE @custom_timestamp >= ?t_start AND @custom_timestamp <= ?t_end | LIMIT 10'
'FROM logs* | WHERE @custom_timestamp >= ?_tstart AND @custom_timestamp <= ?_tend | LIMIT 10'
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function getInitialESQLQuery(dataView: DataView): string {
const timeFieldName = dataView?.timeFieldName;
const filterByTimeParams =
!hasAtTimestampField && timeFieldName
? ` | WHERE ${timeFieldName} >= ?t_start AND ${timeFieldName} <= ?t_end`
? ` | WHERE ${timeFieldName} >= ?_tstart AND ${timeFieldName} <= ?_tend`
: '';
return `FROM ${dataView.getIndexPattern()}${filterByTimeParams} | LIMIT 10`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,27 +154,27 @@ describe('esql query helpers', () => {
});

it('should return the time field if there is at least one time param', () => {
expect(getTimeFieldFromESQLQuery('from a | eval b = 1 | where time >= ?t_start')).toBe(
expect(getTimeFieldFromESQLQuery('from a | eval b = 1 | where time >= ?_tstart')).toBe(
'time'
);
});

it('should return undefined if there is one named param but is not ?t_start or ?t_end', () => {
it('should return undefined if there is one named param but is not ?_tstart or ?_tend', () => {
expect(
getTimeFieldFromESQLQuery('from a | eval b = 1 | where time >= ?late')
).toBeUndefined();
});

it('should return undefined if there is one named param but is used without a time field', () => {
expect(
getTimeFieldFromESQLQuery('from a | eval b = DATE_TRUNC(1 day, ?t_start)')
getTimeFieldFromESQLQuery('from a | eval b = DATE_TRUNC(1 day, ?_tstart)')
).toBeUndefined();
});

it('should return the time field if there is at least one time param in the bucket function', () => {
expect(
getTimeFieldFromESQLQuery(
'from a | stats meow = avg(bytes) by bucket(event.timefield, 200, ?t_start, ?t_end)'
'from a | stats meow = avg(bytes) by bucket(event.timefield, 200, ?_tstart, ?_tend)'
)
).toBe('event.timefield');
});
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-esql-utils/src/utils/query_parsing_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function removeDropCommandsFromESQLQuery(esql?: string): string {
}

/**
* When the ?t_start and ?t_end params are used, we want to retrieve the timefield from the query.
* When the ?_tstart and ?_tend params are used, we want to retrieve the timefield from the query.
* @param esql:string
* @returns string
*/
Expand All @@ -91,7 +91,7 @@ export const getTimeFieldFromESQLQuery = (esql: string) => {

const params = Walker.params(ast);
const timeNamedParam = params.find(
(param) => param.value === 't_start' || param.value === 't_end'
(param) => param.value === '_tstart' || param.value === '_tend'
);
if (!timeNamedParam || !functions.length) {
return undefined;
Expand Down
14 changes: 7 additions & 7 deletions packages/kbn-esql-utils/src/utils/run_query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,26 @@ describe('getStartEndParams', () => {

it('should return an array with the start param if exists at the query', () => {
const time = { from: 'Jul 5, 2024 @ 08:03:56.849', to: 'Jul 5, 2024 @ 10:03:56.849' };
const query = 'FROM foo | where time > ?t_start';
const query = 'FROM foo | where time > ?_tstart';
const params = getStartEndParams(query, time);
expect(params).toHaveLength(1);
expect(params[0]).toHaveProperty('t_start');
expect(params[0]).toHaveProperty('_tstart');
});

it('should return an array with the end param if exists at the query', () => {
const time = { from: 'Jul 5, 2024 @ 08:03:56.849', to: 'Jul 5, 2024 @ 10:03:56.849' };
const query = 'FROM foo | where time < ?t_end';
const query = 'FROM foo | where time < ?_tend';
const params = getStartEndParams(query, time);
expect(params).toHaveLength(1);
expect(params[0]).toHaveProperty('t_end');
expect(params[0]).toHaveProperty('_tend');
});

it('should return an array with the end and start params if exist at the query', () => {
const time = { from: 'Jul 5, 2024 @ 08:03:56.849', to: 'Jul 5, 2024 @ 10:03:56.849' };
const query = 'FROM foo | where time < ?t_end amd time > ?t_start';
const query = 'FROM foo | where time < ?_tend amd time > ?_tstart';
const params = getStartEndParams(query, time);
expect(params).toHaveLength(2);
expect(params[0]).toHaveProperty('t_start');
expect(params[1]).toHaveProperty('t_end');
expect(params[0]).toHaveProperty('_tstart');
expect(params[1]).toHaveProperty('_tend');
});
});
10 changes: 5 additions & 5 deletions packages/kbn-esql-utils/src/utils/run_query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,22 @@ import { esFieldTypeToKibanaFieldType } from '@kbn/field-types';
import type { ESQLColumn, ESQLSearchResponse, ESQLSearchParams } from '@kbn/es-types';
import { lastValueFrom } from 'rxjs';

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

export const getStartEndParams = (query: string, time?: TimeRange) => {
const startNamedParams = /\?t_start/i.test(query);
const endNamedParams = /\?t_end/i.test(query);
const startNamedParams = /\?_tstart/i.test(query);
const endNamedParams = /\?_tend/i.test(query);
if (time && (startNamedParams || endNamedParams)) {
const timeParams = {
start: startNamedParams ? dateMath.parse(time.from)?.toISOString() : undefined,
end: endNamedParams ? dateMath.parse(time.to)?.toISOString() : undefined,
};
const namedParams = [];
if (timeParams?.start) {
namedParams.push({ t_start: timeParams.start });
namedParams.push({ _tstart: timeParams.start });
}
if (timeParams?.end) {
namedParams.push({ t_end: timeParams.end });
namedParams.push({ _tend: timeParams.end });
}
return namedParams;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,14 @@ describe('autocomplete.suggest', () => {
});
test('on space within bucket()', async () => {
const { assertSuggestions } = await setup();
await assertSuggestions('from a | stats avg(b) by BUCKET(/, 50, ?t_start, ?t_end)', [
await assertSuggestions('from a | stats avg(b) by BUCKET(/, 50, ?_tstart, ?_tend)', [
// Note there's no space or comma in the suggested field names
...getFieldNamesByType(['date', ...ESQL_COMMON_NUMERIC_TYPES]),
...getFunctionSignaturesByReturnType('eval', ['date', ...ESQL_COMMON_NUMERIC_TYPES], {
scalar: true,
}),
]);
await assertSuggestions('from a | stats avg(b) by BUCKET( / , 50, ?t_start, ?t_end)', [
await assertSuggestions('from a | stats avg(b) by BUCKET( / , 50, ?_tstart, ?_tend)', [
// Note there's no space or comma in the suggested field names
...getFieldNamesByType(['date', ...ESQL_COMMON_NUMERIC_TYPES]),
...getFunctionSignaturesByReturnType('eval', ['date', ...ESQL_COMMON_NUMERIC_TYPES], {
Expand All @@ -308,7 +308,7 @@ describe('autocomplete.suggest', () => {
]);

await assertSuggestions(
'from a | stats avg(b) by BUCKET(dateField, /50, ?t_start, ?t_end)',
'from a | stats avg(b) by BUCKET(dateField, /50, ?_tstart, ?_tend)',
[
...getLiteralsByType('time_literal'),
...getFunctionSignaturesByReturnType('eval', ['integer', 'date_period'], {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const allFunctions = aggregationFunctionDefinitions
.concat(scalarFunctionDefinitions)
.concat(groupingFunctionDefinitions);

export const TIME_SYSTEM_PARAMS = ['?t_start', '?t_end'];
export const TIME_SYSTEM_PARAMS = ['?_tstart', '?_tend'];

export const getAddDateHistogramSnippet = (histogramBarTarget = 50) => {
return `BUCKET($0, ${histogramBarTarget}, ${TIME_SYSTEM_PARAMS.join(', ')})`;
Expand Down Expand Up @@ -442,13 +442,13 @@ export function getCompatibleLiterals(
}

export const TIME_SYSTEM_DESCRIPTIONS = {
'?t_start': i18n.translate(
'?_tstart': i18n.translate(
'kbn-esql-validation-autocomplete.esql.autocomplete.timeSystemParamStart',
{
defaultMessage: 'The start time from the date picker',
}
),
'?t_end': i18n.translate(
'?_tend': i18n.translate(
'kbn-esql-validation-autocomplete.esql.autocomplete.timeSystemParamEnd',
{
defaultMessage: 'The end time from the date picker',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ test('should allow param inside agg function argument', async () => {
test('allow params in WHERE command expressions', async () => {
const { validate } = await setup();

const res1 = await validate('FROM index | WHERE textField >= ?t_start');
const res1 = await validate('FROM index | WHERE textField >= ?_tstart');
const res2 = await validate(`
FROM index
| WHERE textField >= ?t_start
| WHERE textField >= ?_tstart
| WHERE textField <= ?0
| WHERE textField == ?
`);
const res3 = await validate(`
FROM index
| WHERE textField >= ?t_start
| WHERE textField >= ?_tstart
AND textField <= ?0
AND textField == ?
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('getEsqlDataView', () => {
});

it('returns an adhoc dataview if it is adhoc with named params and query index pattern is the same as the dataview index pattern', async () => {
const query = { esql: 'from data-view-ad-hoc-title | where time >= ?t_start' };
const query = { esql: 'from data-view-ad-hoc-title | where time >= ?_tstart' };
const dataView = await getEsqlDataView(query, dataViewAdHocNoAtTimestamp, services);
expect(dataView.timeFieldName).toBe('time');
});
Expand Down
4 changes: 2 additions & 2 deletions test/functional/apps/discover/esql/_esql_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(await testSubjects.exists('unifiedHistogramChart')).to.be(false);
});

it('should render the histogram for indices with no @timestamp field when the ?t_start, ?t_end params are in the query', async function () {
it('should render the histogram for indices with no @timestamp field when the ?_tstart, ?_tend params are in the query', async function () {
await discover.selectTextBaseLang();
await unifiedFieldList.waitUntilSidebarHasLoaded();

const testQuery = `from kibana_sample_data_flights | limit 10 | where timestamp >= ?t_start and timestamp <= ?t_end`;
const testQuery = `from kibana_sample_data_flights | limit 10 | where timestamp >= ?_tstart and timestamp <= ?_tend`;

await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ describe('Text based languages utils', () => {
const expressionsMock = expressionsPluginMock.createStartContract();
const updatedState = await getStateFromAggregateQuery(
state,
{ esql: 'FROM my-fake-index-pattern | WHERE time <= ?t_end' },
{ esql: 'FROM my-fake-index-pattern | WHERE time <= ?_tend' },
{
...dataViewsMock,
getIdsWithTitle: jest.fn().mockReturnValue(
Expand Down Expand Up @@ -361,7 +361,7 @@ describe('Text based languages utils', () => {
errors: [],
index: '4',
query: {
esql: 'FROM my-fake-index-pattern | WHERE time <= ?t_end',
esql: 'FROM my-fake-index-pattern | WHERE time <= ?_tend',
},
timeField: 'time',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(await testSubjects.exists('unifiedHistogramChart')).to.be(false);
});

it('should render the histogram for indices with no @timestamp field when the ?t_start, ?t_end params are in the query', async function () {
it('should render the histogram for indices with no @timestamp field when the ?_tstart, ?_tend params are in the query', async function () {
await PageObjects.discover.selectTextBaseLang();
await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded();

const testQuery = `from kibana_sample_data_flights | limit 10 | where timestamp >= ?t_start and timestamp <= ?t_end`;
const testQuery = `from kibana_sample_data_flights | limit 10 | where timestamp >= ?_tstart and timestamp <= ?_tend`;

await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
Expand Down

0 comments on commit 172780f

Please sign in to comment.