Skip to content

Commit 0c7bce2

Browse files
[8.19] [ES|QL] Use correct timeFieldName for time brush filter (#221322) (#221524)
# Backport This will backport the following commits from `main` to `8.19`: - [[ES|QL] Use correct timeFieldName for time brush filter (#221322)](#221322) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Marco Vettorello","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-05-26T14:41:06Z","message":"[ES|QL] Use correct timeFieldName for time brush filter (#221322)\n\n## Summary\n\nfix #221241\n\n## Bug and fix descriptions\n\nThe logic that applies a time filter by updating the time picker vs\nadding it as a filter pill in the filter bar is described here:\n\nhttps://github.com/elastic/kibana/blob/e877bafa3e4725d3e7cfd3b2ca0be4bc6d88a0b9/src/platform/plugins/shared/unified_search/public/actions/apply_filter_action/apply_filter_action.tsx#L107-L114\n\nThe `extractTimeFilter` function extracts the `timeRangeFilter` only if\nthe `timeFieldName === keys(filter.query.range)[0]` . The problem was\nthat the first key of `query.range` was different than the\n`timeFieldName`. That `timeFieldName` in the ESQL world was wrongly\napplied via the ` table.columns[xAxisColumnIndex].name` that doesn't\nreflect the actual column name for ES|QL, in fact that name reflect the\nvisualization label name for the time dimension. In particular, before\nthe fix, that timeFieldName was set as the axis name (e.g.`@timestamp\nevery 5 minute` ) and cause the time range filter to fail to be\nextracted from the filters list.\n\nA combination of two PRs caused this to be now anymore the correct\nbehaviour:\n- this PR #196049 introduced the\nuse of the `souceField` as field name for the creation of filter from a\nrange\n- this PR #217719 instead\nintroduced the use of `sourceField` also for ESQL datasources. This\nfield points to the actual column name described in the ESQL query.\n\nBoth PR causes the `extractTimeFilter` to fail to extract the\ntimeFilter, pushing the filter up to the filter pills.\n\n### Side notes\n\nThis could be probably fixed in other ways, like by avoiding using the\n`sourceField` in ESQL, or by using only the column.name in the filter\ncreation, or by avoiding checking the `timeFieldName` against the\n`query.range` key (not really sure why this is required). In general the\nproblem here is that there is a low confidence on what these\nfields/params are supposed to be and which is supposted to be the\nidentifiers to use everywhere. For example the column ids reflects only\na link between the rows and the column descriptions, the name is the\nassociated label, but can we rely on that label for filtering? i believe\nwe need a stronger connection with the data and the actual original\nsource field or column identifier is a better choice.\nI believe a valid subsequent task is\nhttps://github.com//issues/189044","sha":"eb4ba962ec29256f086ade49d04150c3aec1789b","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Visualizations","release_note:skip","Feature:ES|QL","backport:version","v9.1.0","v8.19.0"],"title":"[ES|QL] Use correct timeFieldName for time brush filter","number":221322,"url":"https://github.com/elastic/kibana/pull/221322","mergeCommit":{"message":"[ES|QL] Use correct timeFieldName for time brush filter (#221322)\n\n## Summary\n\nfix #221241\n\n## Bug and fix descriptions\n\nThe logic that applies a time filter by updating the time picker vs\nadding it as a filter pill in the filter bar is described here:\n\nhttps://github.com/elastic/kibana/blob/e877bafa3e4725d3e7cfd3b2ca0be4bc6d88a0b9/src/platform/plugins/shared/unified_search/public/actions/apply_filter_action/apply_filter_action.tsx#L107-L114\n\nThe `extractTimeFilter` function extracts the `timeRangeFilter` only if\nthe `timeFieldName === keys(filter.query.range)[0]` . The problem was\nthat the first key of `query.range` was different than the\n`timeFieldName`. That `timeFieldName` in the ESQL world was wrongly\napplied via the ` table.columns[xAxisColumnIndex].name` that doesn't\nreflect the actual column name for ES|QL, in fact that name reflect the\nvisualization label name for the time dimension. In particular, before\nthe fix, that timeFieldName was set as the axis name (e.g.`@timestamp\nevery 5 minute` ) and cause the time range filter to fail to be\nextracted from the filters list.\n\nA combination of two PRs caused this to be now anymore the correct\nbehaviour:\n- this PR #196049 introduced the\nuse of the `souceField` as field name for the creation of filter from a\nrange\n- this PR #217719 instead\nintroduced the use of `sourceField` also for ESQL datasources. This\nfield points to the actual column name described in the ESQL query.\n\nBoth PR causes the `extractTimeFilter` to fail to extract the\ntimeFilter, pushing the filter up to the filter pills.\n\n### Side notes\n\nThis could be probably fixed in other ways, like by avoiding using the\n`sourceField` in ESQL, or by using only the column.name in the filter\ncreation, or by avoiding checking the `timeFieldName` against the\n`query.range` key (not really sure why this is required). In general the\nproblem here is that there is a low confidence on what these\nfields/params are supposed to be and which is supposted to be the\nidentifiers to use everywhere. For example the column ids reflects only\na link between the rows and the column descriptions, the name is the\nassociated label, but can we rely on that label for filtering? i believe\nwe need a stronger connection with the data and the actual original\nsource field or column identifier is a better choice.\nI believe a valid subsequent task is\nhttps://github.com//issues/189044","sha":"eb4ba962ec29256f086ade49d04150c3aec1789b"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/221322","number":221322,"mergeCommit":{"message":"[ES|QL] Use correct timeFieldName for time brush filter (#221322)\n\n## Summary\n\nfix #221241\n\n## Bug and fix descriptions\n\nThe logic that applies a time filter by updating the time picker vs\nadding it as a filter pill in the filter bar is described here:\n\nhttps://github.com/elastic/kibana/blob/e877bafa3e4725d3e7cfd3b2ca0be4bc6d88a0b9/src/platform/plugins/shared/unified_search/public/actions/apply_filter_action/apply_filter_action.tsx#L107-L114\n\nThe `extractTimeFilter` function extracts the `timeRangeFilter` only if\nthe `timeFieldName === keys(filter.query.range)[0]` . The problem was\nthat the first key of `query.range` was different than the\n`timeFieldName`. That `timeFieldName` in the ESQL world was wrongly\napplied via the ` table.columns[xAxisColumnIndex].name` that doesn't\nreflect the actual column name for ES|QL, in fact that name reflect the\nvisualization label name for the time dimension. In particular, before\nthe fix, that timeFieldName was set as the axis name (e.g.`@timestamp\nevery 5 minute` ) and cause the time range filter to fail to be\nextracted from the filters list.\n\nA combination of two PRs caused this to be now anymore the correct\nbehaviour:\n- this PR #196049 introduced the\nuse of the `souceField` as field name for the creation of filter from a\nrange\n- this PR #217719 instead\nintroduced the use of `sourceField` also for ESQL datasources. This\nfield points to the actual column name described in the ESQL query.\n\nBoth PR causes the `extractTimeFilter` to fail to extract the\ntimeFilter, pushing the filter up to the filter pills.\n\n### Side notes\n\nThis could be probably fixed in other ways, like by avoiding using the\n`sourceField` in ESQL, or by using only the column.name in the filter\ncreation, or by avoiding checking the `timeFieldName` against the\n`query.range` key (not really sure why this is required). In general the\nproblem here is that there is a low confidence on what these\nfields/params are supposed to be and which is supposted to be the\nidentifiers to use everywhere. For example the column ids reflects only\na link between the rows and the column descriptions, the name is the\nassociated label, but can we rely on that label for filtering? i believe\nwe need a stronger connection with the data and the actual original\nsource field or column identifier is a better choice.\nI believe a valid subsequent task is\nhttps://github.com//issues/189044","sha":"eb4ba962ec29256f086ade49d04150c3aec1789b"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Marco Vettorello <[email protected]>
1 parent 92fca14 commit 0c7bce2

File tree

3 files changed

+51
-14
lines changed

3 files changed

+51
-14
lines changed

src/platform/packages/shared/kbn-es-query/src/filters/helpers/extract_time_filter.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,28 @@ import { Filter, isRangeFilter, RangeFilter } from '../build_filters';
1212
import { TimeRange } from './types';
1313
import { convertRangeFilterToTimeRangeString } from './convert_range_filter';
1414

15-
export function extractTimeFilter(timeFieldName: string, filters: Filter[]) {
16-
const [timeRangeFilter, restOfFilters] = partition(filters, (obj: Filter) => {
17-
let key;
18-
19-
if (isRangeFilter(obj)) {
20-
key = keys(obj.query.range)[0];
21-
}
22-
23-
return Boolean(key && key === timeFieldName);
24-
});
15+
export function extractTimeFilter(
16+
timeFieldName: string,
17+
filters: Filter[]
18+
): { restOfFilters: Filter[]; timeRangeFilter?: RangeFilter } {
19+
const [timeRangeFilter, restOfFilters] = partition<Filter, RangeFilter>(
20+
filters,
21+
(f: Filter): f is RangeFilter => isRangeFilter(f) && timeFieldName === keys(f.query.range)[0]
22+
);
2523

2624
return {
2725
restOfFilters,
28-
timeRangeFilter: timeRangeFilter[0] as RangeFilter | undefined,
26+
timeRangeFilter: timeRangeFilter[0],
2927
};
3028
}
3129

3230
export function extractTimeRange(
3331
filters: Filter[],
3432
timeFieldName?: string
3533
): { restOfFilters: Filter[]; timeRange?: TimeRange } {
36-
if (!timeFieldName) return { restOfFilters: filters, timeRange: undefined };
34+
if (!timeFieldName) {
35+
return { restOfFilters: filters };
36+
}
3737
const { timeRangeFilter, restOfFilters } = extractTimeFilter(timeFieldName, filters);
3838
return {
3939
restOfFilters,

src/platform/plugins/shared/chart_expressions/expression_xy/public/components/xy_chart.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -658,12 +658,16 @@ export function XYChart({
658658
? getAccessorByDimension(dataLayers[0].xAccessor, table.columns)
659659
: undefined;
660660
const xAxisColumnIndex = table.columns.findIndex((el) => el.id === xAccessor);
661-
662661
const context: BrushEvent['data'] = {
663662
range: [min, max],
664663
table,
665664
column: xAxisColumnIndex,
666-
...(isEsqlMode ? { timeFieldName: table.columns[xAxisColumnIndex].name } : {}),
665+
...(isEsqlMode
666+
? {
667+
timeFieldName:
668+
table.columns[xAxisColumnIndex].meta.sourceParams?.sourceField?.toString(),
669+
}
670+
: {}),
667671
};
668672
onSelectRange(context);
669673
};

src/platform/test/functional/apps/discover/esql/_esql_view.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
2626
const esql = getService('esql');
2727
const dashboardAddPanel = getService('dashboardAddPanel');
2828
const dataViews = getService('dataViews');
29+
const elasticChart = getService('elasticChart');
30+
const filterBar = getService('filterBar');
31+
2932
const { common, discover, dashboard, header, timePicker, unifiedFieldList, unifiedSearch } =
3033
getPageObjects([
3134
'common',
@@ -236,6 +239,36 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
236239
const cell = await dataGrid.getCellElementExcludingControlColumns(0, 0);
237240
expect(await cell.getVisibleText()).to.be('1');
238241
});
242+
243+
it('should allow brushing time series', async () => {
244+
await timePicker.setDefaultAbsoluteRange();
245+
await discover.selectTextBaseLang();
246+
await header.waitUntilLoadingHasFinished();
247+
await discover.waitUntilSearchingHasFinished();
248+
await unifiedFieldList.waitUntilSidebarHasLoaded();
249+
250+
const testQuery = `from logstash-* | limit 100`;
251+
252+
await monacoEditor.setCodeEditorValue(testQuery);
253+
await testSubjects.click('querySubmitButton');
254+
await header.waitUntilLoadingHasFinished();
255+
await discover.waitUntilSearchingHasFinished();
256+
257+
const initialTimeConfig = await timePicker.getTimeConfigAsAbsoluteTimes();
258+
expect(initialTimeConfig.start).to.equal(timePicker.defaultStartTime);
259+
expect(initialTimeConfig.end).to.equal(timePicker.defaultEndTime);
260+
261+
const renderingCount = await elasticChart.getVisualizationRenderingCount();
262+
await discover.brushHistogram();
263+
await discover.waitUntilSearchingHasFinished();
264+
// no filter pill created for time brush
265+
expect(await filterBar.getFilterCount()).to.be(0);
266+
// chart and time picker updated
267+
await elasticChart.waitForRenderingCount(renderingCount + 1);
268+
const updatedTimeConfig = await timePicker.getTimeConfigAsAbsoluteTimes();
269+
expect(updatedTimeConfig.start).to.be('Sep 20, 2015 @ 08:41:22.854');
270+
expect(updatedTimeConfig.end).to.be('Sep 21, 2015 @ 04:14:56.951');
271+
});
239272
});
240273

241274
describe('errors', () => {

0 commit comments

Comments
 (0)