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

[Discover][ES|QL] Reset selected fields when modifying the ES|QL query #185997

Merged
merged 17 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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 .buildkite/ftr_configs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ enabled:
- test/functional/apps/discover/ccs_compatibility/config.ts
- test/functional/apps/discover/classic/config.ts
- test/functional/apps/discover/embeddable/config.ts
- test/functional/apps/discover/esql/config.ts
- test/functional/apps/discover/group1/config.ts
- test/functional/apps/discover/group2_data_grid1/config.ts
- test/functional/apps/discover/group2_data_grid2/config.ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,11 @@ describe('useEsqlMode', () => {
});
});

test('changing an ES|QL query with same result columns should not change state when loading and finished', async () => {
test('changing an ES|QL query with same result columns should change state when loading and finished', async () => {
jughosta marked this conversation as resolved.
Show resolved Hide resolved
const { replaceUrlState, stateContainer } = renderHookWithContext(false);
const documents$ = stateContainer.dataState.data$.documents$;
stateContainer.dataState.data$.documents$.next(msgComplete);
replaceUrlState.mockReset();

documents$.next({
fetchStatus: FetchStatus.PARTIAL,
Expand All @@ -166,7 +167,54 @@ describe('useEsqlMode', () => {
],
query: { esql: 'from the-data-view-2' },
});
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));

await waitFor(() => {
expect(replaceUrlState).toHaveBeenCalledWith({
columns: [],
});
});
});

test('changing a text based query with no transformational commands should not change state when loading and finished if index pattern is the same', async () => {
jughosta marked this conversation as resolved.
Show resolved Hide resolved
const { replaceUrlState, stateContainer } = renderHookWithContext(false);
const documents$ = stateContainer.dataState.data$.documents$;
stateContainer.dataState.data$.documents$.next(msgComplete);
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(0));
replaceUrlState.mockReset();

documents$.next({
fetchStatus: FetchStatus.PARTIAL,
result: [
{
id: '1',
raw: { field1: 1 },
flattened: { field1: 1 },
} as unknown as DataTableRecord,
],
// non transformational command
query: { esql: 'from the-data-view-title | where field1 > 0' },
});
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(0));
replaceUrlState.mockReset();

documents$.next({
fetchStatus: FetchStatus.PARTIAL,
result: [
{
id: '1',
raw: { field1: 1 },
flattened: { field1: 1 },
} as unknown as DataTableRecord,
],
// non transformational command
query: { esql: 'from the-data-view-title2 | where field1 > 0' },
});
await waitFor(() => {
expect(replaceUrlState).toHaveBeenCalledWith({
columns: [],
});
});
});

test('only changing an ES|QL query with same result columns should not change columns', async () => {
Expand Down Expand Up @@ -268,7 +316,13 @@ describe('useEsqlMode', () => {
query: { esql: 'from the-data-view-title | keep field 1 | WHERE field1=1' },
});

expect(replaceUrlState).toHaveBeenCalledTimes(0);
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));
await waitFor(() => {
expect(replaceUrlState).toHaveBeenCalledWith({
columns: ['field1', 'field2'],
});
});
replaceUrlState.mockReset();

documents$.next({
fetchStatus: FetchStatus.PARTIAL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
*/

import { isEqual } from 'lodash';
import { isOfAggregateQueryType, getAggregateQueryMode } from '@kbn/es-query';
import { hasTransformationalCommand } from '@kbn/esql-utils';
import { isOfAggregateQueryType } from '@kbn/es-query';
import { hasTransformationalCommand, getIndexPatternFromESQLQuery } from '@kbn/esql-utils';
import { useCallback, useEffect, useRef } from 'react';
import type { DataViewsContract } from '@kbn/data-views-plugin/public';
import { switchMap } from 'rxjs';
Expand All @@ -31,9 +31,9 @@ export function useEsqlMode({
}) {
const prev = useRef<{
query: string;
columns: string[];
recentlyUpdatedToColumns: string[];
}>({
columns: [],
recentlyUpdatedToColumns: [],
query: '',
});
const initialFetch = useRef<boolean>(true);
Expand All @@ -43,9 +43,10 @@ export function useEsqlMode({
if (prev.current.query) {
// cleanup when it's not an ES|QL query
prev.current = {
columns: [],
recentlyUpdatedToColumns: [],
query: '',
};
initialFetch.current = true;
}
}, []);

Expand All @@ -57,55 +58,61 @@ export function useEsqlMode({
if (!query || next.fetchStatus === FetchStatus.ERROR) {
return;
}

const sendComplete = () => {
stateContainer.dataState.data$.documents$.next({
...next,
fetchStatus: FetchStatus.COMPLETE,
});
};

const { viewMode } = stateContainer.appState.getState();
let nextColumns: string[] = [];
const isEsqlQuery = isOfAggregateQueryType(query);
const hasResults = Boolean(next.result?.length);
let queryHasTransformationalCommands = false;
if ('esql' in query) {
if (hasTransformationalCommand(query.esql)) {
queryHasTransformationalCommands = true;
}
}

if (isEsqlQuery) {
const language = getAggregateQueryMode(query);
const hasResults = Boolean(next.result?.length);

if (next.fetchStatus !== FetchStatus.PARTIAL) {
return;
}

let nextColumns: string[] = prev.current.recentlyUpdatedToColumns;

if (hasResults) {
// check if state needs to contain column transformation due to a different columns in the resultset
// check if state needs to contain column transformation due to a different columns in the result set
jughosta marked this conversation as resolved.
Show resolved Hide resolved
jughosta marked this conversation as resolved.
Show resolved Hide resolved
const firstRow = next.result![0];
const firstRowColumns = Object.keys(firstRow.raw).slice(0, MAX_NUM_OF_COLUMNS);
if (!queryHasTransformationalCommands) {
nextColumns = [];
initialFetch.current = false;
const firstRowColumns = Object.keys(firstRow.raw);

if (hasTransformationalCommand(query.esql)) {
nextColumns = firstRowColumns.slice(0, MAX_NUM_OF_COLUMNS);
} else {
nextColumns = firstRowColumns;
if (initialFetch.current && !prev.current.columns.length) {
prev.current.columns = firstRowColumns;
}
nextColumns = [];
}
}
const addColumnsToState = !isEqual(nextColumns, prev.current.columns);
const queryChanged = query[language] !== prev.current.query;

if (initialFetch.current) {
initialFetch.current = false;
prev.current.query = query.esql;
prev.current.recentlyUpdatedToColumns = nextColumns;
}

const indexPatternChanged =
getIndexPatternFromESQLQuery(query.esql) !==
getIndexPatternFromESQLQuery(prev.current.query);

const addColumnsToState =
indexPatternChanged || !isEqual(nextColumns, prev.current.recentlyUpdatedToColumns);

const changeViewMode = viewMode !== getValidViewMode({ viewMode, isEsqlMode: true });
if (!queryChanged || (!addColumnsToState && !changeViewMode)) {

if (!indexPatternChanged && !addColumnsToState && !changeViewMode) {
kertal marked this conversation as resolved.
Show resolved Hide resolved
sendComplete();
return;
}

if (queryChanged) {
prev.current.query = query[language];
prev.current.columns = nextColumns;
}
prev.current.query = query.esql;
prev.current.recentlyUpdatedToColumns = nextColumns;

// just change URL state if necessary
if (addColumnsToState || changeViewMode) {
const nextState = {
Expand Down
Loading