Skip to content

Commit

Permalink
[8.x] [ES|QL] Don't suggest unsupported fields (#200544) (#200598)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Don't suggest unsupported fields
(#200544)](#200544)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-18T15:45:25Z","message":"[ES|QL]
Don't suggest unsupported fields (#200544)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/189666\r\n\r\nSpecifically:\r\n\r\n-
we do not suggest unsupported fields\r\n- we add a warning if the field
is used in the query\r\n\r\n### How to test\r\n\r\n1. Create an index
with unsupported fields. I used this\r\n\r\n```\r\nPUT
my-index-test-unsupported\r\n{\r\n \"mappings\" : {\r\n \"properties\" :
{\r\n \"my_histogram\" : {\r\n \"type\" : \"histogram\"\r\n },\r\n
\"my_text\" : {\r\n \"type\" : \"keyword\"\r\n }\r\n }\r\n
}\r\n}\r\n\r\nPUT my-index-test-unsupported/_doc/1\r\n{\r\n \"my_text\"
: \"histogram_1\",\r\n \"my_histogram\" : {\r\n \"values\" : [0.1, 0.2,
0.3, 0.4, 0.5], \r\n \"counts\" : [3, 7, 23, 12, 6] \r\n
}\r\n}\r\n\r\nPUT my-index-test-unsupported/_doc/2\r\n{\r\n \"my_text\"
: \"histogram_2\",\r\n \"my_histogram\" : {\r\n \"values\" : [0.1, 0.25,
0.35, 0.4, 0.45, 0.5], \r\n \"counts\" : [8, 17, 8, 7, 6, 2] \r\n
}\r\n}\r\n```\r\n\r\n2. Go to the editor and try the autocomplete, it
should not suggest the\r\nhistogram field\r\n\r\n<img width=\"998\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/2a9b4594-1f8f-4e7a-9ea3-d9086c770178\">\r\n\r\n3.
Use a query like `FROM my-index-test-unsupported | KEEP
my_histogram\r\n`. You should see a warning\r\n\r\n<img width=\"991\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1bf747da-191f-46e2-b1b8-e5966653f405\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3c3f2c11a9da2a26f75f601ee365af1a227295b6","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","Feature:ES|QL","Team:ESQL","backport:version","v8.17.0"],"title":"[ES|QL]
Don't suggest unsupported
fields","number":200544,"url":"https://github.com/elastic/kibana/pull/200544","mergeCommit":{"message":"[ES|QL]
Don't suggest unsupported fields (#200544)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/189666\r\n\r\nSpecifically:\r\n\r\n-
we do not suggest unsupported fields\r\n- we add a warning if the field
is used in the query\r\n\r\n### How to test\r\n\r\n1. Create an index
with unsupported fields. I used this\r\n\r\n```\r\nPUT
my-index-test-unsupported\r\n{\r\n \"mappings\" : {\r\n \"properties\" :
{\r\n \"my_histogram\" : {\r\n \"type\" : \"histogram\"\r\n },\r\n
\"my_text\" : {\r\n \"type\" : \"keyword\"\r\n }\r\n }\r\n
}\r\n}\r\n\r\nPUT my-index-test-unsupported/_doc/1\r\n{\r\n \"my_text\"
: \"histogram_1\",\r\n \"my_histogram\" : {\r\n \"values\" : [0.1, 0.2,
0.3, 0.4, 0.5], \r\n \"counts\" : [3, 7, 23, 12, 6] \r\n
}\r\n}\r\n\r\nPUT my-index-test-unsupported/_doc/2\r\n{\r\n \"my_text\"
: \"histogram_2\",\r\n \"my_histogram\" : {\r\n \"values\" : [0.1, 0.25,
0.35, 0.4, 0.45, 0.5], \r\n \"counts\" : [8, 17, 8, 7, 6, 2] \r\n
}\r\n}\r\n```\r\n\r\n2. Go to the editor and try the autocomplete, it
should not suggest the\r\nhistogram field\r\n\r\n<img width=\"998\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/2a9b4594-1f8f-4e7a-9ea3-d9086c770178\">\r\n\r\n3.
Use a query like `FROM my-index-test-unsupported | KEEP
my_histogram\r\n`. You should see a warning\r\n\r\n<img width=\"991\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1bf747da-191f-46e2-b1b8-e5966653f405\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3c3f2c11a9da2a26f75f601ee365af1a227295b6"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200544","number":200544,"mergeCommit":{"message":"[ES|QL]
Don't suggest unsupported fields (#200544)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/189666\r\n\r\nSpecifically:\r\n\r\n-
we do not suggest unsupported fields\r\n- we add a warning if the field
is used in the query\r\n\r\n### How to test\r\n\r\n1. Create an index
with unsupported fields. I used this\r\n\r\n```\r\nPUT
my-index-test-unsupported\r\n{\r\n \"mappings\" : {\r\n \"properties\" :
{\r\n \"my_histogram\" : {\r\n \"type\" : \"histogram\"\r\n },\r\n
\"my_text\" : {\r\n \"type\" : \"keyword\"\r\n }\r\n }\r\n
}\r\n}\r\n\r\nPUT my-index-test-unsupported/_doc/1\r\n{\r\n \"my_text\"
: \"histogram_1\",\r\n \"my_histogram\" : {\r\n \"values\" : [0.1, 0.2,
0.3, 0.4, 0.5], \r\n \"counts\" : [3, 7, 23, 12, 6] \r\n
}\r\n}\r\n\r\nPUT my-index-test-unsupported/_doc/2\r\n{\r\n \"my_text\"
: \"histogram_2\",\r\n \"my_histogram\" : {\r\n \"values\" : [0.1, 0.25,
0.35, 0.4, 0.45, 0.5], \r\n \"counts\" : [8, 17, 8, 7, 6, 2] \r\n
}\r\n}\r\n```\r\n\r\n2. Go to the editor and try the autocomplete, it
should not suggest the\r\nhistogram field\r\n\r\n<img width=\"998\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/2a9b4594-1f8f-4e7a-9ea3-d9086c770178\">\r\n\r\n3.
Use a query like `FROM my-index-test-unsupported | KEEP
my_histogram\r\n`. You should see a warning\r\n\r\n<img width=\"991\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/1bf747da-191f-46e2-b1b8-e5966653f405\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3c3f2c11a9da2a26f75f601ee365af1a227295b6"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
kibanamachine and stratoula authored Nov 18, 2024
1 parent 47d9e14 commit dd67c8c
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import { ESQLRealField } from '../validation/types';
import { fieldTypes } from '../definitions/types';

export const fields: ESQLRealField[] = [
...fieldTypes
.map((type) => ({ name: `${camelCase(type)}Field`, type }))
.filter((f) => f.type !== 'unsupported'),
...fieldTypes.map((type) => ({ name: `${camelCase(type)}Field`, type })),
{ name: 'any#Char$Field', type: 'double' },
{ name: 'kubernetes.something.something', type: 'double' },
{ name: '@timestamp', type: 'date' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { camelCase } from 'lodash';
import { parse } from '@kbn/esql-ast';
import { scalarFunctionDefinitions } from '../../definitions/generated/scalar_functions';
import { builtinFunctions } from '../../definitions/builtin';
import { NOT_SUGGESTED_TYPES } from '../../shared/resources_helpers';
import { aggregationFunctionDefinitions } from '../../definitions/generated/aggregation_functions';
import { timeUnitsToSuggest } from '../../definitions/literals';
import { groupingFunctionDefinitions } from '../../definitions/grouping';
Expand Down Expand Up @@ -229,7 +230,11 @@ export function getFieldNamesByType(
) {
const requestedType = Array.isArray(_requestedType) ? _requestedType : [_requestedType];
return fields
.filter(({ type }) => requestedType.includes('any') || requestedType.includes(type))
.filter(
({ type }) =>
(requestedType.includes('any') || requestedType.includes(type)) &&
!NOT_SUGGESTED_TYPES.includes(type)
)
.map(({ name, suggestedAs }) => suggestedAs || name);
}

Expand Down Expand Up @@ -267,7 +272,9 @@ export function createCustomCallbackMocks(
enrichFields: string[];
}>
) {
const finalColumnsSinceLastCommand = customColumnsSinceLastCommand || fields;
const finalColumnsSinceLastCommand =
customColumnsSinceLastCommand ||
fields.filter(({ type }) => !NOT_SUGGESTED_TYPES.includes(type));
const finalSources = customSources || indexes;
const finalPolicies = customPolicies || policies;
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ describe('autocomplete', () => {
'FROM a | ENRICH policy /',
['ON $0', 'WITH $0', '| '].map(attachTriggerCommand)
);

testSuggestions(
'FROM a | ENRICH policy ON /',
getFieldNamesByType('any')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import type { ESQLCallbacks } from './types';
import type { ESQLRealField } from '../validation/types';
import { enrichFieldsWithECSInfo } from '../autocomplete/utils/ecs_metadata_helper';

export const NOT_SUGGESTED_TYPES = ['unsupported'];

export function buildQueryUntilPreviousCommand(ast: ESQLAst, queryString: string) {
const prevCommand = ast[Math.max(ast.length - 2, 0)];
return prevCommand ? queryString.substring(0, prevCommand.location.max + 1) : queryString;
Expand Down Expand Up @@ -54,7 +56,11 @@ export function getFieldsByTypeHelper(queryText: string, resourceRetriever?: ESQ
return (
Array.from(cacheFields.values())?.filter(({ name, type }) => {
const ts = Array.isArray(type) ? type : [type];
return !ignored.includes(name) && ts.some((t) => types[0] === 'any' || types.includes(t));
return (
!ignored.includes(name) &&
ts.some((t) => types[0] === 'any' || types.includes(t)) &&
!NOT_SUGGESTED_TYPES.includes(type)
);
}) || []
);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@
"name": "counterDoubleField",
"type": "counter_double"
},
{
"name": "unsupportedField",
"type": "unsupported"
},
{
"name": "dateNanosField",
"type": "date_nanos"
Expand Down Expand Up @@ -9690,6 +9694,13 @@
],
"warning": []
},
{
"query": "from a_index | keep unsupportedField",
"error": [],
"warning": [
"Field [unsupportedField] cannot be retrieved, it is unsupported or not indexed; returning null"
]
},
{
"query": "f",
"error": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1695,6 +1695,16 @@ describe('validation logic', () => {
['Argument of [trim] must be [keyword], found value [doubleField] type [double]']
);
});

describe('unsupported fields', () => {
testErrorsAndWarnings(
`from a_index | keep unsupportedField`,
[],
[
'Field [unsupportedField] cannot be retrieved, it is unsupported or not indexed; returning null',
]
);
});
});

describe('Ignoring errors based on callbacks', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1223,21 +1223,24 @@ function validateFieldsShadowing(
return messages;
}

function validateUnsupportedTypeFields(fields: Map<string, ESQLRealField>) {
function validateUnsupportedTypeFields(fields: Map<string, ESQLRealField>, ast: ESQLAst) {
const usedColumnsInQuery: string[] = [];

walk(ast, {
visitColumn: (node) => usedColumnsInQuery.push(node.name),
});
const messages: ESQLMessage[] = [];
for (const field of fields.values()) {
if (field.type === 'unsupported') {
// Removed temporarily to supress all these warnings
// Issue to re-enable in a better way: https://github.com/elastic/kibana/issues/189666
// messages.push(
// getMessageFromId({
// messageId: 'unsupportedFieldType',
// values: {
// field: field.name,
// },
// locations: { min: 1, max: 1 },
// })
// );
for (const column of usedColumnsInQuery) {
if (fields.has(column) && fields.get(column)!.type === 'unsupported') {
messages.push(
getMessageFromId({
messageId: 'unsupportedFieldType',
values: {
field: column,
},
locations: { min: 1, max: 1 },
})
);
}
}
return messages;
Expand Down Expand Up @@ -1350,7 +1353,7 @@ async function validateAst(
const variables = collectVariables(ast, availableFields, queryString);
// notify if the user is rewriting a column as variable with another type
messages.push(...validateFieldsShadowing(availableFields, variables));
messages.push(...validateUnsupportedTypeFields(availableFields));
messages.push(...validateUnsupportedTypeFields(availableFields, ast));

for (const [index, command] of ast.entries()) {
const references: ReferenceMaps = {
Expand Down

0 comments on commit dd67c8c

Please sign in to comment.