Skip to content

Commit

Permalink
[ES|QL] Don't suggest unsupported fields (elastic#200544)
Browse files Browse the repository at this point in the history
## Summary

Closes elastic#189666

Specifically:

- we do not suggest unsupported fields
- we add a warning if the field is used in the query

### How to test

1. Create an index with unsupported fields. I used this

```
PUT my-index-test-unsupported
{
  "mappings" : {
    "properties" : {
      "my_histogram" : {
        "type" : "histogram"
      },
      "my_text" : {
        "type" : "keyword"
      }
    }
  }
}

PUT my-index-test-unsupported/_doc/1
{
  "my_text" : "histogram_1",
  "my_histogram" : {
      "values" : [0.1, 0.2, 0.3, 0.4, 0.5], 
      "counts" : [3, 7, 23, 12, 6] 
   }
}

PUT my-index-test-unsupported/_doc/2
{
  "my_text" : "histogram_2",
  "my_histogram" : {
      "values" : [0.1, 0.25, 0.35, 0.4, 0.45, 0.5], 
      "counts" : [8, 17, 8, 7, 6, 2] 
   }
}
```

2. Go to the editor and try the autocomplete, it should not suggest the
histogram field

<img width="998" alt="image"
src="https://github.com/user-attachments/assets/2a9b4594-1f8f-4e7a-9ea3-d9086c770178">

3. Use a query like `FROM my-index-test-unsupported | KEEP my_histogram
`. You should see a warning

<img width="991" alt="image"
src="https://github.com/user-attachments/assets/1bf747da-191f-46e2-b1b8-e5966653f405">


### Checklist

- [ ] [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
  • Loading branch information
stratoula authored and jesuswr committed Nov 18, 2024
1 parent e1ce698 commit 5edb9ab
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 5edb9ab

Please sign in to comment.