Skip to content

Commit 151ea7c

Browse files
committed
[ES|QL] Use correct timeFieldName for time brush filter (elastic#221322)
## Summary fix elastic#221241 ## Bug and fix descriptions The logic that applies a time filter by updating the time picker vs adding it as a filter pill in the filter bar is described here: https://github.com/elastic/kibana/blob/e877bafa3e4725d3e7cfd3b2ca0be4bc6d88a0b9/src/platform/plugins/shared/unified_search/public/actions/apply_filter_action/apply_filter_action.tsx#L107-L114 The `extractTimeFilter` function extracts the `timeRangeFilter` only if the `timeFieldName === keys(filter.query.range)[0]` . The problem was that the first key of `query.range` was different than the `timeFieldName`. That `timeFieldName` in the ESQL world was wrongly applied via the ` table.columns[xAxisColumnIndex].name` that doesn't reflect the actual column name for ES|QL, in fact that name reflect the visualization label name for the time dimension. In particular, before the fix, that timeFieldName was set as the axis name (e.g.`@timestamp every 5 minute` ) and cause the time range filter to fail to be extracted from the filters list. A combination of two PRs caused this to be now anymore the correct behaviour: - this PR elastic#196049 introduced the use of the `souceField` as field name for the creation of filter from a range - this PR elastic#217719 instead introduced the use of `sourceField` also for ESQL datasources. This field points to the actual column name described in the ESQL query. Both PR causes the `extractTimeFilter` to fail to extract the timeFilter, pushing the filter up to the filter pills. ### Side notes This could be probably fixed in other ways, like by avoiding using the `sourceField` in ESQL, or by using only the column.name in the filter creation, or by avoiding checking the `timeFieldName` against the `query.range` key (not really sure why this is required). In general the problem here is that there is a low confidence on what these fields/params are supposed to be and which is supposted to be the identifiers to use everywhere. For example the column ids reflects only a link between the rows and the column descriptions, the name is the associated label, but can we rely on that label for filtering? i believe we need a stronger connection with the data and the actual original source field or column identifier is a better choice. I believe a valid subsequent task is elastic#189044 (cherry picked from commit eb4ba96)
1 parent fcc1f14 commit 151ea7c

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)