[ES|QL] Ignore drop commands for date histogram in discover (#171769)

## Summary

Fixes #169907 

This PR cleans the ES|QL statement from DROP commands before sending it
over for the date histogram chart in Lens.


### Checklist

Delete any items that are not applicable to this PR.

- [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

---------

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
This commit is contained in:
Marco Liberati 2023-11-27 16:02:27 +01:00 committed by GitHub
parent 31a8b7bca6
commit d900f8473b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 290 additions and 5 deletions

View file

@ -56,6 +56,7 @@ export {
getIndexPatternFromSQLQuery,
getIndexPatternFromESQLQuery,
getLanguageDisplayName,
cleanupESQLQueryForLensSuggestions,
} from './src/es_query';
export {

View file

@ -12,6 +12,7 @@ import {
getAggregateQueryMode,
getIndexPatternFromSQLQuery,
getIndexPatternFromESQLQuery,
cleanupESQLQueryForLensSuggestions,
} from './es_aggregate_query';
describe('sql query helpers', () => {
@ -115,4 +116,18 @@ describe('sql query helpers', () => {
expect(idxPattern9).toBe('foo-1, foo-2');
});
});
describe('cleanupESQLQueryForLensSuggestions', () => {
it('should not remove anything if a drop command is not present', () => {
expect(cleanupESQLQueryForLensSuggestions('from a | eval b = 1')).toBe('from a | eval b = 1');
});
it('should remove multiple drop statement if present', () => {
expect(
cleanupESQLQueryForLensSuggestions(
'from a | drop @timestamp | drop a | drop b | keep c | drop d'
)
).toBe('from a | keep c ');
});
});
});

View file

@ -66,3 +66,8 @@ export function getIndexPatternFromESQLQuery(esql?: string): string {
}
return '';
}
export function cleanupESQLQueryForLensSuggestions(esql?: string): string {
const pipes = (esql || '').split('|');
return pipes.filter((statement) => !/DROP\s/i.test(statement)).join('|');
}

View file

@ -20,6 +20,7 @@ export {
getIndexPatternFromSQLQuery,
getLanguageDisplayName,
getIndexPatternFromESQLQuery,
cleanupESQLQueryForLensSuggestions,
} from './es_aggregate_query';
export { fromCombinedFilter } from './from_combined_filter';
export type {

View file

@ -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,
});

View file

@ -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,15 @@ export const UnifiedDataTable = ({
[dataView, onFieldEdited, services.dataViewFieldEditor]
);
const shouldShowTimeField = useMemo(
() =>
hasSourceTimeFieldValue(displayedColumns, dataView, columnTypes, showTimeCol, isPlainRecord),
[dataView, displayedColumns, isPlainRecord, showTimeCol, columnTypes]
);
const visibleColumns = useMemo(
() => getVisibleColumns(displayedColumns, dataView, showTimeCol),
[dataView, displayedColumns, showTimeCol]
() => getVisibleColumns(displayedColumns, dataView, shouldShowTimeField),
[dataView, displayedColumns, shouldShowTimeField]
);
const getCellValue = useCallback<UseDataGridColumnsCellActionsProps['getCellValue']>(

View file

@ -7,8 +7,14 @@
*/
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';
@ -110,6 +116,133 @@ describe('Data table columns', function () {
});
});
describe('hasSourceTimeFieldValue', () => {
function buildColumnTypes(dataView: DataView) {
const columnTypes: Record<string, string> = {};
for (const field of dataView.fields) {
columnTypes[field.name] = '';
}
return columnTypes;
}
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,
buildColumnTypes(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,
buildColumnTypes(dataViewWithTimefieldMock),
showTimeCol,
true
)
).toBe(showTimeCol);
}
});
it('should forward showTimeCol if _source column is passed', () => {
for (const showTimeCol of [true, false]) {
expect(
hasSourceTimeFieldValue(
['_source'],
dataViewWithTimefieldMock,
buildColumnTypes(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,
buildColumnTypes(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,
buildColumnTypes(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,
buildColumnTypes(dataViewWithoutTimefieldMock),
showTimeCol,
true
)
).toBe(showTimeCol);
}
});
it('should forward showTimeCol if _source column is passed', () => {
for (const showTimeCol of [true, false]) {
expect(
hasSourceTimeFieldValue(
['_source'],
dataViewWithoutTimefieldMock,
buildColumnTypes(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,
buildColumnTypes(dataViewWithoutTimefieldMock),
showTimeCol,
true
)
).toBe(showTimeCol);
}
});
});
});
describe('column tokens', () => {
it('returns eui grid columns with tokens', async () => {
const actual = getEuiGridColumns({

View file

@ -267,6 +267,20 @@ export function getEuiGridColumns({
);
}
export function hasSourceTimeFieldValue(
columns: string[],
dataView: DataView,
columnTypes: DataTableColumnTypes | undefined,
showTimeCol: boolean,
isPlainRecord: boolean
) {
const timeFieldName = dataView.timeFieldName;
if (!isPlainRecord || !columns.includes('_source') || !timeFieldName || !columnTypes) {
return showTimeCol;
}
return timeFieldName in columnTypes;
}
export function getVisibleColumns(columns: string[], dataView: DataView, showTimeCol: boolean) {
const timeFieldName = dataView.timeFieldName;

View file

@ -143,6 +143,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;

View file

@ -11,6 +11,7 @@ import {
AggregateQuery,
isOfAggregateQueryType,
getAggregateQueryMode,
cleanupESQLQueryForLensSuggestions,
Query,
TimeRange,
} from '@kbn/es-query';
@ -85,7 +86,8 @@ export const useLensSuggestions = ({
const interval = computeInterval(timeRange, data);
const language = getAggregateQueryMode(query);
const esqlQuery = `${query[language]} | EVAL timestamp=DATE_TRUNC(${interval}, ${dataView.timeFieldName}) | stats rows = count(*) by timestamp | rename timestamp as \`${dataView.timeFieldName} every ${interval}\``;
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(),
fieldName: '',