elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.35k stars 7.98k forks source link

[ES|QL] implicit casting changes #182989

Closed drewdaemon closed 1 week ago

drewdaemon commented 1 week ago

Summary

This is a follow-on from https://github.com/elastic/elasticsearch/pull/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 https://github.com/elastic/kibana/issues/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 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 https://github.com/elastic/kibana/pull/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-xing-esql , we determined that this is a valid use case, so I have created https://github.com/elastic/elasticsearch/issues/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

drewdaemon commented 1 week ago

/ci

drewdaemon commented 1 week ago

/ci

elasticmachine commented 1 week ago

Pinging @elastic/kibana-esql (Team:ESQL)

kibana-ci commented 1 week ago

:yellow_heart: Build succeeded, but was flaky

Failed CI Steps

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.1MB 3.1MB +679.0B

History

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

stratoula commented 1 week ago

Ok the latest snapshot has the changes now! 👍

kibanamachine commented 1 week ago

💚 All backports created successfully

Status Branch Result
8.14

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation