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

[ES|QL] Distinguish among empty and available fields #174585

Merged
merged 35 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
13e441d
[ES|QL] Distinguish among empty and available fields
stratoula Jan 10, 2024
004da7e
Merge with main and resolve conflicts
stratoula Jan 15, 2024
37c9cfd
Update with latest ES response
stratoula Jan 15, 2024
faf7bd9
Merge with main and resolve conflict
stratoula Jan 22, 2024
d6e3d34
Adds tests
stratoula Jan 22, 2024
cb9e509
Fix tests
stratoula Jan 22, 2024
096aeb3
Update FT
stratoula Jan 22, 2024
024ab4d
Fix FTs
stratoula Jan 22, 2024
f472b5a
More fixes
stratoula Jan 22, 2024
8c9ce1f
Merge branch 'main' into available-empty-esql
stratoula Jan 23, 2024
ed0aafe
Merge branch 'main' into available-empty-esql
stratoula Jan 29, 2024
ecce242
Merge branch 'main' into available-empty-esql
stratoula Jan 31, 2024
6e3ec96
Implement for async query
stratoula Jan 31, 2024
7e69d0e
Fix some FTs
stratoula Jan 31, 2024
bfb8486
Fix tests
stratoula Jan 31, 2024
928462e
FTs
stratoula Jan 31, 2024
f20f403
Fix FT flakiness
stratoula Jan 31, 2024
044d306
Revert changes
stratoula Jan 31, 2024
b8bfc4c
Attempts to fix the FT
stratoula Jan 31, 2024
5d935db
Increase the limit to get more accurate results
stratoula Feb 1, 2024
04210d6
Apply PR comment
stratoula Feb 1, 2024
c3b0750
Fix type check
stratoula Feb 1, 2024
b1c01f0
Merge branch 'main' into available-empty-esql
stratoula Feb 1, 2024
9704daa
Update packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts
stratoula Feb 1, 2024
a0dce6b
Some comments
stratoula Feb 1, 2024
447e439
More PR comments
stratoula Feb 1, 2024
a757bd8
Simplification
stratoula Feb 1, 2024
d9ce4b1
Merge branch 'main' into available-empty-esql
stratoula Feb 2, 2024
d57069d
Fix FT
stratoula Feb 2, 2024
2ace8c7
Add an empty fields FT
stratoula Feb 2, 2024
28c991a
PR comments
stratoula Feb 2, 2024
fe962ea
Apply performance optimization comment
stratoula Feb 2, 2024
be8967a
Revert
stratoula Feb 2, 2024
3c15ccc
Final chanegs and test addition to be sure that sorting is kept
stratoula Feb 2, 2024
a8d77df
Fix FT failures
stratoula Feb 2, 2024
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
5 changes: 5 additions & 0 deletions packages/kbn-es-types/src/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,10 @@ export type ESQLRow = unknown[];

export interface ESQLSearchReponse {
columns: ESQLColumn[];
// In case of ?drop_null_columns in the query, then
// all_columns will have available and empty fields
// while columns only the available ones (non nulls)
all_columns?: ESQLColumn[];
values: ESQLRow[];
}

