From 0da7abf53e47613e670df5f71621fa92ae7e420b Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 31 May 2023 15:09:35 +0300 Subject: [PATCH 1/2] Enable Document view for non transformational commands --- .../use_test_based_query_language.test.tsx | 10 ++-- .../hooks/use_text_based_query_language.ts | 60 ++++++++++--------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx index 05ef44a03f01b..a3dcb005cf764 100644 --- a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx @@ -193,7 +193,7 @@ describe('useTextBasedQueryLanguage', () => { query: { sql: 'SELECT field1 from the-data-view-title WHERE field1=1' }, }); - await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(0)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); }); test('if its not a text based query coming along, it should be ignored', async () => { const { replaceUrlState, stateContainer } = renderHookWithContext(false); @@ -270,7 +270,7 @@ describe('useTextBasedQueryLanguage', () => { }); await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); expect(replaceUrlState).toHaveBeenCalledWith({ - columns: ['field1'], + index: 'the-data-view-id', }); }); @@ -286,7 +286,7 @@ describe('useTextBasedQueryLanguage', () => { fetchStatus: FetchStatus.LOADING, query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' }, }); - await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); documents$.next({ recordRawType: RecordRawType.PLAIN, fetchStatus: FetchStatus.COMPLETE, @@ -299,7 +299,7 @@ describe('useTextBasedQueryLanguage', () => { ], query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' }, }); - await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(3)); stateContainer.appState.getState = jest.fn(() => { return { columns: ['field1', 'field2'], index: 'the-data-view-id' }; }); @@ -335,7 +335,7 @@ describe('useTextBasedQueryLanguage', () => { query: { sql: 'SELECT field1 from the-data-view-title' }, }); - await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(3)); expect(replaceUrlState).toHaveBeenCalledWith({ columns: ['field1'], }); diff --git a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts index 1421e9a54dca7..99794a28b1ae1 100644 --- a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts +++ b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts @@ -6,13 +6,7 @@ * Side Public License, v 1. */ import { isEqual } from 'lodash'; -import { - isOfAggregateQueryType, - getIndexPatternFromSQLQuery, - getIndexPatternFromESQLQuery, - AggregateQuery, - Query, -} from '@kbn/es-query'; +import { isOfAggregateQueryType, AggregateQuery, Query } from '@kbn/es-query'; import { useCallback, useEffect, useRef } from 'react'; import type { DataViewsContract } from '@kbn/data-views-plugin/public'; import { VIEW_MODE } from '@kbn/saved-search-plugin/public'; @@ -22,6 +16,7 @@ import { getValidViewMode } from '../utils/get_valid_view_mode'; import { FetchStatus } from '../../types'; const MAX_NUM_OF_COLUMNS = 50; +const TRANSFORMATIONAL_COMMANDS = ['stats', 'project']; /** * Hook to take care of text based query language state transformations when a new result is returned @@ -38,7 +33,6 @@ export function useTextBasedQueryLanguage({ columns: [], query: undefined, }); - const indexTitle = useRef(''); const savedSearch = useSavedSearchInitial(); const cleanup = useCallback(() => { @@ -66,46 +60,54 @@ export function useTextBasedQueryLanguage({ ('sql' in query || 'esql' in query); const hasResults = next.result?.length && next.fetchStatus === FetchStatus.COMPLETE; const initialFetch = !prev.current.columns.length; - + let queryHasTransformationalCommands = 'sql' in query; + if ('esql' in query) { + TRANSFORMATIONAL_COMMANDS.forEach((command: string) => { + if (query.esql.toLowerCase().includes(command)) { + queryHasTransformationalCommands = true; + return; + } + }); + } if (isTextBasedQueryLang) { if (hasResults) { // check if state needs to contain column transformation due to a different columns in the resultset const firstRow = next.result![0]; const firstRowColumns = Object.keys(firstRow.raw).slice(0, MAX_NUM_OF_COLUMNS); - if ( - !isEqual(firstRowColumns, prev.current.columns) && - !isEqual(query, prev.current.query) - ) { - prev.current = { columns: firstRowColumns, query }; - nextColumns = firstRowColumns; - } + if (!queryHasTransformationalCommands) { + prev.current = { columns: [], query }; + nextColumns = []; + } else { + if ( + !isEqual(firstRowColumns, prev.current.columns) && + !isEqual(query, prev.current.query) + ) { + prev.current = { columns: firstRowColumns, query }; + nextColumns = firstRowColumns; + } - if (firstRowColumns && initialFetch) { - prev.current = { columns: firstRowColumns, query }; + if (firstRowColumns && initialFetch) { + prev.current = { columns: firstRowColumns, query }; + } } } - const indexPatternFromQuery = - 'sql' in query - ? getIndexPatternFromSQLQuery(query.sql) - : getIndexPatternFromESQLQuery(query.esql); const dataViewObj = stateContainer.internalState.getState().dataView!; // don't set the columns on initial fetch, to prevent overwriting existing state const addColumnsToState = Boolean( - nextColumns.length && (!initialFetch || !stateColumns?.length) + (nextColumns.length && + (!initialFetch || !stateColumns?.length) && + queryHasTransformationalCommands) || + !queryHasTransformationalCommands ); // no need to reset index to state if it hasn't changed const addDataViewToState = Boolean(dataViewObj?.id !== index) || initialFetch; - const queryChanged = indexPatternFromQuery !== indexTitle.current; - if (!addColumnsToState && !queryChanged) { + const columnsHaveChanged = !isEqual(stateColumns, nextColumns); + if (!addDataViewToState && !columnsHaveChanged) { return; } - if (queryChanged) { - indexTitle.current = indexPatternFromQuery; - } - const nextState = { ...(addDataViewToState && { index: dataViewObj.id }), ...(addColumnsToState && { columns: nextColumns }), From db4cd754d56aa1a8d46a35b95818d3033390957b Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Thu, 1 Jun 2023 11:27:16 +0300 Subject: [PATCH 2/2] Fixes --- .../use_test_based_query_language.test.tsx | 8 ++-- .../hooks/use_text_based_query_language.ts | 38 +++++++++++++------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx index a3dcb005cf764..3ca0002bc7f44 100644 --- a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx @@ -270,7 +270,7 @@ describe('useTextBasedQueryLanguage', () => { }); await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); expect(replaceUrlState).toHaveBeenCalledWith({ - index: 'the-data-view-id', + columns: ['field1', 'field2'], }); }); @@ -286,7 +286,7 @@ describe('useTextBasedQueryLanguage', () => { fetchStatus: FetchStatus.LOADING, query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' }, }); - await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(0)); documents$.next({ recordRawType: RecordRawType.PLAIN, fetchStatus: FetchStatus.COMPLETE, @@ -299,7 +299,7 @@ describe('useTextBasedQueryLanguage', () => { ], query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' }, }); - await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(3)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); stateContainer.appState.getState = jest.fn(() => { return { columns: ['field1', 'field2'], index: 'the-data-view-id' }; }); @@ -335,7 +335,7 @@ describe('useTextBasedQueryLanguage', () => { query: { sql: 'SELECT field1 from the-data-view-title' }, }); - await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(3)); + await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1)); expect(replaceUrlState).toHaveBeenCalledWith({ columns: ['field1'], }); diff --git a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts index 99794a28b1ae1..31927914c52d7 100644 --- a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts +++ b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts @@ -6,7 +6,12 @@ * Side Public License, v 1. */ import { isEqual } from 'lodash'; -import { isOfAggregateQueryType, AggregateQuery, Query } from '@kbn/es-query'; +import { + isOfAggregateQueryType, + AggregateQuery, + Query, + getAggregateQueryMode, +} from '@kbn/es-query'; import { useCallback, useEffect, useRef } from 'react'; import type { DataViewsContract } from '@kbn/data-views-plugin/public'; import { VIEW_MODE } from '@kbn/saved-search-plugin/public'; @@ -29,10 +34,15 @@ export function useTextBasedQueryLanguage({ stateContainer: DiscoverStateContainer; dataViews: DataViewsContract; }) { - const prev = useRef<{ query: AggregateQuery | Query | undefined; columns: string[] }>({ + const prev = useRef<{ + query: AggregateQuery | Query | undefined; + columns: string[]; + }>({ columns: [], query: undefined, }); + const queryString = useRef(''); + const isPrevTransformationalMode = useRef(true); const savedSearch = useSavedSearchInitial(); const cleanup = useCallback(() => { @@ -51,7 +61,7 @@ export function useTextBasedQueryLanguage({ if (!query || next.fetchStatus === FetchStatus.ERROR) { return; } - const { columns: stateColumns, index, viewMode } = stateContainer.appState.getState(); + const { index, viewMode } = stateContainer.appState.getState(); let nextColumns: string[] = []; const isTextBasedQueryLang = recordRawType === 'plain' && @@ -70,6 +80,7 @@ export function useTextBasedQueryLanguage({ }); } if (isTextBasedQueryLang) { + const language = getAggregateQueryMode(query); if (hasResults) { // check if state needs to contain column transformation due to a different columns in the resultset const firstRow = next.result![0]; @@ -96,18 +107,21 @@ export function useTextBasedQueryLanguage({ // don't set the columns on initial fetch, to prevent overwriting existing state const addColumnsToState = Boolean( - (nextColumns.length && - (!initialFetch || !stateColumns?.length) && - queryHasTransformationalCommands) || - !queryHasTransformationalCommands + (nextColumns.length && queryHasTransformationalCommands) || + (!queryHasTransformationalCommands && isPrevTransformationalMode.current) ); + const queryChanged = query[language] !== queryString.current; // no need to reset index to state if it hasn't changed - const addDataViewToState = Boolean(dataViewObj?.id !== index) || initialFetch; - const columnsHaveChanged = !isEqual(stateColumns, nextColumns); - if (!addDataViewToState && !columnsHaveChanged) { + const addDataViewToState = Boolean(dataViewObj?.id !== index); + if (!queryChanged && !addColumnsToState) { return; } + if (queryChanged) { + queryString.current = query[language]; + isPrevTransformationalMode.current = queryHasTransformationalCommands; + } + const nextState = { ...(addDataViewToState && { index: dataViewObj.id }), ...(addColumnsToState && { columns: nextColumns }), @@ -115,7 +129,9 @@ export function useTextBasedQueryLanguage({ viewMode: getValidViewMode({ viewMode, isTextBasedQueryMode: true }), }), }; - stateContainer.appState.replaceUrlState(nextState); + if (Object.keys(nextState).length !== 0) { + stateContainer.appState.replaceUrlState(nextState); + } } else { // cleanup for a "regular" query cleanup();