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 all 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
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`;
stratoula marked this conversation as resolved.
Show resolved Hide resolved
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