Expand All @@ -674,4 +678,5 @@ export interface ESQLSearchParams {
query: string;
filter?: unknown;
locale?: string;
dropNullColumns?: boolean;
}
1 change: 1 addition & 0 deletions packages/kbn-field-utils/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface FieldBase {
timeSeriesMetric?: DataViewField['timeSeriesMetric'];
esTypes?: DataViewField['esTypes'];
scripted?: DataViewField['scripted'];
isNull?: DataViewField['isNull'];
conflictDescriptions?: Record<string, string[]>;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export function SourceDocument({
{pairs.map(([fieldDisplayName, value, fieldName]) => {
// temporary solution for text based mode. As there are a lot of unsupported fields we want to
// hide the empty one from the Document view
if (isPlainRecord && fieldName && row.flattened[fieldName] === null) return null;
if (isPlainRecord && fieldName && !row.flattened[fieldName]) return null;
return (
<Fragment key={fieldDisplayName}>
<EuiDescriptionListTitle className="unifiedDataTable__descriptionListTitle">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,45 @@ describe('UnifiedFieldList useGroupedFields()', () => {
expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true);
});

it('should work correctly for text-based queries (no data view) with isNull fields', async () => {
const allFieldsEmpty = [...new Array(2)].flatMap((_, index) =>
allFields.map((field) => {
return new DataViewField({
...field.toSpec(),
name: `${field.name}${index || ''}`,
isNull: true,
});
})
);
const { result } = renderHook(useGroupedFields, {
initialProps: {
dataViewId: null,
allFields: allFieldsEmpty,
services: mockedServices,
},
});

const fieldListGroupedProps = result.current.fieldListGroupedProps;
const fieldGroups = fieldListGroupedProps.fieldGroups;

expect(
Object.keys(fieldGroups!).map(
(key) => `${key}-${fieldGroups![key as FieldsGroupNames]?.fields.length}`
)
).toStrictEqual([
'SpecialFields-0',
'SelectedFields-0',
'PopularFields-0',
'AvailableFields-0',
'UnmappedFields-0',
'EmptyFields-56',
'MetaFields-0',
]);

expect(fieldListGroupedProps.fieldsExistenceStatus).toBe(ExistenceFetchStatus.succeeded);
expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true);
});

it('should work correctly when details are overwritten', async () => {
const onOverrideFieldGroupDetails: GroupedFieldsParams<DataViewField>['onOverrideFieldGroupDetails'] =
jest.fn((groupName) => {
Expand Down
13 changes: 11 additions & 2 deletions packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
if (field.type === 'nested') {
return 'availableFields';
}

if (field?.isNull) {
return 'emptyFields';
}
if (dataView?.getFieldByName && !dataView.getFieldByName(field.name)) {
return 'unmappedFields';
}
Expand Down Expand Up @@ -303,8 +307,13 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
},
};

// do not show empty field accordion if there is no existence information
if (fieldsExistenceInfoUnavailable) {
// dataViewId is null for text based queries
// the fieldsExistenceInfoUnavailable check should happen only for dataview based
// for textbased queries, rely on the empty fields length
if (
(fieldsExistenceInfoUnavailable && dataViewId) ||
(!dataViewId && !groupedFields.emptyFields.length)
) {
stratoula marked this conversation as resolved.
Show resolved Hide resolved
delete fieldGroupDefinitions.EmptyFields;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
interface Args extends QueryState {
timeFieldName?: string;
inputQuery?: Query;
dropNullColumns?: boolean;
}

/**
Expand All @@ -34,6 +35,7 @@ export function textBasedQueryStateToExpressionAst({
inputQuery,
time,
timeFieldName,
dropNullColumns,
}: Args) {
const kibana = buildExpressionFunction<ExpressionFunctionKibana>('kibana', {});
let q;
Expand All @@ -51,7 +53,7 @@ export function textBasedQueryStateToExpressionAst({
const mode = getAggregateQueryMode(query);
for (const esMode of ['sql', 'esql']) {
if (mode === esMode && esMode in query) {
const essql = aggregateQueryToAst(query, timeFieldName);
const essql = aggregateQueryToAst(query, timeFieldName, dropNullColumns);
stratoula marked this conversation as resolved.
Show resolved Hide resolved

if (essql) {
ast.chain.push(essql);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ interface Args extends QueryState {
dataView?: DataView;
inputQuery?: Query;
timeFieldName?: string;
dropNullColumns?: boolean;
}

/**
Expand All @@ -28,6 +29,7 @@ export async function textBasedQueryStateToAstWithValidation({
inputQuery,
time,
dataView,
dropNullColumns,
}: Args) {
let ast;
if (query && isOfAggregateQueryType(query)) {
Expand All @@ -37,6 +39,7 @@ export async function textBasedQueryStateToAstWithValidation({
inputQuery,
time,
timeFieldName: dataView?.timeFieldName,
dropNullColumns,
});
}
return ast;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import { EsqlExpressionFunctionDefinition } from './esql';

export const aggregateQueryToAst = (
query: AggregateQuery,
timeField?: string
timeField?: string,
dropNullColumns?: boolean
): undefined | ExpressionAstFunction => {
if ('sql' in query) {
return buildExpressionFunction<EssqlExpressionFunctionDefinition>('essql', {
Expand All @@ -26,6 +27,7 @@ export const aggregateQueryToAst = (
query: query.esql,
timeField,
locale: i18n.getLocale(),
dropNullColumns,
}).toAst();
}
};
34 changes: 31 additions & 3 deletions src/plugins/data/common/search/expressions/esql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ interface Arguments {
// timezone?: string;
timeField?: string;
locale?: string;
dropNullColumns?: boolean;
}

export type EsqlExpressionFunctionDefinition = ExpressionFunctionDefinition<
Expand Down Expand Up @@ -125,10 +126,17 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
defaultMessage: 'The locale to use.',
}),
},
dropNullColumns: {
aliases: ['dropNullColumns'],
types: ['boolean'],
help: i18n.translate('data.search.essql.dropNull.help', {
defaultMessage: 'If true adds all_columns in a separate property',
}),
},
},
fn(
input,
{ query, /* timezone, */ timeField, locale },
{ query, /* timezone, */ timeField, locale, dropNullColumns },
{ abortSignal, inspectorAdapters, getKibanaRequest }
) {
return defer(() =>
Expand All @@ -149,6 +157,7 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
query,
// time_zone: timezone,
locale,
dropNullColumns,
};
if (input) {
const esQueryConfigs = getEsQueryConfig(
Expand Down Expand Up @@ -243,15 +252,34 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
name,
meta: { type: normalizeType(type) },
})) ?? [];
const columnNames = columns.map(({ name }) => name);

// all_columns in the response means that there is a separation between
// columns with data and empty columns
// columns contain only columns with data while all_columns everything
const difference =
body.all_columns?.filter((col1) => {
return !body.columns.some((col2) => {
return col1.name === col2.name;
});
}) ?? [];
const emptyColumns =
difference?.map(({ name, type }) => ({
id: name,
name,
meta: { type: normalizeType(type) },
isNull: true,
})) ?? [];

const allColumns = [...columns, ...emptyColumns];
stratoula marked this conversation as resolved.
Show resolved Hide resolved
const columnNames = allColumns.map(({ name }) => name);
const rows = body.values.map((row) => zipObject(columnNames, row));

return {
type: 'datatable',
meta: {
type: 'es_ql',
},
columns,
columns: allColumns,
rows,
warning,
} as Datatable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@ import { IKibanaSearchRequest, IKibanaSearchResponse, pollSearch } from '../../.
import { toAsyncKibanaSearchResponse } from './response_utils';
import { SearchConfigSchema } from '../../../../config';

interface ESQLQueryRequest extends SqlQueryRequest {
body?: SqlQueryRequest['body'] & { dropNullColumns?: boolean };
stratoula marked this conversation as resolved.
Show resolved Hide resolved
}

export const esqlAsyncSearchStrategyProvider = (
searchConfig: SearchConfigSchema,
logger: Logger,
useInternalUser: boolean = false
): ISearchStrategy<
IKibanaSearchRequest<SqlQueryRequest['body']>,
IKibanaSearchRequest<ESQLQueryRequest['body']>,
IKibanaSearchResponse<SqlGetAsyncResponse>
> => {
function cancelAsyncSearch(id: string, esClient: IScopedClusterClient) {
Expand All @@ -46,13 +50,15 @@ export const esqlAsyncSearchStrategyProvider = (
}

function asyncSearch(
{ id, ...request }: IKibanaSearchRequest<SqlQueryRequest['body']>,
{ id, ...request }: IKibanaSearchRequest<ESQLQueryRequest['body']>,
options: IAsyncSearchOptions,
{ esClient, uiSettingsClient }: SearchStrategyDependencies
) {
const client = useInternalUser ? esClient.asInternalUser : esClient.asCurrentUser;

const search = async () => {
const { dropNullColumns, ...requestParams } = request.params ?? {};
const queryParams = dropNullColumns ? '?drop_null_columns' : '';
const params = id
? {
...getCommonDefaultAsyncGetParams(searchConfig, options),
Expand All @@ -63,15 +69,19 @@ export const esqlAsyncSearchStrategyProvider = (
}
: {
...(await getCommonDefaultAsyncSubmitParams(searchConfig, options)),
...request.params,
...requestParams,
stratoula marked this conversation as resolved.
Show resolved Hide resolved
};
const { body, headers, meta } = id
? await client.transport.request<SqlGetAsyncResponse>(
{ method: 'GET', path: `/_query/async/${id}`, querystring: { ...params } },
{
method: 'GET',
path: `/_query/async/${id}`,
querystring: { ...params },
},
{ ...options.transport, signal: options.abortSignal, meta: true }
)
: await client.transport.request<SqlGetAsyncResponse>(
{ method: 'POST', path: `/_query/async`, body: params },
{ method: 'POST', path: `/_query/async${queryParams}`, body: params },
{ ...options.transport, signal: options.abortSignal, meta: true }
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ export const esqlSearchStrategyProvider = (

const search = async () => {
try {
const { terminateAfter, ...requestParams } = request.params ?? {};
const { terminateAfter, dropNullColumns, ...requestParams } = request.params ?? {};
const queryParams = dropNullColumns ? '?drop_null_columns' : '';
const { headers, body, meta } = await esClient.asCurrentUser.transport.request(
{
method: 'POST',
path: '/_query',
path: `/_query${queryParams}`,
stratoula marked this conversation as resolved.
Show resolved Hide resolved
body: {
...requestParams,
},
Expand Down
8 changes: 8 additions & 0 deletions src/plugins/data_views/common/fields/data_view_field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,14 @@ export class DataViewField implements DataViewFieldBase {
return this.aggregatable && !notVisualizableFieldTypes.includes(this.spec.type);
}

/**
* Returns true if field is Empty
*/

public get isNull() {
return Boolean(this.spec.isNull);
}

/**
* Returns true if field is subtype nested
*/
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/data_views/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,10 @@ export type FieldSpec = DataViewFieldBase & {
* True if field is aggregatable
*/
aggregatable: boolean;
/**
* True if field is empty
*/
isNull?: boolean;
/**
* True if can be read from doc values
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export function getTextBasedQueryFieldList(
type: column.meta?.type ?? 'unknown',
searchable: false,
aggregatable: false,
isNull: Boolean(column?.isNull),
})
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe('sidebar reducer', function () {
meta: {
type: 'number',
},
isNull: true,
},
{
id: '2',
Expand All @@ -127,12 +128,14 @@ describe('sidebar reducer', function () {
name: 'text1',
type: 'number',
aggregatable: false,
empty: true,
searchable: false,
}),
new DataViewField({
name: 'text2',
type: 'keyword',
aggregatable: false,
empty: false,
searchable: false,
}),
],
Expand Down
Loading