[8.x] [ES|QL] Omits sorting non sortable fields on Discover histogram (#195531) (#195809)

# Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] Omits sorting non sortable fields on Discover histogram (#195531)](https://github.com/elastic/kibana/pull/195531)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
2025-04-24 09:48:58 -04:00 · 2024-10-11 06:09:41 +11:00 · 2024-10-11 06:09:41 +11:00 · 048bf1e3a4
commit 048bf1e3a4
parent e2bed0f502
6 changed files with 179 additions and 6 deletions
--- a/packages/kbn-esql-utils/index.ts
+++ b/packages/kbn-esql-utils/index.ts
@ -29,6 +29,7 @@ export {
  isQueryWrappedByPipes,
  retrieveMetadataColumns,
  getQueryColumnsFromESQLQuery,
+  isESQLColumnSortable,
  TextBasedLanguages,
 } from './src';

--- a/packages/kbn-esql-utils/src/index.ts
+++ b/packages/kbn-esql-utils/src/index.ts
@ -31,3 +31,4 @@ export {
  getStartEndParams,
  hasStartEndParams,
 } from './utils/run_query';
+export { isESQLColumnSortable } from './utils/esql_fields_utils';
--- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts
+++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts
@ -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();
+    });
+  });
+});
--- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts
+++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts
@ -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;
+};
--- a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts
+++ b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts
@ -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', () => {
@ -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' },
@ -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',
@ -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);
+  });
 });
--- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts
+++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts
@ -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,
@ -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}\``
    );
  };