Skip to content

Commit

Permalink
[ES|QL] Omits sorting non sortable fields on Discover histogram (#195531
Browse files Browse the repository at this point in the history
)

## Summary

Closes #195510

Sorting by `geo_point`, tsdb counter fields and _source is not supported
in ES|QL. This PR is omitting the sorting for these types and now the
breakdown works fine.

<img width="2500" alt="image"
src="https://github.com/user-attachments/assets/1526f516-5f8d-491d-8b77-1f9734ce83a4">

Note: This behavior is unreleased.

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 2bb9c3c)
  • Loading branch information
stratoula committed Oct 10, 2024
1 parent 4b9dae8 commit 0fab85f
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 6 deletions.
1 change: 1 addition & 0 deletions packages/kbn-esql-utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export {
isQueryWrappedByPipes,
retrieveMetadataColumns,
getQueryColumnsFromESQLQuery,
isESQLColumnSortable,
TextBasedLanguages,
} from './src';

Expand Down
1 change: 1 addition & 0 deletions packages/kbn-esql-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ export {
getStartEndParams,
hasStartEndParams,
} from './utils/run_query';
export { isESQLColumnSortable } from './utils/esql_fields_utils';
66 changes: 66 additions & 0 deletions packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* 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';

describe('esql fields helpers', () => {
describe('isESQLColumnSortable', () => {
it('returns false for geo fields', () => {
const geoField = {
id: 'geo.coordinates',
name: 'geo.coordinates',
meta: {
type: 'geo_point',
esType: 'geo_point',
},
isNull: false,
} as DatatableColumn;
expect(isESQLColumnSortable(geoField)).toBeFalsy();
});

it('returns false for source fields', () => {
const sourceField = {
id: '_source',
name: '_source',
meta: {
type: '_source',
esType: '_source',
},
isNull: false,
} as DatatableColumn;
expect(isESQLColumnSortable(sourceField)).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(isESQLColumnSortable(tsdbField)).toBeFalsy();
});

it('returns true for everything else', () => {
const keywordField = {
id: 'sortable',
name: 'sortable',
meta: {
type: 'string',
esType: 'keyword',
},
isNull: false,
} as DatatableColumn;
expect(isESQLColumnSortable(keywordField)).toBeTruthy();
});
});
});
40 changes: 40 additions & 0 deletions packages/kbn-esql-utils/src/utils/esql_fields_utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

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_';

/**
* Check if a column is sortable.
*
* @param column The DatatableColumn of the field.
* @returns True if the column is sortable, false otherwise.
*/

export const isESQLColumnSortable = (column: DatatableColumn): boolean => {
// We don't allow sorting on spatial fields
if (SPATIAL_FIELDS.includes(column.meta?.type)) {
return false;
}

// we don't allow sorting on the _source field
if (column.meta?.type === SOURCE_FIELD) {
return false;
}

// we don't allow sorting 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 @@ -12,6 +12,7 @@ import { DataViewField } from '@kbn/data-views-plugin/common';
import { deepMockedFields, buildDataViewMock } from '@kbn/discover-utils/src/__mocks__';
import { allSuggestionsMock } from '../__mocks__/suggestions';
import { getLensVisMock } from '../__mocks__/lens_vis';
import { convertDatatableColumnToDataViewFieldSpec } from '@kbn/data-view-utils';
import { UnifiedHistogramSuggestionType } from '../types';

describe('LensVisService suggestions', () => {
Expand Down Expand Up @@ -198,6 +199,11 @@ describe('LensVisService suggestions', () => {
});

test('should return histogramSuggestion if no suggestions returned by the api with the breakdown field if it is given', async () => {
const breakdown = convertDatatableColumnToDataViewFieldSpec({
name: 'var0',
id: 'var0',
meta: { type: 'number' },
});
const lensVis = await getLensVisMock({
filters: [],
query: { esql: 'from the-data-view | limit 100' },
Expand All @@ -207,7 +213,7 @@ describe('LensVisService suggestions', () => {
from: '2023-09-03T08:00:00.000Z',
to: '2023-09-04T08:56:28.274Z',
},
breakdownField: { name: 'var0' } as DataViewField,
breakdownField: breakdown as DataViewField,
columns: [
{
id: 'var0',
Expand Down Expand Up @@ -247,4 +253,54 @@ describe('LensVisService suggestions', () => {

expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery);
});

test('should return histogramSuggestion if no suggestions returned by the api with a geo point breakdown field correctly', 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: { name: 'coordinates' } as DataViewField,
columns: [
{
id: 'coordinates',
name: 'coordinates',
meta: {
type: 'geo_point',
},
},
],
isPlainRecord: true,
allSuggestions: [],
hasHistogramSuggestionForESQL: true,
});

expect(lensVis.currentSuggestionContext?.type).toBe(
UnifiedHistogramSuggestionType.histogramForESQL
);
expect(lensVis.currentSuggestionContext?.suggestion).toBeDefined();
expect(lensVis.currentSuggestionContext?.suggestion?.visualizationState).toHaveProperty(
'layers',
[
{
layerId: '662552df-2cdc-4539-bf3b-73b9f827252c',
seriesType: 'bar_stacked',
xAccessor: '@timestamp every 30 second',
accessors: ['results'],
layerType: 'data',
},
]
);

const histogramQuery = {
esql: `from the-data-view | limit 100
| EVAL timestamp=DATE_TRUNC(30 minute, @timestamp) | stats results = count(*) by timestamp, \`coordinates\` | rename timestamp as \`@timestamp every 30 minute\``,
};

expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery);
});
});
19 changes: 14 additions & 5 deletions src/plugins/unified_histogram/public/services/lens_vis_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@

import { BehaviorSubject, distinctUntilChanged, map, Observable } from 'rxjs';
import { isEqual } from 'lodash';
import { removeDropCommandsFromESQLQuery, appendToESQLQuery } from '@kbn/esql-utils';
import {
removeDropCommandsFromESQLQuery,
appendToESQLQuery,
isESQLColumnSortable,
} from '@kbn/esql-utils';
import type { DataView, DataViewField } from '@kbn/data-views-plugin/common';
import type {
CountIndexPatternColumn,
Expand Down Expand Up @@ -553,12 +557,17 @@ export class LensVisService {
const queryInterval = interval ?? computeInterval(timeRange, this.services.data);
const language = getAggregateQueryMode(query);
const safeQuery = removeDropCommandsFromESQLQuery(query[language]);
const breakdown = breakdownColumn
? `, \`${breakdownColumn.name}\` | sort \`${breakdownColumn.name}\` asc`
: '';
const breakdown = breakdownColumn ? `, \`${breakdownColumn.name}\`` : '';

// sort by breakdown column if it's sortable
const sortBy =
breakdownColumn && isESQLColumnSortable(breakdownColumn)
? ` | sort \`${breakdownColumn.name}\` asc`
: '';

return appendToESQLQuery(
safeQuery,
`| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\``
`| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown}${sortBy} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\``
);
};

Expand Down

0 comments on commit 0fab85f

Please sign in to comment.