Skip to content

Commit b7c0e07

Browse files
kibanamachinestratoula
kibanamachine
and
stratoula
authored
[8.x] [ES|QL] [Discover] Displays the histogram suggestion always for non transformational commands (#195863) (#196074)
# Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] [Discover] Displays the histogram suggestion always for non transformational commands (#195863)](#195863) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Stratoula Kalafateli","email":"efstratia.kalafateli@elastic.co"},"sourceCommit":{"committedDate":"2024-10-14T09:52:16Z","message":"[ES|QL] [Discover] Displays the histogram suggestion always for non transformational commands (#195863)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/195752\r\n\r\nThis PR is fixing 2 bugs:\r\n\r\n- It filters out counter fields from the breakdown as they are not\r\nsupported. I created a new util for this\r\n- Fixes a bug unrelated with the breakdown (it also exists in previous\r\nminors). The LensVis service is computing suggestions and pushes them to\r\n`availableSuggestionsWithType `. In some indexes (it depends on the\r\ntypes of the first 5 columns of the index) the lens suggestions api\r\nmight return a suggestion. So in that case the array has the histogram\r\nsuggestion + the suggestion from the suggestions api. So the service\r\nwill pick the first one which is not the histogram. But we know that in\r\ncase of non transformational commands we want to suggest the histogram.\r\nSo this PR is fixing it by ensuring that the array is cleaned up before\r\npushing the histogram suggestion.\r\n\r\n\r\nNote: The 2 bugs are unrelated I just decided to fix them in one PR as\r\nthey are both histogram bugs.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"f962cdcd796af9908449155c989dd03438165773","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:skip","v9.0.0","Team:DataDiscovery","backport:prev-minor","Feature:ES|QL"],"title":"[ES|QL] [Discover] Displays the histogram suggestion always for non transformational commands","number":195863,"url":"https://github.com/elastic/kibana/pull/195863","mergeCommit":{"message":"[ES|QL] [Discover] Displays the histogram suggestion always for non transformational commands (#195863)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/195752\r\n\r\nThis PR is fixing 2 bugs:\r\n\r\n- It filters out counter fields from the breakdown as they are not\r\nsupported. I created a new util for this\r\n- Fixes a bug unrelated with the breakdown (it also exists in previous\r\nminors). The LensVis service is computing suggestions and pushes them to\r\n`availableSuggestionsWithType `. In some indexes (it depends on the\r\ntypes of the first 5 columns of the index) the lens suggestions api\r\nmight return a suggestion. So in that case the array has the histogram\r\nsuggestion + the suggestion from the suggestions api. So the service\r\nwill pick the first one which is not the histogram. But we know that in\r\ncase of non transformational commands we want to suggest the histogram.\r\nSo this PR is fixing it by ensuring that the array is cleaned up before\r\npushing the histogram suggestion.\r\n\r\n\r\nNote: The 2 bugs are unrelated I just decided to fix them in one PR as\r\nthey are both histogram bugs.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"f962cdcd796af9908449155c989dd03438165773"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195863","number":195863,"mergeCommit":{"message":"[ES|QL] [Discover] Displays the histogram suggestion always for non transformational commands (#195863)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/195752\r\n\r\nThis PR is fixing 2 bugs:\r\n\r\n- It filters out counter fields from the breakdown as they are not\r\nsupported. I created a new util for this\r\n- Fixes a bug unrelated with the breakdown (it also exists in previous\r\nminors). The LensVis service is computing suggestions and pushes them to\r\n`availableSuggestionsWithType `. In some indexes (it depends on the\r\ntypes of the first 5 columns of the index) the lens suggestions api\r\nmight return a suggestion. So in that case the array has the histogram\r\nsuggestion + the suggestion from the suggestions api. So the service\r\nwill pick the first one which is not the histogram. But we know that in\r\ncase of non transformational commands we want to suggest the histogram.\r\nSo this PR is fixing it by ensuring that the array is cleaned up before\r\npushing the histogram suggestion.\r\n\r\n\r\nNote: The 2 bugs are unrelated I just decided to fix them in one PR as\r\nthey are both histogram bugs.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"f962cdcd796af9908449155c989dd03438165773"}}]}] BACKPORT--> Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
1 parent 93e770c commit b7c0e07

8 files changed

Lines changed: 106 additions & 7 deletions

File tree

‎packages/kbn-esql-utils/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export {
3030
retrieveMetadataColumns,
3131
getQueryColumnsFromESQLQuery,
3232
isESQLColumnSortable,
33+
isESQLColumnGroupable,
3334
TextBasedLanguages,
3435
} from './src';
3536

‎packages/kbn-esql-utils/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,4 @@ export {
3131
getStartEndParams,
3232
hasStartEndParams,
3333
} from './utils/run_query';
34-
export { isESQLColumnSortable } from './utils/esql_fields_utils';
34+
export { isESQLColumnSortable, isESQLColumnGroupable } from './utils/esql_fields_utils';

‎packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99
import type { DatatableColumn } from '@kbn/expressions-plugin/common';
10-
import { isESQLColumnSortable } from './esql_fields_utils';
10+
import { isESQLColumnSortable, isESQLColumnGroupable } from './esql_fields_utils';
1111

1212
describe('esql fields helpers', () => {
1313
describe('isESQLColumnSortable', () => {
@@ -63,4 +63,45 @@ describe('esql fields helpers', () => {
6363
expect(isESQLColumnSortable(keywordField)).toBeTruthy();
6464
});
6565
});
66+
67+
describe('isESQLColumnGroupable', () => {
68+
it('returns false for unsupported fields', () => {
69+
const unsupportedField = {
70+
id: 'unsupported',
71+
name: 'unsupported',
72+
meta: {
73+
type: 'unknown',
74+
esType: 'unknown',
75+
},
76+
isNull: false,
77+
} as DatatableColumn;
78+
expect(isESQLColumnGroupable(unsupportedField)).toBeFalsy();
79+
});
80+
81+
it('returns false for counter fields', () => {
82+
const tsdbField = {
83+
id: 'tsbd_counter',
84+
name: 'tsbd_counter',
85+
meta: {
86+
type: 'number',
87+
esType: 'counter_long',
88+
},
89+
isNull: false,
90+
} as DatatableColumn;
91+
expect(isESQLColumnGroupable(tsdbField)).toBeFalsy();
92+
});
93+
94+
it('returns true for everything else', () => {
95+
const keywordField = {
96+
id: 'sortable',
97+
name: 'sortable',
98+
meta: {
99+
type: 'string',
100+
esType: 'keyword',
101+
},
102+
isNull: false,
103+
} as DatatableColumn;
104+
expect(isESQLColumnGroupable(keywordField)).toBeTruthy();
105+
});
106+
});
66107
});

‎packages/kbn-esql-utils/src/utils/esql_fields_utils.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type { DatatableColumn } from '@kbn/expressions-plugin/common';
1212
const SPATIAL_FIELDS = ['geo_point', 'geo_shape', 'point', 'shape'];
1313
const SOURCE_FIELD = '_source';
1414
const TSDB_COUNTER_FIELDS_PREFIX = 'counter_';
15+
const UNKNOWN_FIELD = 'unknown';
1516

1617
/**
1718
* Check if a column is sortable.
@@ -38,3 +39,23 @@ export const isESQLColumnSortable = (column: DatatableColumn): boolean => {
3839

3940
return true;
4041
};
42+
43+
/**
44+
* Check if a column is groupable (| STATS ... BY <column>).
45+
*
46+
* @param column The DatatableColumn of the field.
47+
* @returns True if the column is groupable, false otherwise.
48+
*/
49+
50+
export const isESQLColumnGroupable = (column: DatatableColumn): boolean => {
51+
// we don't allow grouping on the unknown field types
52+
if (column.meta?.type === UNKNOWN_FIELD) {
53+
return false;
54+
}
55+
// we don't allow grouping on tsdb counter fields
56+
if (column.meta?.esType && column.meta?.esType?.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) {
57+
return false;
58+
}
59+
60+
return true;
61+
};

‎src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import React, { useCallback, useMemo } from 'react';
1111
import { EuiSelectableOption } from '@elastic/eui';
1212
import { FieldIcon, getFieldIconProps, comboBoxFieldOptionMatcher } from '@kbn/field-utils';
1313
import { css } from '@emotion/react';
14+
import { isESQLColumnGroupable } from '@kbn/esql-utils';
1415
import { type DataView, DataViewField } from '@kbn/data-views-plugin/common';
1516
import type { DatatableColumn } from '@kbn/expressions-plugin/common';
1617
import { convertDatatableColumnToDataViewFieldSpec } from '@kbn/data-view-utils';
@@ -34,10 +35,10 @@ export interface BreakdownFieldSelectorProps {
3435
const mapToDropdownFields = (dataView: DataView, esqlColumns?: DatatableColumn[]) => {
3536
if (esqlColumns) {
3637
return (
38+
// filter out unsupported field types and counter time series metrics
3739
esqlColumns
40+
.filter(isESQLColumnGroupable)
3841
.map((column) => new DataViewField(convertDatatableColumnToDataViewFieldSpec(column)))
39-
// filter out unsupported field types
40-
.filter((field) => field.type !== 'unknown')
4142
);
4243
}
4344

‎src/plugins/unified_histogram/public/services/lens_vis_service.attributes.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -674,7 +674,8 @@ describe('LensVisService attributes', () => {
674674
},
675675
],
676676
"query": Object {
677-
"esql": "from logstash-* | limit 10",
677+
"esql": "from logstash-* | limit 10
678+
| EVAL timestamp=DATE_TRUNC(10 minute, timestamp) | stats results = count(*) by timestamp | rename timestamp as \`timestamp every 10 minute\`",
678679
},
679680
"visualization": Object {
680681
"gridConfig": Object {
@@ -706,7 +707,7 @@ describe('LensVisService attributes', () => {
706707
"timeField": "timestamp",
707708
"timeInterval": undefined,
708709
},
709-
"suggestionType": "lensSuggestion",
710+
"suggestionType": "histogramForESQL",
710711
}
711712
`);
712713
});

‎src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,37 @@ describe('LensVisService suggestions', () => {
254254
expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery);
255255
});
256256

