Skip to content

Commit

Permalink
[ES|QL][Lens] Remove allColumns from state (elastic#174500)
Browse files Browse the repository at this point in the history
## Summary

The basic feature that this PR offers is the removal of allColumns from
the state. It now retrieves them from the cache when they are needed.

This is going to make the SO much lighter and improve a bit the
performance. This must be bwc.
I also think that it makes the state much easier to understand and the
code so it is a very nice technical improvement.

### Checklist

- [ ] [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
  • Loading branch information
stratoula authored and delanni committed Jan 11, 2024
1 parent 4ba6be2 commit 526d816
Show file tree
Hide file tree
Showing 26 changed files with 345 additions and 552 deletions.
32 changes: 0 additions & 32 deletions src/plugins/unified_histogram/public/__mocks__/suggestions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,6 @@ export const currentSuggestionMock = {
},
},
],
allColumns: [
{
columnId: '81e332d6-ee37-42a8-a646-cea4fc75d2d3',
fieldName: 'Dest',
meta: {
type: 'string',
},
},
{
columnId: '5b9b8b76-0836-4a12-b9c0-980c9900502f',
fieldName: 'AvgTicketPrice',
meta: {
type: 'number',
},
},
],
timeField: 'timestamp',
},
},
Expand Down Expand Up @@ -195,22 +179,6 @@ export const allSuggestionsMock = [
},
},
],
allColumns: [
{
columnId: '923f0681-3fe1-4987-aa27-d9c91fb95fa6',
fieldName: 'Dest',
meta: {
type: 'string',
},
},
{
columnId: 'b5f41c04-4bca-4abe-ae5c-b1d4d6fb00e0',
fieldName: 'AvgTicketPrice',
meta: {
type: 'number',
},
},
],
timeField: 'timestamp',
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,22 +643,6 @@ describe('getLensAttributes', () => {
},
"layers": Object {
"46aa21fa-b747-4543-bf90-0b40007c546d": Object {
"allColumns": Array [
Object {
"columnId": "81e332d6-ee37-42a8-a646-cea4fc75d2d3",
"fieldName": "Dest",
"meta": Object {
"type": "string",
},
},
Object {
"columnId": "5b9b8b76-0836-4a12-b9c0-980c9900502f",
"fieldName": "AvgTicketPrice",
"meta": Object {
"type": "number",
},
},
],
"columns": Array [
Object {
"columnId": "81e332d6-ee37-42a8-a646-cea4fc75d2d3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ import {
mockAllSuggestions,
} from '../../../mocks';
import { suggestionsApi } from '../../../lens_suggestions_api';
import { fetchDataFromAggregateQuery } from '../../../datasources/text_based/fetch_data_from_aggregate_query';
import { fetchFieldsFromESQL } from '../../../datasources/text_based/fetch_fields_from_esql';
import { getSuggestions } from './helpers';

const mockSuggestionApi = suggestionsApi as jest.Mock;
const mockFetchData = fetchDataFromAggregateQuery as jest.Mock;
const mockFetchData = fetchFieldsFromESQL as jest.Mock;

jest.mock('../../../lens_suggestions_api', () => ({
suggestionsApi: jest.fn(() => mockAllSuggestions),
}));

jest.mock('../../../datasources/text_based/fetch_data_from_aggregate_query', () => ({
fetchDataFromAggregateQuery: jest.fn(() => {
jest.mock('../../../datasources/text_based/fetch_fields_from_esql', () => ({
fetchFieldsFromESQL: jest.fn(() => {
return {
columns: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,10 @@ import type { Suggestion } from '../../../types';
import type { TypedLensByValueInput } from '../../../embeddable/embeddable_component';
import type { LensPluginStartDependencies } from '../../../plugin';
import type { DatasourceMap, VisualizationMap } from '../../../types';
import { fetchDataFromAggregateQuery } from '../../../datasources/text_based/fetch_data_from_aggregate_query';
import { fetchFieldsFromESQL } from '../../../datasources/text_based/fetch_fields_from_esql';
import { suggestionsApi } from '../../../lens_suggestions_api';

export const getQueryColumns = async (
query: AggregateQuery,
dataView: DataView,
deps: LensPluginStartDependencies
) => {
export const getQueryColumns = async (query: AggregateQuery, deps: LensPluginStartDependencies) => {
// Fetching only columns for ES|QL for performance reasons with limit 0
// Important note: ES doesnt return the warnings for 0 limit,
// I am skipping them in favor of performance now
Expand All @@ -28,12 +24,7 @@ export const getQueryColumns = async (
if ('esql' in performantQuery && performantQuery.esql) {
performantQuery.esql = `${performantQuery.esql} | limit 0`;
}
const table = await fetchDataFromAggregateQuery(
performantQuery,
dataView,
deps.data,
deps.expressions
);
const table = await fetchFieldsFromESQL(performantQuery, deps.expressions);
return table?.columns;
};

Expand Down Expand Up @@ -65,7 +56,7 @@ export const getSuggestions = async (
if (dataView.fields.getByName('@timestamp')?.type === 'date' && !dataViewSpec) {
dataView.timeFieldName = '@timestamp';
}
const columns = await getQueryColumns(query, dataView, deps);
const columns = await getQueryColumns(query, deps);
const context = {
dataViewSpec: dataView?.toSpec(),
fieldName: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type { AggregateQuery, Query } from '@kbn/es-query';
import { TextBasedLangEditor } from '@kbn/text-based-languages/public';
import { DefaultInspectorAdapters } from '@kbn/expressions-plugin/common';
import { buildExpression } from '../../../editor_frame_service/editor_frame/expression_helpers';
import { MAX_NUM_OF_COLUMNS } from '../../../datasources/text_based/utils';
import {
useLensSelector,
selectFramePublicAPI,
Expand Down Expand Up @@ -76,6 +77,7 @@ export function LensEditConfigurationFlyout({
const [errors, setErrors] = useState<Error[] | undefined>();
const [isInlineFlyoutVisible, setIsInlineFlyoutVisible] = useState(true);
const [isLayerAccordionOpen, setIsLayerAccordionOpen] = useState(true);
const [suggestsLimitedColumns, setSuggestsLimitedColumns] = useState(false);
const [isSuggestionsAccordionOpen, setIsSuggestionsAccordionOpen] = useState(false);
const datasourceState = attributes.state.datasourceStates[datasourceId];
const activeDatasource = datasourceMap[datasourceId];
Expand All @@ -87,7 +89,6 @@ export function LensEditConfigurationFlyout({
visualizationMap[visualization.activeId ?? attributes.visualizationType];

const framePublicAPI = useLensSelector((state) => selectFramePublicAPI(state, datasourceMap));
const suggestsLimitedColumns = activeDatasource?.suggestsLimitedColumns?.(datasourceState);

const layers = useMemo(
() => activeDatasource.getLayers(datasourceState),
Expand All @@ -101,6 +102,11 @@ export function LensEditConfigurationFlyout({
const adaptersTables = previousAdapters.current?.tables?.tables as Record<string, Datatable>;
const [table] = Object.values(adaptersTables || {});
if (table) {
// there are cases where a query can return a big amount of columns
// at this case we don't suggest all columns in a table but the first
// MAX_NUM_OF_COLUMNS
const columns = Object.keys(table.rows?.[0]) ?? [];
setSuggestsLimitedColumns(columns.length >= MAX_NUM_OF_COLUMNS);
layers.forEach((layer) => {
activeData[layer] = table;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ import { EuiHighlight, EuiToken } from '@elastic/eui';
import { type TextBasedDataPanelProps, TextBasedDataPanel } from './datapanel';

import { coreMock } from '@kbn/core/public/mocks';
import type { TextBasedPrivateState } from './types';
import type { TextBasedPrivateState } from '../types';
import { mountWithIntl } from '@kbn/test-jest-helpers';

import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks';
import { createIndexPatternServiceMock } from '../../mocks/data_views_service_mock';
import { createMockFramePublicAPI } from '../../mocks';
import { DataViewsState } from '../../state_management';
import { addColumnsToCache } from './fieldlist_cache';
import { createIndexPatternServiceMock } from '../../../mocks/data_views_service_mock';
import { createMockFramePublicAPI } from '../../../mocks';
import { DataViewsState } from '../../../state_management';
import { addColumnsToCache } from '../fieldlist_cache';

const fieldsFromQuery = [
{
Expand Down Expand Up @@ -106,7 +106,6 @@ const initialState: TextBasedPrivateState = {
first: {
index: '1',
columns: [],
allColumns: [],
query: { esql: 'SELECT * FROM foo' },
},
},
Expand All @@ -117,7 +116,7 @@ const initialState: TextBasedPrivateState = {
],
};

addColumnsToCache('SELECT * FROM my-fake-index-pattern', fieldsFromQuery);
addColumnsToCache({ esql: 'SELECT * FROM my-fake-index-pattern' }, fieldsFromQuery);

function getFrameAPIMock({
indexPatterns,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { isEqual } from 'lodash';
import { DataPublicPluginStart } from '@kbn/data-plugin/public';
import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';

import { isOfAggregateQueryType, getAggregateQueryMode } from '@kbn/es-query';
import { isOfAggregateQueryType } from '@kbn/es-query';
import { DatatableColumn, ExpressionsStart } from '@kbn/expressions-plugin/public';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import {
Expand All @@ -24,11 +24,11 @@ import {
GetCustomFieldType,
useGroupedFields,
} from '@kbn/unified-field-list';
import type { DatasourceDataPanelProps } from '../../types';
import type { TextBasedPrivateState } from './types';
import { getStateFromAggregateQuery } from './utils';
import { FieldItem } from '../common/field_item';
import { getColumnsFromCache } from './fieldlist_cache';
import type { DatasourceDataPanelProps } from '../../../types';
import type { TextBasedPrivateState } from '../types';
import { getStateFromAggregateQuery } from '../utils';
import { FieldItem } from '../../common/field_item';
import { getColumnsFromCache } from '../fieldlist_cache';

const getCustomFieldType: GetCustomFieldType<DatatableColumn> = (field) => field?.meta.type;

Expand Down Expand Up @@ -74,8 +74,7 @@ export function TextBasedDataPanel({
}
fetchData();
}, [data, dataViews, expressions, prevQuery, query, setState, state, frame.dataViews]);
const language = isOfAggregateQueryType(query) ? getAggregateQueryMode(query) : null;
const fieldList = language ? getColumnsFromCache(query[language]) : [];
const fieldList = isOfAggregateQueryType(query) ? getColumnsFromCache(query) : [];

const onSelectedFieldFilter = useCallback(
(field: DatatableColumn): boolean => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { i18n } from '@kbn/i18n';
import { EuiFormRow } from '@elastic/eui';
import { euiThemeVars } from '@kbn/ui-theme';
import type { ExpressionsStart } from '@kbn/expressions-plugin/public';
import type { DatasourceDimensionEditorProps, DataType } from '../../../types';
import { FieldSelect } from './field_select';
import type { TextBasedPrivateState } from '../types';
import { retrieveLayerColumnsFromCache, getColumnsFromCache } from '../fieldlist_cache';

export type TextBasedDimensionEditorProps =
DatasourceDimensionEditorProps<TextBasedPrivateState> & {
expressions: ExpressionsStart;
};

export function TextBasedDimensionEditor(props: TextBasedDimensionEditorProps) {
const query = props.state.layers[props.layerId]?.query;

const allColumns = retrieveLayerColumnsFromCache(
props.state.layers[props.layerId]?.columns ?? [],
query
);
const allFields = query ? getColumnsFromCache(query) : [];
const hasNumberTypeColumns = allColumns?.some((c) => c?.meta?.type === 'number');
const fields = allFields.map((col) => {
return {
id: col.id,
name: col.name,
meta: col?.meta ?? { type: 'number' },
compatible:
props.isMetricDimension && hasNumberTypeColumns
? props.filterOperations({
dataType: col?.meta?.type as DataType,
isBucketed: Boolean(col?.meta?.type !== 'number'),
scale: 'ordinal',
})
: true,
};
});
const selectedField = allColumns?.find((column) => column.columnId === props.columnId);

return (
<>
<EuiFormRow
data-test-subj="text-based-languages-field-selection-row"
label={i18n.translate('xpack.lens.textBasedLanguages.chooseField', {
defaultMessage: 'Field',
})}
fullWidth
className="lnsIndexPatternDimensionEditor--padded"
>
<FieldSelect
existingFields={fields ?? []}
selectedField={selectedField}
onChoose={(choice) => {
const meta = fields?.find((f) => f.name === choice.field)?.meta;
const newColumn = {
columnId: props.columnId,
fieldName: choice.field,
meta,
};
return props.setState(
!selectedField
? {
...props.state,
layers: {
...props.state.layers,
[props.layerId]: {
...props.state.layers[props.layerId],
columns: [...props.state.layers[props.layerId].columns, newColumn],
},
},
}
: {
...props.state,
layers: {
...props.state.layers,
[props.layerId]: {
...props.state.layers[props.layerId],
columns: props.state.layers[props.layerId].columns.map((col) =>
col.columnId !== props.columnId
? col
: { ...col, fieldName: choice.field, meta }
),
},
},
}
);
}}
/>
</EuiFormRow>
{props.dataSectionExtra && (
<div
style={{
paddingLeft: euiThemeVars.euiSize,
paddingRight: euiThemeVars.euiSize,
}}
>
{props.dataSectionExtra}
</div>
)}
</>
);
}
Loading

0 comments on commit 526d816

Please sign in to comment.