Skip to content

[8.14] [ES|QL] implicit casting changes (#182989)#183011

Merged
kibanamachine merged 1 commit intoelastic:8.14from
kibanamachine:backport/8.14/pr-182989
May 9, 2024
Merged

[8.14] [ES|QL] implicit casting changes (#182989)#183011
kibanamachine merged 1 commit intoelastic:8.14from
kibanamachine:backport/8.14/pr-182989

Conversation

@kibanamachine
Copy link
Copy Markdown
Contributor

Backport

This will backport the following commits from main to 8.14:

Questions ?

Please refer to the Backport tool documentation

## 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 elastic#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 elastic#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)
@kibanamachine kibanamachine added the backport This PR is a backport of another PR label May 9, 2024
@kibanamachine kibanamachine enabled auto-merge (squash) May 9, 2024 05:55
@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-srcJs 3.2MB 3.2MB +679.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @drewdaemon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants