Skip to content

Commit f7e61de

Browse files
drewdaemondrewdaemon
committed
[ES|QL] implicit casting changes (#182989)
## Summary This is a follow-on from elastic/elasticsearch#107859. Two main changes to our client-side validation code. 1. comparison operators like `==` and `>=` now support implicit casting from strings for `version`, `ip`, and `boolean` types 2. `in` and `not in` operators support implicit casting from strings for `version`, `ip`, `boolean`, and `date` constants. To address this quickly, I have disabled type-checking for array values (e.g. `in ( version_field, "1.2.3", "2.3.1" )`) because our parameter typing system does not currently support arrays of mixed types which it will need to in order to re-enable checking while allowing string literals. I have added this to #177699 ### A note on testing These implicit casting changes may not be on the latest Elasticsearch snapshot. Instead, check out the `8.14` branch in a local Elasticsearch repo and run `yarn es source --source-path='path/to/elasticsearch'` See [these tests](https://github.com/elastic/kibana/pull/182989/files#diff-89c4af0faedcf80d51cfb19ae397a5898c7293055b19284a94cb3f6a7cd4d071R1727-R1763) for a set of good use cases to try. `to_<type>` functions can be used if the sample data doesn't contain a field of the type you want to test. ### A note on string to date casting In #182856 we started accepting string literals for any function arguments that are dates. By the same logic, `"2022" - 1 day` would work, so our validator doesn't complain about it. However, it currently fails at the Elasticsearch level. In a discussion with Fang, we determined that this is a valid use case, so I have created elastic/elasticsearch#108432 and determined not to tighten the client-side validation since this seems more like a casting "hole" that will soon be filled in on the ES side (though not in 8.14). ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or (cherry picked from commit c3e1027)
1 parent a14ee25 commit f7e61de

4 files changed

Lines changed: 2638 additions & 1071 deletions

File tree

‎packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts‎

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,22 @@ function createComparisonDefinition(
9191
],
9292
returnType: 'boolean',
9393
},
94-
...['date', 'number'].flatMap((type) => [
94+
{
95+
params: [
96+
{ name: 'left', type: 'version' },
97+
{ name: 'right', type: 'version' },
98+
],
99+
returnType: 'boolean',
100+
},
101+
// constant strings okay because of implicit casting for
102+
// string to version and ip
103+
//
104+
// boolean casting is handled on the specific comparison function
105+
// that support booleans
106+
//
107+
// date casting is handled in the validation routine since it's a
108+
// general rule. Look in compareLiteralType()
109+
...['ip', 'version'].flatMap((type) => [
95110
{
96111
params: [
97112
{ name: 'left', type },
@@ -214,6 +229,21 @@ export const builtinFunctions: FunctionDefinition[] = [
214229
],
215230
returnType: 'boolean',
216231
},
232+
// constant strings okay because of implicit casting
233+
{
234+
params: [
235+
{ name: 'left', type: 'boolean' },
236+
{ name: 'right', type: 'string', constantOnly: true },
237+
],
238+
returnType: 'boolean',
239+
},
240+
{
241+
params: [
242+
{ name: 'right', type: 'string', constantOnly: true },
243+
{ name: 'right', type: 'boolean' },
244+
],
245+
returnType: 'boolean',
246+
},
217247
],
218248
},
219249
{
@@ -232,6 +262,21 @@ export const builtinFunctions: FunctionDefinition[] = [
232262
],
233263
returnType: 'boolean',
234264
},
265+
// constant strings okay because of implicit casting
266+
{
267+
params: [
268+
{ name: 'left', type: 'boolean' },
269+
{ name: 'right', type: 'string', constantOnly: true },
270+
],
271+
returnType: 'boolean',
272+
},
273+
{
274+
params: [
275+
{ name: 'right', type: 'string', constantOnly: true },
276+
{ name: 'right', type: 'boolean' },
277+
],
278+
returnType: 'boolean',
279+
},
235280
],
236281
},
237282
{
@@ -318,6 +363,15 @@ export const builtinFunctions: FunctionDefinition[] = [
318363
},
319364
{ name: 'not_in', description: '' },
320365
].map<FunctionDefinition>(({ name, description }) => ({
366+
// set all arrays to type "any" for now
367+
// this only applies to the "in" operator
368+
// e.g. "foo" in ( "foo", "bar" )
369+
//
370+
// we did this because the "in" operator now supports
371+
// mixed-type arrays like ( "1.2.3", versionVar )
372+
// because of implicit casting.
373+
//
374+
// we need to revisit with more robust validation
321375
type: 'builtin',
322376
ignoreAsSuggestion: /not/.test(name),
323377
name,
@@ -327,28 +381,43 @@ export const builtinFunctions: FunctionDefinition[] = [
327381
{
328382
params: [
329383
{ name: 'left', type: 'number' },
330-
{ name: 'right', type: 'number[]' },
384+
385+
{ name: 'right', type: 'any[]' },
331386
],
332387
returnType: 'boolean',
333388
},
334389
{
335390
params: [
336391
{ name: 'left', type: 'string' },
337-
{ name: 'right', type: 'string[]' },
392+
{ name: 'right', type: 'any[]' },
338393
],
339394
returnType: 'boolean',
340395
},
341396
{
342397
params: [
343398
{ name: 'left', type: 'boolean' },
344-
{ name: 'right', type: 'boolean[]' },
399+
{ name: 'right', type: 'any[]' },
345400
],
346401
returnType: 'boolean',
347402
},
348403
{
349404
params: [
350405
{ name: 'left', type: 'date' },
351-
{ name: 'right', type: 'date[]' },
406+
{ name: 'right', type: 'any[]' },
407+
],
408+
returnType: 'boolean',
409+
},
410+
{
411+
params: [
412+
{ name: 'left', type: 'version' },
413+
{ name: 'right', type: 'any[]' },
414+
],
415+
returnType: 'boolean',
416+
},
417+
{
418+
params: [
419+
{ name: 'left', type: 'ip' },
420+
{ name: 'right', type: 'any[]' },
352421
],
353422
returnType: 'boolean',
354423
},

‎packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [
557557
signatures: [
558558
{
559559
params: [{ name: 'field', type: 'string' }],
560-
returnType: 'string',
560+
returnType: 'version',
561561
examples: [`from index | EVAL version = to_version(stringField)`],
562562
},
563563
],

0 commit comments

Comments
 (0)