Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ES|QL] [Discover] Displays the histogram suggestion always for non transformational commands #195863

Merged
merged 5 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/kbn-esql-utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export {
retrieveMetadataColumns,
getQueryColumnsFromESQLQuery,
isESQLColumnSortable,
isESQLColumnGroupable,
TextBasedLanguages,
} from './src';

Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ export {
getStartEndParams,
hasStartEndParams,
} from './utils/run_query';
export { isESQLColumnSortable } from './utils/esql_fields_utils';
export { isESQLColumnSortable, isESQLColumnGroupable } from './utils/esql_fields_utils';
43 changes: 42 additions & 1 deletion packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/
import type { DatatableColumn } from '@kbn/expressions-plugin/common';
import { isESQLColumnSortable } from './esql_fields_utils';
import { isESQLColumnSortable, isESQLColumnGroupable } from './esql_fields_utils';

describe('esql fields helpers', () => {
describe('isESQLColumnSortable', () => {
Expand Down Expand Up @@ -63,4 +63,45 @@ describe('esql fields helpers', () => {
expect(isESQLColumnSortable(keywordField)).toBeTruthy();
});
});

describe('isESQLColumnGroupable', () => {
it('returns false for unsupported fields', () => {
const unsupportedField = {
id: 'unsupported',
name: 'unsupported',
meta: {
type: 'unknown',
esType: 'unknown',
},
isNull: false,
} as DatatableColumn;
expect(isESQLColumnGroupable(unsupportedField)).toBeFalsy();
});

it('returns false for counter fields', () => {
const tsdbField = {
id: 'tsbd_counter',
name: 'tsbd_counter',
meta: {
type: 'number',
esType: 'counter_long',
},
isNull: false,
} as DatatableColumn;
expect(isESQLColumnGroupable(tsdbField)).toBeFalsy();
});

it('returns true for everything else', () => {
const keywordField = {
id: 'sortable',
name: 'sortable',
meta: {
type: 'string',
esType: 'keyword',
},
isNull: false,
} as DatatableColumn;
expect(isESQLColumnGroupable(keywordField)).toBeTruthy();
});
});
});
21 changes: 21 additions & 0 deletions packages/kbn-esql-utils/src/utils/esql_fields_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { DatatableColumn } from '@kbn/expressions-plugin/common';
const SPATIAL_FIELDS = ['geo_point', 'geo_shape', 'point', 'shape'];
const SOURCE_FIELD = '_source';
const TSDB_COUNTER_FIELDS_PREFIX = 'counter_';
const UNKNOWN_FIELD = 'unknown';

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

return true;
};

/**
* Check if a column is groupable (| STATS ... BY <column>).
*
* @param column The DatatableColumn of the field.
* @returns True if the column is groupable, false otherwise.
*/

export const isESQLColumnGroupable = (column: DatatableColumn): boolean => {
// we don't allow grouping on the unknown field types
if (column.meta?.type === UNKNOWN_FIELD) {
return false;
}
// we don't allow grouping on tsdb counter fields
if (column.meta?.esType && column.meta?.esType?.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) {
return false;
}

return true;
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import React, { useCallback, useMemo } from 'react';
import { EuiSelectableOption } from '@elastic/eui';
import { FieldIcon, getFieldIconProps, comboBoxFieldOptionMatcher } from '@kbn/field-utils';
import { css } from '@emotion/react';
import { isESQLColumnGroupable } from '@kbn/esql-utils';
import { type DataView, DataViewField } from '@kbn/data-views-plugin/common';
import type { DatatableColumn } from '@kbn/expressions-plugin/common';
import { convertDatatableColumnToDataViewFieldSpec } from '@kbn/data-view-utils';
Expand All @@ -34,10 +35,10 @@ export interface BreakdownFieldSelectorProps {
const mapToDropdownFields = (dataView: DataView, esqlColumns?: DatatableColumn[]) => {
if (esqlColumns) {
return (
// filter out unsupported field types and counter time series metrics
esqlColumns
.filter(isESQLColumnGroupable)
.map((column) => new DataViewField(convertDatatableColumnToDataViewFieldSpec(column)))
// filter out unsupported field types
.filter((field) => field.type !== 'unknown')
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,8 @@ describe('LensVisService attributes', () => {
},
],
"query": Object {
"esql": "from logstash-* | limit 10",
"esql": "from logstash-* | limit 10
| EVAL timestamp=DATE_TRUNC(10 minute, timestamp) | stats results = count(*) by timestamp | rename timestamp as \`timestamp every 10 minute\`",
},
"visualization": Object {
"gridConfig": Object {
Expand Down Expand Up @@ -706,7 +707,7 @@ describe('LensVisService attributes', () => {
"timeField": "timestamp",
"timeInterval": undefined,
},
"suggestionType": "lensSuggestion",
"suggestionType": "histogramForESQL",
}
`);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,37 @@ describe('LensVisService suggestions', () => {
expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery);
});

test('should return histogramSuggestion even if suggestions returned by the api', async () => {
const lensVis = await getLensVisMock({
filters: [],
query: { esql: 'from the-data-view | limit 100' },
dataView: dataViewMock,
timeInterval: 'auto',
timeRange: {
from: '2023-09-03T08:00:00.000Z',
to: '2023-09-04T08:56:28.274Z',
},
breakdownField: undefined,
columns: [
{
id: 'var0',
name: 'var0',
meta: {
type: 'number',
},
},
],
isPlainRecord: true,
allSuggestions: allSuggestionsMock,
hasHistogramSuggestionForESQL: true,
});

expect(lensVis.currentSuggestionContext?.type).toBe(
UnifiedHistogramSuggestionType.histogramForESQL
);
expect(lensVis.currentSuggestionContext?.suggestion).toBeDefined();
});

test('should return histogramSuggestion if no suggestions returned by the api with a geo point breakdown field correctly', async () => {
const lensVis = await getLensVisMock({
filters: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export class LensVisService {
let currentSuggestion: Suggestion | undefined;

// takes lens suggestions if provided
const availableSuggestionsWithType: Array<{
let availableSuggestionsWithType: Array<{
suggestion: UnifiedHistogramSuggestionContext['suggestion'];
type: UnifiedHistogramSuggestionType;
}> = [];
Expand All @@ -254,6 +254,9 @@ export class LensVisService {
breakdownField,
});
if (histogramSuggestionForESQL) {
// In case if histogram suggestion, we want to empty the array and push the new suggestion
// to ensure that only the histogram suggestion is available
availableSuggestionsWithType = [];
availableSuggestionsWithType.push({
suggestion: histogramSuggestionForESQL,
type: UnifiedHistogramSuggestionType.histogramForESQL,
Expand Down