Skip to content

Commit

Permalink
[ES|QL] Distinguish among empty and available fields (elastic#174585)
Browse files Browse the repository at this point in the history
  • Loading branch information
stratoula authored and fkanout committed Mar 4, 2024
1 parent 9846e7e commit ae04101
Show file tree
Hide file tree
Showing 17 changed files with 165 additions and 16 deletions.
4 changes: 4 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 Down
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
12 changes: 10 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,12 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
},
};

// do not show empty field accordion if there is no existence information
if (fieldsExistenceInfoUnavailable) {
// the fieldsExistenceInfoUnavailable check should happen only for dataview based
const dataViewFieldsExistenceUnavailable = dataViewId && fieldsExistenceInfoUnavailable;
// for textbased queries, rely on the empty fields length
const textBasedFieldsExistenceUnavailable = !dataViewId && !groupedFields.emptyFields.length;

if (dataViewFieldsExistenceUnavailable || textBasedFieldsExistenceUnavailable) {
delete fieldGroupDefinitions.EmptyFields;
}

Expand Down
29 changes: 27 additions & 2 deletions src/plugins/data/common/search/expressions/esql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { castEsToKbnFieldTypeName, ES_FIELD_TYPES, KBN_FIELD_TYPES } from '@kbn/
import { i18n } from '@kbn/i18n';
import type {
Datatable,
DatatableColumn,
DatatableColumnType,
ExpressionFunctionDefinition,
} from '@kbn/expressions-plugin/common';
Expand Down Expand Up @@ -243,15 +244,39 @@ 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 hasEmptyColumns =
body.all_columns && body.all_columns?.length > body.columns.length;

let emptyColumns: DatatableColumn[] = [];

if (hasEmptyColumns) {
const difference =
body.all_columns?.filter((col1) => {
return !body.columns.some((col2) => {
return col1.name === col2.name;
});
}) ?? [];
emptyColumns =
difference?.map(({ name, type }) => ({
id: name,
name,
meta: { type: normalizeType(type) },
isNull: true,
})) ?? [];
}
const allColumns = [...columns, ...emptyColumns];
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 @@ -67,11 +67,20 @@ export const esqlAsyncSearchStrategyProvider = (
};
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`,
body: params,
querystring: 'drop_null_columns',
},
{ ...options.transport, signal: options.abortSignal, meta: true }
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ export const esqlSearchStrategyProvider = (
const { headers, body, meta } = await esClient.asCurrentUser.transport.request(
{
method: 'POST',
path: '/_query',
path: `/_query`,
querystring: 'drop_null_columns',
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,
isNull: true,
searchable: false,
}),
new DataViewField({
name: 'text2',
type: 'keyword',
aggregatable: false,
isNull: false,
searchable: false,
}),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,13 @@ export function fetchTextBased(
const rows = table?.rows ?? [];
textBasedQueryColumns = table?.columns ?? undefined;
textBasedHeaderWarning = table.warning ?? undefined;
finalData = rows.map(
(row: Record<string, string>, idx: number) =>
({
id: String(idx),
raw: row,
flattened: row,
} as unknown as DataTableRecord)
);
finalData = rows.map((row: Record<string, string>, idx: number) => {
return {
id: String(idx),
raw: row,
flattened: row,
} as unknown as DataTableRecord;
});
}
});
return lastValueFrom(execution).then(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export interface DatatableColumn {
id: string;
name: string;
meta: DatatableColumnMeta;
isNull?: boolean;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions test/api_integration/apis/search/bsearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ export default function ({ getService }: FtrProviderContext) {
expect(jsonBody[0].result.requestParams).to.eql({
method: 'POST',
path: '/_query',
querystring: 'drop_null_columns',
});
});

Expand Down Expand Up @@ -456,6 +457,7 @@ export default function ({ getService }: FtrProviderContext) {
expect(jsonBody[0].error.attributes.requestParams).to.eql({
method: 'POST',
path: '/_query',
querystring: 'drop_null_columns',
});
});
});
Expand Down
26 changes: 26 additions & 0 deletions test/functional/apps/discover/group3/_sidebar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

await PageObjects.discover.selectTextBaseLang();

const testQuery = `from logstash-* | limit 10000`;
await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded();
await PageObjects.unifiedFieldList.openSidebarFieldFilter();
options = await find.allByCssSelector('[data-test-subj*="typeFilter"]');
Expand All @@ -125,6 +129,25 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
);
});
});

it('should show empty fields in text-based view', async function () {
await kibanaServer.uiSettings.update({ 'discover:enableESQL': true });
await browser.refresh();

await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded();
await PageObjects.discover.selectTextBaseLang();

const testQuery = `from logstash-* | limit 10 | keep machine.ram_range, bytes `;
await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded();
await PageObjects.unifiedFieldList.openSidebarFieldFilter();

expect(await PageObjects.unifiedFieldList.getSidebarAriaDescription()).to.be(
'2 selected fields. 1 available field. 1 empty field.'
);
});
});

describe('search', function () {
Expand Down Expand Up @@ -422,6 +445,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
);

await PageObjects.discover.selectTextBaseLang();
await monacoEditor.setCodeEditorValue('from logstash-* | limit 10000');
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded();

expect(await PageObjects.unifiedFieldList.getSidebarAriaDescription()).to.be(
Expand Down
18 changes: 18 additions & 0 deletions test/functional/apps/discover/group4/_esql_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,24 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const cell = await dataGrid.getCellElement(0, 2);
expect(await cell.getVisibleText()).to.be('1');
});

it('should render correctly if there are empty fields', async function () {
await PageObjects.discover.selectTextBaseLang();
const testQuery = `from logstash-* | limit 10 | keep machine.ram_range, bytes`;

await monacoEditor.setCodeEditorValue(testQuery);
await testSubjects.click('querySubmitButton');
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.discover.waitUntilSearchingHasFinished();
const cell = await dataGrid.getCellElement(0, 3);
expect(await cell.getVisibleText()).to.be(' - ');
expect(await dataGrid.getHeaders()).to.eql([
'Control column',
'Select column',
'Numberbytes',
'machine.ram_range',
]);
});
});
describe('errors', () => {
it('should show error messages for syntax errors in query', async function () {
Expand Down

0 comments on commit ae04101

Please sign in to comment.