Skip to content

Commit

Permalink
[ES|QL] Rename the params in ?t_start ?t_end (elastic#190115)
Browse files Browse the repository at this point in the history
## Summary

Renames the variables from `?start` to `?t_start` and `?end` to `?t_end`

Naming is hard so bare with us 😅 (I think this will be the last change)
  • Loading branch information
stratoula authored Aug 9, 2024
1 parent 06d5d25 commit 04c46db
Show file tree
Hide file tree
Showing 13 changed files with 39 additions and 34 deletions.
7 changes: 4 additions & 3 deletions packages/kbn-esql-ast/src/walker/walker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,8 @@ 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, ?start,?end)';
const query =
'ROW x=1, time=2024-07-10 | stats z = avg(x) by bucket(time, 20, ?t_start,?t_end)';
const { ast } = getAstAndSyntaxErrors(query);
const params = Walker.params(ast);

Expand All @@ -800,13 +801,13 @@ describe('Walker.params()', () => {
type: 'literal',
literalType: 'param',
paramType: 'named',
value: 'start',
value: 't_start',
},
{
type: 'literal',
literalType: 'param',
paramType: 'named',
value: 'end',
value: 't_end',
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('getInitialESQLQuery', () => {
] as DataView['fields'];
const dataView = getDataView('logs*', fields, '@custom_timestamp');
expect(getInitialESQLQuery(dataView)).toBe(
'FROM logs* | WHERE @custom_timestamp >= ?start AND @custom_timestamp <= ?end | LIMIT 10'
'FROM logs* | WHERE @custom_timestamp >= ?t_start AND @custom_timestamp <= ?t_end | LIMIT 10'
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function getInitialESQLQuery(dataView: DataView): string {
const timeFieldName = dataView?.timeFieldName;
const filterByTimeParams =
!hasAtTimestampField && timeFieldName
? ` | WHERE ${timeFieldName} >= ?start AND ${timeFieldName} <= ?end`
? ` | WHERE ${timeFieldName} >= ?t_start AND ${timeFieldName} <= ?t_end`
: '';
return `FROM ${dataView.getIndexPattern()}${filterByTimeParams} | LIMIT 10`;
}
10 changes: 6 additions & 4 deletions packages/kbn-esql-utils/src/utils/query_parsing_helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,25 +150,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 >= ?start')).toBe('time');
expect(getTimeFieldFromESQLQuery('from a | eval b = 1 | where time >= ?t_start')).toBe(
'time'
);
});

it('should return undefined if there is one named param but is not ?start or ?end', () => {
it('should return undefined if there is one named param but is not ?t_start or ?t_end', () => {
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, ?start)')
getTimeFieldFromESQLQuery('from a | eval b = DATE_TRUNC(1 day, ?t_start)')
).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, ?start, ?end)'
'from a | stats meow = avg(bytes) by bucket(event.timefield, 200, ?t_start, ?t_end)'
)
).toBe('event.timefield');
});
Expand Down
6 changes: 4 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 @@ -56,7 +56,7 @@ export function removeDropCommandsFromESQLQuery(esql?: string): string {
}

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

const params = Walker.params(ast);
const timeNamedParam = params.find((param) => param.value === 'start' || param.value === 'end');
const timeNamedParam = params.find(
(param) => param.value === 't_start' || param.value === 't_end'
);
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 @@ -17,26 +17,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 > ?start';
const query = 'FROM foo | where time > ?t_start';
const params = getStartEndParams(query, time);
expect(params).toHaveLength(1);
expect(params[0]).toHaveProperty('start');
expect(params[0]).toHaveProperty('t_start');
});

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 < ?end';
const query = 'FROM foo | where time < ?t_end';
const params = getStartEndParams(query, time);
expect(params).toHaveLength(1);
expect(params[0]).toHaveProperty('end');
expect(params[0]).toHaveProperty('t_end');
});

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 < ?end amd time > ?start';
const query = 'FROM foo | where time < ?t_end amd time > ?t_start';
const params = getStartEndParams(query, time);
expect(params).toHaveLength(2);
expect(params[0]).toHaveProperty('start');
expect(params[1]).toHaveProperty('end');
expect(params[0]).toHaveProperty('t_start');
expect(params[1]).toHaveProperty('t_end');
});
});
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 @@ -14,22 +14,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) => /\?start|\?end/i.test(query);
export const hasStartEndParams = (query: string) => /\?t_start|\?t_end/i.test(query);

export const getStartEndParams = (query: string, time?: TimeRange) => {
const startNamedParams = /\?start/i.test(query);
const endNamedParams = /\?end/i.test(query);
const startNamedParams = /\?t_start/i.test(query);
const endNamedParams = /\?t_end/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({ start: timeParams.start });
namedParams.push({ t_start: timeParams.start });
}
if (timeParams?.end) {
namedParams.push({ end: timeParams.end });
namedParams.push({ t_end: timeParams.end });
}
return namedParams;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const allFunctions = statsAggregationFunctionDefinitions
.concat(evalFunctionDefinitions)
.concat(groupingFunctionDefinitions);

export const TIME_SYSTEM_PARAMS = ['?start', '?end'];
export const TIME_SYSTEM_PARAMS = ['?t_start', '?t_end'];

export const TRIGGER_SUGGESTION_COMMAND = {
title: 'Trigger Suggestion Dialog',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,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 >= ?start');
const res1 = await validate('FROM index | WHERE textField >= ?t_start');
const res2 = await validate(`
FROM index
| WHERE textField >= ?start
| WHERE textField >= ?t_start
| WHERE textField <= ?0
| WHERE textField == ?
`);
const res3 = await validate(`
FROM index
| WHERE textField >= ?start
| WHERE textField >= ?t_start
AND textField <= ?0
AND textField == ?
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,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 >= ?start' };
const query = { esql: 'from data-view-ad-hoc-title | where time >= ?t_start' };
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 @@ -113,11 +113,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 ?start, ?end params are in the query', async function () {
it('should render the histogram for indices with no @timestamp field when the ?t_start, ?t_end 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 >= ?start and timestamp <= ?end`;
const testQuery = `from kibana_sample_data_flights | limit 10 | where timestamp >= ?t_start and timestamp <= ?t_end`;

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 <= ?end' },
{ esql: 'FROM my-fake-index-pattern | WHERE time <= ?t_end' },
{
...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 <= ?end',
esql: 'FROM my-fake-index-pattern | WHERE time <= ?t_end',
},
timeField: 'time',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,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 ?start, ?end params are in the query', async function () {
it('should render the histogram for indices with no @timestamp field when the ?t_start, ?t_end 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 >= ?start and timestamp <= ?end`;
const testQuery = `from kibana_sample_data_flights | limit 10 | where timestamp >= ?t_start and timestamp <= ?t_end`;

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

0 comments on commit 04c46db

Please sign in to comment.