mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
[ES|QL] Identify conflict types (#217656)
## Summary Closes https://github.com/elastic/kibana/issues/215157 This is fixing the wrong client side validation error that appears here. ``` FROM kibana_sample_data_ecommerce | EVAL customer_id = TO_LONG(customer_id) | LOOKUP JOIN customers ON customer_id ``` <img width="588" alt="image" src="https://github.com/user-attachments/assets/50a9365f-91c6-45c8-ad04-48be5145eb77" /> We are using the new originalTypes that is being returned by ES to identify if the field is a conflict and turn off the client side validation for these fields. This is not closing the aforementioned issue. It is the first step to improve the join autocomplete experience. The fields retrieval for joins is happening by running `from index1, lookup_index` but in case of conflicts this is problematic. We could def improve it in the future but identifying for now that this is a conflict, muting the client side validation and let ES handling the error is better than what we have now. As a bonus the field list recognizes them too as conflict now <img width="683" alt="image" src="https://github.com/user-attachments/assets/7edf8cdf-156b-4933-b9dc-225e211bf2ec" /> ### Checklist - [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: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
parent
ae9e5d679c
commit
685f026c29
10 changed files with 70 additions and 33 deletions
|
@ -21,6 +21,7 @@ import { i18n } from '@kbn/i18n';
|
|||
import moment from 'moment';
|
||||
import { isEqual } from 'lodash';
|
||||
import { CodeEditor, CodeEditorProps } from '@kbn/code-editor';
|
||||
import { KBN_FIELD_TYPES } from '@kbn/field-types';
|
||||
import type { CoreStart } from '@kbn/core/public';
|
||||
import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';
|
||||
import type { AggregateQuery, TimeRange } from '@kbn/es-query';
|
||||
|
@ -477,6 +478,7 @@ export const ESQLEditor = memo(function ESQLEditor({
|
|||
return {
|
||||
name: c.name,
|
||||
type: c.meta.esType as FieldType,
|
||||
hasConflict: c.meta.type === KBN_FIELD_TYPES.CONFLICT,
|
||||
};
|
||||
}) || [];
|
||||
|
||||
|
|
|
@ -35,6 +35,7 @@
|
|||
"@kbn/ui-actions-plugin",
|
||||
"@kbn/shared-ux-table-persist",
|
||||
"@kbn/esql-types",
|
||||
"@kbn/field-types",
|
||||
"@kbn/licensing-plugin"
|
||||
],
|
||||
"exclude": [
|
||||
|
|
|
@ -657,6 +657,7 @@ export type InferSearchResponseOf<
|
|||
export interface ESQLColumn {
|
||||
name: string;
|
||||
type: string;
|
||||
original_types?: string[];
|
||||
}
|
||||
|
||||
export type ESQLRow = unknown[];
|
||||
|
|
|
@ -64,6 +64,11 @@ export const joinIndices: JoinIndexAutocompleteItem[] = [
|
|||
mode: 'lookup',
|
||||
aliases: ['join_index_alias_1', 'join_index_alias_2'],
|
||||
},
|
||||
{
|
||||
name: 'lookup_index',
|
||||
mode: 'lookup',
|
||||
aliases: [],
|
||||
},
|
||||
];
|
||||
|
||||
export function getCallbackMocks(): ESQLCallbacks {
|
||||
|
@ -79,6 +84,14 @@ export function getCallbackMocks(): ESQLCallbacks {
|
|||
const field: ESQLRealField = { name: 'firstWord', type: 'text' };
|
||||
return [field];
|
||||
}
|
||||
if (/join_index/.test(query)) {
|
||||
const field: ESQLRealField = {
|
||||
name: 'keywordField',
|
||||
type: 'unsupported',
|
||||
hasConflict: true,
|
||||
};
|
||||
return [field];
|
||||
}
|
||||
return fields;
|
||||
}),
|
||||
getSources: jest.fn(async () =>
|
||||
|
|
|
@ -68,6 +68,7 @@ describe('autocomplete.suggest', () => {
|
|||
expect(labels).toEqual([
|
||||
'join_index',
|
||||
'join_index_with_alias',
|
||||
'lookup_index',
|
||||
'join_index_alias_1',
|
||||
'join_index_alias_2',
|
||||
]);
|
||||
|
@ -86,7 +87,7 @@ describe('autocomplete.suggest', () => {
|
|||
.map((s) => s.label)
|
||||
.sort();
|
||||
|
||||
expect(indices).toEqual(['join_index', 'join_index_with_alias']);
|
||||
expect(indices).toEqual(['join_index', 'join_index_with_alias', 'lookup_index']);
|
||||
expect(aliases).toEqual(['join_index_alias_1', 'join_index_alias_2']);
|
||||
});
|
||||
});
|
||||
|
|
|
@ -69,6 +69,7 @@ describe('joinIndicesToSuggestions()', () => {
|
|||
expect(labels).toEqual([
|
||||
'join_index',
|
||||
'join_index_with_alias',
|
||||
'lookup_index',
|
||||
'join_index_alias_1',
|
||||
'join_index_alias_2',
|
||||
]);
|
||||
|
|
|
@ -44,6 +44,15 @@ export const validationJoinCommandTestSuite = (setup: helpers.Setup) => {
|
|||
await expectErrors('FROM index | LEFT JOIN join_index_alias_1 ON stringField', []);
|
||||
await expectErrors('FROM index | LEFT JOIN join_index_alias_2 ON stringField', []);
|
||||
});
|
||||
|
||||
test('handles correctly conflicts', async () => {
|
||||
const { expectErrors } = await setup();
|
||||
|
||||
await expectErrors(
|
||||
'FROM index | EVAL keywordField = to_IP(keywordField) | LEFT JOIN join_index ON keywordField',
|
||||
[]
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test.todo('... AS <alias> ...');
|
||||
|
|
|
@ -545,18 +545,21 @@ function validateFunctionColumnArg(
|
|||
!checkFunctionArgMatchesDefinition(actualArg, parameterDefinition, references, parentCommand)
|
||||
) {
|
||||
const columnHit = getColumnForASTNode(actualArg, references);
|
||||
messages.push(
|
||||
getMessageFromId({
|
||||
messageId: 'wrongArgumentType',
|
||||
values: {
|
||||
name: astFunction.name,
|
||||
argType: parameterDefinition.type as string,
|
||||
value: actualArg.name,
|
||||
givenType: columnHit!.type,
|
||||
},
|
||||
locations: actualArg.location,
|
||||
})
|
||||
);
|
||||
const isConflictType = columnHit && 'hasConflict' in columnHit && columnHit.hasConflict;
|
||||
if (!isConflictType) {
|
||||
messages.push(
|
||||
getMessageFromId({
|
||||
messageId: 'wrongArgumentType',
|
||||
values: {
|
||||
name: astFunction.name,
|
||||
argType: parameterDefinition.type as string,
|
||||
value: actualArg.name,
|
||||
givenType: columnHit!.type,
|
||||
},
|
||||
locations: actualArg.location,
|
||||
})
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return messages;
|
||||
|
|
|
@ -24,6 +24,7 @@ export interface ESQLRealField {
|
|||
name: string;
|
||||
type: FieldType;
|
||||
isEcs?: boolean;
|
||||
hasConflict?: boolean;
|
||||
metadata?: {
|
||||
description?: string;
|
||||
};
|
||||
|
|
|
@ -8,7 +8,7 @@
|
|||
*/
|
||||
|
||||
import type { KibanaRequest } from '@kbn/core/server';
|
||||
import { esFieldTypeToKibanaFieldType } from '@kbn/field-types';
|
||||
import { esFieldTypeToKibanaFieldType, KBN_FIELD_TYPES } from '@kbn/field-types';
|
||||
import { i18n } from '@kbn/i18n';
|
||||
import type {
|
||||
IKibanaSearchRequest,
|
||||
|
@ -351,25 +351,30 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
|
|||
: undefined;
|
||||
|
||||
const allColumns =
|
||||
(body.all_columns ?? body.columns)?.map(({ name, type }) => ({
|
||||
id: name,
|
||||
name,
|
||||
meta: {
|
||||
type: esFieldTypeToKibanaFieldType(type),
|
||||
esType: type,
|
||||
sourceParams:
|
||||
type === 'date'
|
||||
? {
|
||||
appliedTimeRange,
|
||||
params: {},
|
||||
indexPattern,
|
||||
}
|
||||
: {
|
||||
indexPattern,
|
||||
},
|
||||
},
|
||||
isNull: hasEmptyColumns ? !lookup.has(name) : false,
|
||||
})) ?? [];
|
||||
// eslint-disable-next-line @typescript-eslint/naming-convention
|
||||
(body.all_columns ?? body.columns)?.map(({ name, type, original_types }) => {
|
||||
const originalTypes = original_types ?? [];
|
||||
const hasConflict = type === 'unsupported' && originalTypes.length > 1;
|
||||
return {
|
||||
id: name,
|
||||
name,
|
||||
meta: {
|
||||
type: hasConflict ? KBN_FIELD_TYPES.CONFLICT : esFieldTypeToKibanaFieldType(type),
|
||||
esType: type,
|
||||
sourceParams:
|
||||
type === 'date'
|
||||
? {
|
||||
appliedTimeRange,
|
||||
params: {},
|
||||
indexPattern,
|
||||
}
|
||||
: {
|
||||
indexPattern,
|
||||
},
|
||||
},
|
||||
isNull: hasEmptyColumns ? !lookup.has(name) : false,
|
||||
};
|
||||
}) ?? [];
|
||||
|
||||
const fixedQuery = fixESQLQueryWithVariables(query, input?.esqlVariables ?? []);
|
||||
const updatedWithVariablesColumns = mapVariableToColumn(
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue