From ab3b74034201e100104f2db9f611bedfc6fb4c3b Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 22 Nov 2023 15:43:19 +0100 Subject: [PATCH 1/5] :bug: ignore drop statements for histogram --- packages/kbn-es-query/index.ts | 1 + .../src/es_query/es_aggregate_query.test.ts | 13 +++++++ .../src/es_query/es_aggregate_query.ts | 5 +++ packages/kbn-es-query/src/es_query/index.ts | 1 + .../layout/hooks/use_lens_suggestions.test.ts | 39 +++++++++++++++++++ .../layout/hooks/use_lens_suggestions.ts | 4 +- 6 files changed, 62 insertions(+), 1 deletion(-) diff --git a/packages/kbn-es-query/index.ts b/packages/kbn-es-query/index.ts index a14724fc3a748..f1ebaa9963106 100644 --- a/packages/kbn-es-query/index.ts +++ b/packages/kbn-es-query/index.ts @@ -56,6 +56,7 @@ export { getIndexPatternFromSQLQuery, getIndexPatternFromESQLQuery, getLanguageDisplayName, + cleanupESQLQuery, } from './src/es_query'; export { diff --git a/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts b/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts index ca98864aab446..34f183d78b7f2 100644 --- a/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts +++ b/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts @@ -12,6 +12,7 @@ import { getAggregateQueryMode, getIndexPatternFromSQLQuery, getIndexPatternFromESQLQuery, + cleanupESQLQuery, } from './es_aggregate_query'; describe('sql query helpers', () => { @@ -115,4 +116,16 @@ describe('sql query helpers', () => { expect(idxPattern9).toBe('foo-1, foo-2'); }); }); + + describe('cleanupESQLQuery', () => { + it('should not remove anything if a drop command is not present', () => { + expect(cleanupESQLQuery('from a | eval b = 1')).toBe('from a | eval b = 1'); + }); + + it('should remove multiple drop statement if present', () => { + expect(cleanupESQLQuery('from a | drop @timestamp | drop a | drop b | keep c | drop d')).toBe( + 'from a | keep c ' + ); + }); + }); }); diff --git a/packages/kbn-es-query/src/es_query/es_aggregate_query.ts b/packages/kbn-es-query/src/es_query/es_aggregate_query.ts index 27a5e790569c8..fe5efef2d55f9 100644 --- a/packages/kbn-es-query/src/es_query/es_aggregate_query.ts +++ b/packages/kbn-es-query/src/es_query/es_aggregate_query.ts @@ -66,3 +66,8 @@ export function getIndexPatternFromESQLQuery(esql?: string): string { } return ''; } + +export function cleanupESQLQuery(esql?: string): string { + const pipes = (esql || '').split('|'); + return pipes.filter((statement) => !/DROP\s/i.test(statement)).join('|'); +} diff --git a/packages/kbn-es-query/src/es_query/index.ts b/packages/kbn-es-query/src/es_query/index.ts index 22141a52e93f5..5bca96286bdf9 100644 --- a/packages/kbn-es-query/src/es_query/index.ts +++ b/packages/kbn-es-query/src/es_query/index.ts @@ -20,6 +20,7 @@ export { getIndexPatternFromSQLQuery, getLanguageDisplayName, getIndexPatternFromESQLQuery, + cleanupESQLQuery, } from './es_aggregate_query'; export { fromCombinedFilter } from './from_combined_filter'; export type { diff --git a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.test.ts b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.test.ts index d891c62df3514..a63c19383687a 100644 --- a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.test.ts +++ b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.test.ts @@ -137,6 +137,45 @@ describe('useLensSuggestions', () => { }); }); + test('should return histogramSuggestion even if the ESQL query contains a DROP @timestamp statement', async () => { + const firstMockReturn = undefined; + const secondMockReturn = allSuggestionsMock; + const lensSuggestionsApi = jest + .fn() + .mockReturnValueOnce(firstMockReturn) // will return to firstMockReturn object firstly + .mockReturnValueOnce(secondMockReturn); // will return to secondMockReturn object secondly + + renderHook(() => { + return useLensSuggestions({ + dataView: dataViewMock, + query: { esql: 'from the-data-view | DROP @timestamp | limit 100' }, + isPlainRecord: true, + columns: [ + { + id: 'var0', + name: 'var0', + meta: { + type: 'number', + }, + }, + ], + data: dataMock, + lensSuggestionsApi, + timeRange: { + from: '2023-09-03T08:00:00.000Z', + to: '2023-09-04T08:56:28.274Z', + }, + }); + }); + expect(lensSuggestionsApi).toHaveBeenLastCalledWith( + expect.objectContaining({ + query: { esql: expect.stringMatching('from the-data-view | limit 100 ') }, + }), + expect.anything(), + ['lnsDatatable'] + ); + }); + test('should not return histogramSuggestion if no suggestions returned by the api and transformational commands', async () => { const firstMockReturn = undefined; const secondMockReturn = allSuggestionsMock; diff --git a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts index 514c32cefcb80..4951d04333d68 100644 --- a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts +++ b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts @@ -11,6 +11,7 @@ import { AggregateQuery, isOfAggregateQueryType, getAggregateQueryMode, + cleanupESQLQuery, Query, TimeRange, } from '@kbn/es-query'; @@ -85,7 +86,8 @@ export const useLensSuggestions = ({ const interval = computeInterval(timeRange, data); const language = getAggregateQueryMode(query); - const histogramQuery = `${query[language]} + const safeQuery = cleanupESQLQuery(query[language]); + const histogramQuery = `${safeQuery} | EVAL timestamp=DATE_TRUNC(${interval}, ${dataView.timeFieldName}) | stats rows = count(*) by timestamp | rename timestamp as \`${dataView.timeFieldName} every ${interval}\``; const context = { dataViewSpec: dataView?.toSpec(), From c47ab3f8df5dd8e63d53167889faf6367181c70b Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 24 Nov 2023 14:00:53 +0100 Subject: [PATCH 2/5] :bug: Do not extends with timeField if the dataView is text based --- .../src/components/data_table.tsx | 4 ++-- .../src/components/data_table_columns.test.tsx | 10 ++++++++++ .../src/components/data_table_columns.tsx | 11 ++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 4ce88e52e6c8d..51947fade0135 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -618,8 +618,8 @@ export const UnifiedDataTable = ({ ); const visibleColumns = useMemo( - () => getVisibleColumns(displayedColumns, dataView, showTimeCol), - [dataView, displayedColumns, showTimeCol] + () => getVisibleColumns(displayedColumns, dataView, showTimeCol, isPlainRecord), + [dataView, displayedColumns, showTimeCol, isPlainRecord] ); const getCellValue = useCallback( diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx index 38cbdb5aeb63a..d625fee4ba252 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx @@ -108,6 +108,16 @@ describe('Data table columns', function () { ) as string[]; expect(actual).toEqual(['timestamp', 'extension', 'message']); }); + + it('returns grid columns without time column if the dataView is text-based', () => { + const actual = getVisibleColumns( + ['extension', 'message'], + dataViewWithTimefieldMock, + false, + true + ) as string[]; + expect(actual).toEqual(['extension', 'message']); + }); }); describe('column tokens', () => { diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx index 274b1148df4eb..c0bdd3fb8e78c 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx @@ -267,9 +267,18 @@ export function getEuiGridColumns({ ); } -export function getVisibleColumns(columns: string[], dataView: DataView, showTimeCol: boolean) { +export function getVisibleColumns( + columns: string[], + dataView: DataView, + showTimeCol: boolean, + isPlainRecord: boolean = false +) { const timeFieldName = dataView.timeFieldName; + if (isPlainRecord) { + return columns; + } + if (showTimeCol && timeFieldName && !columns.find((col) => col === timeFieldName)) { return [timeFieldName, ...columns]; } From 78fae0af98f53034fdeaaaf6db58721817bd2b20 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 27 Nov 2023 10:52:39 +0100 Subject: [PATCH 3/5] :bug: Hide timeField column only in the drop specific case --- .../__mocks__/data_view_without_timefield.ts | 64 ++++++++ .../src/components/data_table.tsx | 23 ++- .../components/data_table_columns.test.tsx | 142 ++++++++++++++++-- .../src/components/data_table_columns.tsx | 17 ++- 4 files changed, 229 insertions(+), 17 deletions(-) create mode 100644 packages/kbn-unified-data-table/__mocks__/data_view_without_timefield.ts diff --git a/packages/kbn-unified-data-table/__mocks__/data_view_without_timefield.ts b/packages/kbn-unified-data-table/__mocks__/data_view_without_timefield.ts new file mode 100644 index 0000000000000..cc07103a54486 --- /dev/null +++ b/packages/kbn-unified-data-table/__mocks__/data_view_without_timefield.ts @@ -0,0 +1,64 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +import { DataView } from '@kbn/data-views-plugin/public'; +import { buildDataViewMock } from '@kbn/discover-utils/src/__mocks__'; + +const fields = [ + { + name: '_index', + type: 'string', + scripted: false, + filterable: true, + }, + { + name: 'timestamp', + displayName: 'timestamp', + type: 'date', + scripted: false, + filterable: true, + aggregatable: true, + sortable: true, + }, + { + name: 'message', + displayName: 'message', + type: 'string', + scripted: false, + filterable: false, + }, + { + name: 'extension', + displayName: 'extension', + type: 'string', + scripted: false, + filterable: true, + aggregatable: true, + }, + { + name: 'bytes', + displayName: 'bytes', + type: 'number', + scripted: false, + filterable: true, + aggregatable: true, + }, + { + name: 'scripted', + displayName: 'scripted', + type: 'number', + scripted: true, + filterable: false, + }, +] as DataView['fields']; + +export const dataViewWithoutTimefieldMock = buildDataViewMock({ + name: 'index-pattern-without-timefield', + fields, + timeFieldName: undefined, +}); diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 51947fade0135..0993b399149a1 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -56,7 +56,12 @@ import { getDisplayedColumns } from '../utils/columns'; import { convertValueToString } from '../utils/convert_value_to_string'; import { getRowsPerPageOptions } from '../utils/rows_per_page'; import { getRenderCellValueFn } from '../utils/get_render_cell_value'; -import { getEuiGridColumns, getLeadControlColumns, getVisibleColumns } from './data_table_columns'; +import { + getEuiGridColumns, + getLeadControlColumns, + getVisibleColumns, + hasSourceTimeFieldValue, +} from './data_table_columns'; import { UnifiedDataTableContext } from '../table_context'; import { getSchemaDetectors } from './data_table_schema'; import { DataTableDocumentToolbarBtn } from './data_table_document_selection'; @@ -617,9 +622,21 @@ export const UnifiedDataTable = ({ [dataView, onFieldEdited, services.dataViewFieldEditor] ); + const shouldShowTimeField = useMemo( + () => + hasSourceTimeFieldValue( + displayedColumns, + dataView, + displayedRows, + showTimeCol, + isPlainRecord + ), + [dataView, displayedColumns, isPlainRecord, showTimeCol, displayedRows] + ); + const visibleColumns = useMemo( - () => getVisibleColumns(displayedColumns, dataView, showTimeCol, isPlainRecord), - [dataView, displayedColumns, showTimeCol, isPlainRecord] + () => getVisibleColumns(displayedColumns, dataView, shouldShowTimeField), + [dataView, displayedColumns, shouldShowTimeField] ); const getCellValue = useCallback( diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx index d625fee4ba252..544a24ed54970 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx @@ -7,10 +7,17 @@ */ import { dataViewMock } from '@kbn/discover-utils/src/__mocks__'; -import { getEuiGridColumns, getVisibleColumns } from './data_table_columns'; +import type { DataView } from '@kbn/data-views-plugin/public'; +import { + getEuiGridColumns, + getVisibleColumns, + hasSourceTimeFieldValue, +} from './data_table_columns'; import { dataViewWithTimefieldMock } from '../../__mocks__/data_view_with_timefield'; +import { dataViewWithoutTimefieldMock } from '../../__mocks__/data_view_without_timefield'; import { dataTableContextMock } from '../../__mocks__/table_context'; import { servicesMock } from '../../__mocks__/services'; +import { DataTableRecord } from '@kbn/discover-utils/types'; const columns = ['extension', 'message']; const columnsWithTimeCol = getVisibleColumns( @@ -108,15 +115,132 @@ describe('Data table columns', function () { ) as string[]; expect(actual).toEqual(['timestamp', 'extension', 'message']); }); + }); - it('returns grid columns without time column if the dataView is text-based', () => { - const actual = getVisibleColumns( - ['extension', 'message'], - dataViewWithTimefieldMock, - false, - true - ) as string[]; - expect(actual).toEqual(['extension', 'message']); + describe('hasSourceTimeFieldValue', () => { + function buildRows(dataView: DataView) { + const flattenedRow: Record = {}; + for (const field of dataView.fields) { + flattenedRow[field.name] = ''; + } + return [{ flattened: flattenedRow }] as DataTableRecord[]; + } + + describe('dataView with timeField', () => { + it('should forward showTimeCol if no _source columns is passed', () => { + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['extension', 'message'], + dataViewWithTimefieldMock, + buildRows(dataViewWithTimefieldMock), + showTimeCol, + false + ) + ).toBe(showTimeCol); + } + }); + + it('should forward showTimeCol if no _source columns is passed, text-based datasource', () => { + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['extension', 'message'], + dataViewWithTimefieldMock, + buildRows(dataViewWithTimefieldMock), + showTimeCol, + true + ) + ).toBe(showTimeCol); + } + }); + + it('should forward showTimeCol if _source column is passed', () => { + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['_source'], + dataViewWithTimefieldMock, + buildRows(dataViewWithTimefieldMock), + showTimeCol, + false + ) + ).toBe(showTimeCol); + } + }); + + it('should return true if _source column is passed, text-based datasource', () => { + // ... | DROP @timestamp test case + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['_source'], + dataViewWithTimefieldMock, + buildRows(dataViewWithTimefieldMock), + showTimeCol, + true + ) + ).toBe(true); + } + }); + }); + + describe('dataView without timeField', () => { + it('should forward showTimeCol if no _source columns is passed', () => { + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['extension', 'message'], + dataViewWithoutTimefieldMock, + buildRows(dataViewWithoutTimefieldMock), + showTimeCol, + false + ) + ).toBe(showTimeCol); + } + }); + + it('should forward showTimeCol if no _source columns is passed, text-based datasource', () => { + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['extension', 'message'], + dataViewWithoutTimefieldMock, + buildRows(dataViewWithoutTimefieldMock), + showTimeCol, + true + ) + ).toBe(showTimeCol); + } + }); + + it('should forward showTimeCol if _source column is passed', () => { + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['_source'], + dataViewWithoutTimefieldMock, + buildRows(dataViewWithoutTimefieldMock), + showTimeCol, + false + ) + ).toBe(showTimeCol); + } + }); + + it('should return false if _source column is passed, text-based datasource', () => { + for (const showTimeCol of [true, false]) { + expect( + hasSourceTimeFieldValue( + ['_source'], + dataViewWithoutTimefieldMock, + buildRows(dataViewWithoutTimefieldMock), + showTimeCol, + true + ) + ).toBe(showTimeCol); + } + }); }); }); diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx index c0bdd3fb8e78c..b37b14c08519e 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx @@ -18,6 +18,7 @@ import { import type { DataView } from '@kbn/data-views-plugin/public'; import { ToastsStart, IUiSettingsClient } from '@kbn/core/public'; import { DocViewFilterFn } from '@kbn/unified-doc-viewer/types'; +import { DataTableRecord } from '@kbn/discover-utils/types'; import { ExpandButton } from './data_table_expand_button'; import { UnifiedDataTableSettings } from '../types'; import type { ValueToStringConverter, DataTableColumnTypes } from '../types'; @@ -267,17 +268,23 @@ export function getEuiGridColumns({ ); } -export function getVisibleColumns( +export function hasSourceTimeFieldValue( columns: string[], dataView: DataView, + rows: DataTableRecord[], showTimeCol: boolean, - isPlainRecord: boolean = false + isPlainRecord: boolean ) { const timeFieldName = dataView.timeFieldName; - - if (isPlainRecord) { - return columns; + if (!isPlainRecord || !columns.includes('_source') || !timeFieldName) { + return showTimeCol; } + // flattened is the _source content in this case + return rows.some((row) => timeFieldName in row.flattened); +} + +export function getVisibleColumns(columns: string[], dataView: DataView, showTimeCol: boolean) { + const timeFieldName = dataView.timeFieldName; if (showTimeCol && timeFieldName && !columns.find((col) => col === timeFieldName)) { return [timeFieldName, ...columns]; From fc4c37f001bc1e65e9ba50aa698c6e952be49529 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 27 Nov 2023 14:50:15 +0100 Subject: [PATCH 4/5] :ok_hand: Integrate feedback for fn name --- packages/kbn-es-query/index.ts | 2 +- .../src/es_query/es_aggregate_query.test.ts | 14 ++++++++------ .../src/es_query/es_aggregate_query.ts | 2 +- packages/kbn-es-query/src/es_query/index.ts | 2 +- .../public/layout/hooks/use_lens_suggestions.ts | 4 ++-- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/kbn-es-query/index.ts b/packages/kbn-es-query/index.ts index f1ebaa9963106..ac0a078e34d94 100644 --- a/packages/kbn-es-query/index.ts +++ b/packages/kbn-es-query/index.ts @@ -56,7 +56,7 @@ export { getIndexPatternFromSQLQuery, getIndexPatternFromESQLQuery, getLanguageDisplayName, - cleanupESQLQuery, + cleanupESQLQueryForLensSuggestions, } from './src/es_query'; export { diff --git a/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts b/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts index 34f183d78b7f2..504e6a5c93d44 100644 --- a/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts +++ b/packages/kbn-es-query/src/es_query/es_aggregate_query.test.ts @@ -12,7 +12,7 @@ import { getAggregateQueryMode, getIndexPatternFromSQLQuery, getIndexPatternFromESQLQuery, - cleanupESQLQuery, + cleanupESQLQueryForLensSuggestions, } from './es_aggregate_query'; describe('sql query helpers', () => { @@ -117,15 +117,17 @@ describe('sql query helpers', () => { }); }); - describe('cleanupESQLQuery', () => { + describe('cleanupESQLQueryForLensSuggestions', () => { it('should not remove anything if a drop command is not present', () => { - expect(cleanupESQLQuery('from a | eval b = 1')).toBe('from a | eval b = 1'); + expect(cleanupESQLQueryForLensSuggestions('from a | eval b = 1')).toBe('from a | eval b = 1'); }); it('should remove multiple drop statement if present', () => { - expect(cleanupESQLQuery('from a | drop @timestamp | drop a | drop b | keep c | drop d')).toBe( - 'from a | keep c ' - ); + expect( + cleanupESQLQueryForLensSuggestions( + 'from a | drop @timestamp | drop a | drop b | keep c | drop d' + ) + ).toBe('from a | keep c '); }); }); }); diff --git a/packages/kbn-es-query/src/es_query/es_aggregate_query.ts b/packages/kbn-es-query/src/es_query/es_aggregate_query.ts index fe5efef2d55f9..f746505896360 100644 --- a/packages/kbn-es-query/src/es_query/es_aggregate_query.ts +++ b/packages/kbn-es-query/src/es_query/es_aggregate_query.ts @@ -67,7 +67,7 @@ export function getIndexPatternFromESQLQuery(esql?: string): string { return ''; } -export function cleanupESQLQuery(esql?: string): string { +export function cleanupESQLQueryForLensSuggestions(esql?: string): string { const pipes = (esql || '').split('|'); return pipes.filter((statement) => !/DROP\s/i.test(statement)).join('|'); } diff --git a/packages/kbn-es-query/src/es_query/index.ts b/packages/kbn-es-query/src/es_query/index.ts index 5bca96286bdf9..18009145a432f 100644 --- a/packages/kbn-es-query/src/es_query/index.ts +++ b/packages/kbn-es-query/src/es_query/index.ts @@ -20,7 +20,7 @@ export { getIndexPatternFromSQLQuery, getLanguageDisplayName, getIndexPatternFromESQLQuery, - cleanupESQLQuery, + cleanupESQLQueryForLensSuggestions, } from './es_aggregate_query'; export { fromCombinedFilter } from './from_combined_filter'; export type { diff --git a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts index cc32ba2f820d5..063e1b7ef89a2 100644 --- a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts +++ b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts @@ -11,7 +11,7 @@ import { AggregateQuery, isOfAggregateQueryType, getAggregateQueryMode, - cleanupESQLQuery, + cleanupESQLQueryForLensSuggestions, Query, TimeRange, } from '@kbn/es-query'; @@ -86,7 +86,7 @@ export const useLensSuggestions = ({ const interval = computeInterval(timeRange, data); const language = getAggregateQueryMode(query); - const safeQuery = cleanupESQLQuery(query[language]); + const safeQuery = cleanupESQLQueryForLensSuggestions(query[language]); const esqlQuery = `${safeQuery} | EVAL timestamp=DATE_TRUNC(${interval}, ${dataView.timeFieldName}) | stats rows = count(*) by timestamp | rename timestamp as \`${dataView.timeFieldName} every ${interval}\``; const context = { dataViewSpec: dataView?.toSpec(), From 4db4e3aff4d05707683e2aeb76e7053510423564 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 27 Nov 2023 14:50:39 +0100 Subject: [PATCH 5/5] :ok_hand: Integrate feedback on fast lookups --- .../src/components/data_table.tsx | 10 ++------ .../components/data_table_columns.test.tsx | 25 +++++++++---------- .../src/components/data_table_columns.tsx | 8 +++--- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 0993b399149a1..1f05601fd6892 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -624,14 +624,8 @@ export const UnifiedDataTable = ({ const shouldShowTimeField = useMemo( () => - hasSourceTimeFieldValue( - displayedColumns, - dataView, - displayedRows, - showTimeCol, - isPlainRecord - ), - [dataView, displayedColumns, isPlainRecord, showTimeCol, displayedRows] + hasSourceTimeFieldValue(displayedColumns, dataView, columnTypes, showTimeCol, isPlainRecord), + [dataView, displayedColumns, isPlainRecord, showTimeCol, columnTypes] ); const visibleColumns = useMemo( diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx index 544a24ed54970..e1d7082353e1c 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx @@ -17,7 +17,6 @@ import { dataViewWithTimefieldMock } from '../../__mocks__/data_view_with_timefi import { dataViewWithoutTimefieldMock } from '../../__mocks__/data_view_without_timefield'; import { dataTableContextMock } from '../../__mocks__/table_context'; import { servicesMock } from '../../__mocks__/services'; -import { DataTableRecord } from '@kbn/discover-utils/types'; const columns = ['extension', 'message']; const columnsWithTimeCol = getVisibleColumns( @@ -118,12 +117,12 @@ describe('Data table columns', function () { }); describe('hasSourceTimeFieldValue', () => { - function buildRows(dataView: DataView) { - const flattenedRow: Record = {}; + function buildColumnTypes(dataView: DataView) { + const columnTypes: Record = {}; for (const field of dataView.fields) { - flattenedRow[field.name] = ''; + columnTypes[field.name] = ''; } - return [{ flattened: flattenedRow }] as DataTableRecord[]; + return columnTypes; } describe('dataView with timeField', () => { @@ -133,7 +132,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['extension', 'message'], dataViewWithTimefieldMock, - buildRows(dataViewWithTimefieldMock), + buildColumnTypes(dataViewWithTimefieldMock), showTimeCol, false ) @@ -147,7 +146,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['extension', 'message'], dataViewWithTimefieldMock, - buildRows(dataViewWithTimefieldMock), + buildColumnTypes(dataViewWithTimefieldMock), showTimeCol, true ) @@ -161,7 +160,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['_source'], dataViewWithTimefieldMock, - buildRows(dataViewWithTimefieldMock), + buildColumnTypes(dataViewWithTimefieldMock), showTimeCol, false ) @@ -176,7 +175,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['_source'], dataViewWithTimefieldMock, - buildRows(dataViewWithTimefieldMock), + buildColumnTypes(dataViewWithTimefieldMock), showTimeCol, true ) @@ -192,7 +191,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['extension', 'message'], dataViewWithoutTimefieldMock, - buildRows(dataViewWithoutTimefieldMock), + buildColumnTypes(dataViewWithoutTimefieldMock), showTimeCol, false ) @@ -206,7 +205,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['extension', 'message'], dataViewWithoutTimefieldMock, - buildRows(dataViewWithoutTimefieldMock), + buildColumnTypes(dataViewWithoutTimefieldMock), showTimeCol, true ) @@ -220,7 +219,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['_source'], dataViewWithoutTimefieldMock, - buildRows(dataViewWithoutTimefieldMock), + buildColumnTypes(dataViewWithoutTimefieldMock), showTimeCol, false ) @@ -234,7 +233,7 @@ describe('Data table columns', function () { hasSourceTimeFieldValue( ['_source'], dataViewWithoutTimefieldMock, - buildRows(dataViewWithoutTimefieldMock), + buildColumnTypes(dataViewWithoutTimefieldMock), showTimeCol, true ) diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx index b37b14c08519e..f7a3d41bf330e 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx @@ -18,7 +18,6 @@ import { import type { DataView } from '@kbn/data-views-plugin/public'; import { ToastsStart, IUiSettingsClient } from '@kbn/core/public'; import { DocViewFilterFn } from '@kbn/unified-doc-viewer/types'; -import { DataTableRecord } from '@kbn/discover-utils/types'; import { ExpandButton } from './data_table_expand_button'; import { UnifiedDataTableSettings } from '../types'; import type { ValueToStringConverter, DataTableColumnTypes } from '../types'; @@ -271,16 +270,15 @@ export function getEuiGridColumns({ export function hasSourceTimeFieldValue( columns: string[], dataView: DataView, - rows: DataTableRecord[], + columnTypes: DataTableColumnTypes | undefined, showTimeCol: boolean, isPlainRecord: boolean ) { const timeFieldName = dataView.timeFieldName; - if (!isPlainRecord || !columns.includes('_source') || !timeFieldName) { + if (!isPlainRecord || !columns.includes('_source') || !timeFieldName || !columnTypes) { return showTimeCol; } - // flattened is the _source content in this case - return rows.some((row) => timeFieldName in row.flattened); + return timeFieldName in columnTypes; } export function getVisibleColumns(columns: string[], dataView: DataView, showTimeCol: boolean) {