257+
test('should return histogramSuggestion even if suggestions returned by the api', async () => {
258+
const lensVis = await getLensVisMock({
259+
filters: [],
260+
query: { esql: 'from the-data-view | limit 100' },
261+
dataView: dataViewMock,
262+
timeInterval: 'auto',
263+
timeRange: {
264+
from: '2023-09-03T08:00:00.000Z',
265+
to: '2023-09-04T08:56:28.274Z',
266+
},
267+
breakdownField: undefined,
268+
columns: [
269+
{
270+
id: 'var0',
271+
name: 'var0',
272+
meta: {
273+
type: 'number',
274+
},
275+
},
276+
],
277+
isPlainRecord: true,
278+
allSuggestions: allSuggestionsMock,
279+
hasHistogramSuggestionForESQL: true,
280+
});
281+
282+
expect(lensVis.currentSuggestionContext?.type).toBe(
283+
UnifiedHistogramSuggestionType.histogramForESQL
284+
);
285+
expect(lensVis.currentSuggestionContext?.suggestion).toBeDefined();
286+
});
287+
257288
test('should return histogramSuggestion if no suggestions returned by the api with a geo point breakdown field correctly', async () => {
258289
const lensVis = await getLensVisMock({
259290
filters: [],

‎src/plugins/unified_histogram/public/services/lens_vis_service.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ export class LensVisService {
235235
let currentSuggestion: Suggestion | undefined;
236236

237237
// takes lens suggestions if provided
238-
const availableSuggestionsWithType: Array<{
238+
let availableSuggestionsWithType: Array<{
239239
suggestion: UnifiedHistogramSuggestionContext['suggestion'];
240240
type: UnifiedHistogramSuggestionType;
241241
}> = [];
@@ -254,6 +254,9 @@ export class LensVisService {
254254
breakdownField,
255255
});
256256
if (histogramSuggestionForESQL) {
257+
// In case if histogram suggestion, we want to empty the array and push the new suggestion
258+
// to ensure that only the histogram suggestion is available
259+
availableSuggestionsWithType = [];
257260
availableSuggestionsWithType.push({
258261
suggestion: histogramSuggestionForESQL,
259262
type: UnifiedHistogramSuggestionType.histogramForESQL,

0 commit comments

Comments
 (0)