Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ES|QL] accept string constants for date args #182856

Merged
merged 2 commits into from
May 8, 2024

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented May 7, 2024

Summary

String constants in place of dates

As of elastic/elasticsearch#106932 string constants can always be used in place of dates.

  • | eval date_extract("DAY_OF_WEEK", "2024-05-07T13:20:40.554Z")
  • | STATS AVG(salary) BY bucket = BUCKET(hire_date, 20, "1985-01-01T00:00:00Z", "1986-01-01T00:00:00Z") | SORT bucket
  • | eval date_diff("day", "2021-01-01", "2022-01-01")

Checklist

@drewdaemon
Copy link
Contributor Author

/ci

@drewdaemon drewdaemon changed the title [ES|QL] accept string literals for date args [ES|QL] accept string constants for date args May 7, 2024
@drewdaemon drewdaemon added backport:prev-minor Backport to the previous minor version (i.e. one version back from main) Feature:ES|QL v8.14.0 Team:ESQL ES|QL related features in Kibana v8.15.0 release_note:skip Skip the PR/issue when compiling release notes labels May 7, 2024
@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@drewdaemon
Copy link
Contributor Author

/ci

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

The CI Stats report is too large to be displayed here, check out the CI build annotation for this information.

History

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

@drewdaemon drewdaemon marked this pull request as ready for review May 7, 2024 19:25
@drewdaemon drewdaemon requested a review from a team as a code owner May 7, 2024 19:25
@elasticmachine
Copy link
Contributor

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

@drewdaemon drewdaemon merged commit 9ddc3c6 into elastic:main May 8, 2024
18 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 8, 2024
## Summary

## String constants in place of dates

As of elastic/elasticsearch#106932 string
constants can always be used in place of dates.

- `| eval date_extract("DAY_OF_WEEK", "2024-05-07T13:20:40.554Z")`
- `| STATS AVG(salary) BY bucket = BUCKET(hire_date, 20,
"1985-01-01T00:00:00Z", "1986-01-01T00:00:00Z")
| SORT bucket`
- `| eval date_diff("day", "2021-01-01", "2022-01-01")`

### Checklist

- [x] [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

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 9ddc3c6)
@kibanamachine
Copy link
Contributor

💚 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

kibanamachine added a commit that referenced this pull request May 8, 2024
# Backport

This will backport the following commits from `main` to `8.14`:
- [[ES|QL] accept string constants for date args
(#182856)](#182856)

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

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

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"drew.tate@elastic.co"},"sourceCommit":{"committedDate":"2024-05-08T13:53:47Z","message":"[ES|QL]
accept string constants for date args (#182856)\n\n## Summary\r\n\r\n##
String constants in place of dates\r\n\r\nAs of
elastic/elasticsearch#106932 string\r\nconstants
can always be used in place of dates.\r\n\r\n- `| eval
date_extract(\"DAY_OF_WEEK\", \"2024-05-07T13:20:40.554Z\")`\r\n- `|
STATS AVG(salary) BY bucket = BUCKET(hire_date,
20,\r\n\"1985-01-01T00:00:00Z\", \"1986-01-01T00:00:00Z\")\r\n| SORT
bucket`\r\n- `| eval date_diff(\"day\", \"2021-01-01\",
\"2022-01-01\")`\r\n\r\n### Checklist\r\n\r\n- [x] [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\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"9ddc3c6aa836e9e5d3bdd30512f7de55d62a16d2","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:prev-minor","Feature:ES|QL","v8.14.0","Team:ESQL","v8.15.0"],"title":"[ES|QL]
accept string constants for date
args","number":182856,"url":"#182856
accept string constants for date args (#182856)\n\n## Summary\r\n\r\n##
String constants in place of dates\r\n\r\nAs of
elastic/elasticsearch#106932 string\r\nconstants
can always be used in place of dates.\r\n\r\n- `| eval
date_extract(\"DAY_OF_WEEK\", \"2024-05-07T13:20:40.554Z\")`\r\n- `|
STATS AVG(salary) BY bucket = BUCKET(hire_date,
20,\r\n\"1985-01-01T00:00:00Z\", \"1986-01-01T00:00:00Z\")\r\n| SORT
bucket`\r\n- `| eval date_diff(\"day\", \"2021-01-01\",
\"2022-01-01\")`\r\n\r\n### Checklist\r\n\r\n- [x] [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\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"9ddc3c6aa836e9e5d3bdd30512f7de55d62a16d2"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#182856
accept string constants for date args (#182856)\n\n## Summary\r\n\r\n##
String constants in place of dates\r\n\r\nAs of
elastic/elasticsearch#106932 string\r\nconstants
can always be used in place of dates.\r\n\r\n- `| eval
date_extract(\"DAY_OF_WEEK\", \"2024-05-07T13:20:40.554Z\")`\r\n- `|
STATS AVG(salary) BY bucket = BUCKET(hire_date,
20,\r\n\"1985-01-01T00:00:00Z\", \"1986-01-01T00:00:00Z\")\r\n| SORT
bucket`\r\n- `| eval date_diff(\"day\", \"2021-01-01\",
\"2022-01-01\")`\r\n\r\n### Checklist\r\n\r\n- [x] [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\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"9ddc3c6aa836e9e5d3bdd30512f7de55d62a16d2"}}]}]
BACKPORT-->

Co-authored-by: Drew Tate <drew.tate@elastic.co>
drewdaemon added a commit that referenced this pull request May 9, 2024
## 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
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 9, 2024
## 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 added a commit that referenced this pull request May 9, 2024
# Backport

This will backport the following commits from `main` to `8.14`:
- [[ES|QL] implicit casting changes
(#182989)](#182989)

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

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

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"drew.tate@elastic.co"},"sourceCommit":{"committedDate":"2024-05-09T05:51:18Z","message":"[ES|QL]
implicit casting changes (#182989)\n\n## Summary\r\n\r\nThis is a
follow-on
from\r\nhttps://github.com/elastic/elasticsearch/pull/107859.\r\n\r\nTwo
main changes to our client-side validation code.\r\n1. comparison
operators like `==` and `>=` now support implicit casting\r\nfrom
strings for `version`, `ip`, and `boolean` types\r\n\r\n2. `in` and `not
in` operators support implicit casting from strings for\r\n`version`,
`ip`, `boolean`, and `date` constants. To address this\r\nquickly, I
have disabled type-checking for array values (e.g. `in
(\r\nversion_field, \"1.2.3\", \"2.3.1\" )`) because our parameter
typing system\r\ndoes not currently support arrays of mixed types which
it will need to\r\nin order to re-enable checking while allowing string
literals. I have\r\nadded this to
#177699 A note on
testing\r\n\r\nThese implicit casting changes may not be on the latest
Elasticsearch\r\nsnapshot. Instead, check out the `8.14` branch in a
local Elasticsearch\r\nrepo and run `yarn es source
--source-path='path/to/elasticsearch'`\r\n\r\nSee
[these\r\ntests](https://github.com/elastic/kibana/pull/182989/files#diff-89c4af0faedcf80d51cfb19ae397a5898c7293055b19284a94cb3f6a7cd4d071R1727-R1763)\r\nfor
a set of good use cases to try. `to_<type>` functions can be used
if\r\nthe sample data doesn't contain a field of the type you want to
test.\r\n\r\n### A note on string to date casting\r\n\r\nIn
#182856 we started
accepting\r\nstring literals for any function arguments that are
dates.\r\n\r\nBy the same logic, `\"2022\" - 1 day` would work, so our
validator doesn't\r\ncomplain about it. However, it currently fails at
the Elasticsearch\r\nlevel.\r\n\r\nIn a discussion with Fang, we
determined that this is a\r\nvalid use case, so I have
created\r\nhttps://github.com/elastic/elasticsearch/issues/108432 and
determined\r\nnot to tighten the client-side validation since this seems
more like a\r\ncasting \"hole\" that will soon be filled in on the ES
side (though not in\r\n8.14).\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated
or","sha":"c3e1027b2d5da9361251e3af3304098717193ded","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:prev-minor","Feature:ES|QL","v8.14.0","Team:ESQL","v8.15.0"],"title":"[ES|QL]
implicit casting
changes","number":182989,"url":"#182989
implicit casting changes (#182989)\n\n## Summary\r\n\r\nThis is a
follow-on
from\r\nhttps://github.com/elastic/elasticsearch/pull/107859.\r\n\r\nTwo
main changes to our client-side validation code.\r\n1. comparison
operators like `==` and `>=` now support implicit casting\r\nfrom
strings for `version`, `ip`, and `boolean` types\r\n\r\n2. `in` and `not
in` operators support implicit casting from strings for\r\n`version`,
`ip`, `boolean`, and `date` constants. To address this\r\nquickly, I
have disabled type-checking for array values (e.g. `in
(\r\nversion_field, \"1.2.3\", \"2.3.1\" )`) because our parameter
typing system\r\ndoes not currently support arrays of mixed types which
it will need to\r\nin order to re-enable checking while allowing string
literals. I have\r\nadded this to
#177699 A note on
testing\r\n\r\nThese implicit casting changes may not be on the latest
Elasticsearch\r\nsnapshot. Instead, check out the `8.14` branch in a
local Elasticsearch\r\nrepo and run `yarn es source
--source-path='path/to/elasticsearch'`\r\n\r\nSee
[these\r\ntests](https://github.com/elastic/kibana/pull/182989/files#diff-89c4af0faedcf80d51cfb19ae397a5898c7293055b19284a94cb3f6a7cd4d071R1727-R1763)\r\nfor
a set of good use cases to try. `to_<type>` functions can be used
if\r\nthe sample data doesn't contain a field of the type you want to
test.\r\n\r\n### A note on string to date casting\r\n\r\nIn
#182856 we started
accepting\r\nstring literals for any function arguments that are
dates.\r\n\r\nBy the same logic, `\"2022\" - 1 day` would work, so our
validator doesn't\r\ncomplain about it. However, it currently fails at
the Elasticsearch\r\nlevel.\r\n\r\nIn a discussion with Fang, we
determined that this is a\r\nvalid use case, so I have
created\r\nhttps://github.com/elastic/elasticsearch/issues/108432 and
determined\r\nnot to tighten the client-side validation since this seems
more like a\r\ncasting \"hole\" that will soon be filled in on the ES
side (though not in\r\n8.14).\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated
or","sha":"c3e1027b2d5da9361251e3af3304098717193ded"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#182989
implicit casting changes (#182989)\n\n## Summary\r\n\r\nThis is a
follow-on
from\r\nhttps://github.com/elastic/elasticsearch/pull/107859.\r\n\r\nTwo
main changes to our client-side validation code.\r\n1. comparison
operators like `==` and `>=` now support implicit casting\r\nfrom
strings for `version`, `ip`, and `boolean` types\r\n\r\n2. `in` and `not
in` operators support implicit casting from strings for\r\n`version`,
`ip`, `boolean`, and `date` constants. To address this\r\nquickly, I
have disabled type-checking for array values (e.g. `in
(\r\nversion_field, \"1.2.3\", \"2.3.1\" )`) because our parameter
typing system\r\ndoes not currently support arrays of mixed types which
it will need to\r\nin order to re-enable checking while allowing string
literals. I have\r\nadded this to
#177699 A note on
testing\r\n\r\nThese implicit casting changes may not be on the latest
Elasticsearch\r\nsnapshot. Instead, check out the `8.14` branch in a
local Elasticsearch\r\nrepo and run `yarn es source
--source-path='path/to/elasticsearch'`\r\n\r\nSee
[these\r\ntests](https://github.com/elastic/kibana/pull/182989/files#diff-89c4af0faedcf80d51cfb19ae397a5898c7293055b19284a94cb3f6a7cd4d071R1727-R1763)\r\nfor
a set of good use cases to try. `to_<type>` functions can be used
if\r\nthe sample data doesn't contain a field of the type you want to
test.\r\n\r\n### A note on string to date casting\r\n\r\nIn
#182856 we started
accepting\r\nstring literals for any function arguments that are
dates.\r\n\r\nBy the same logic, `\"2022\" - 1 day` would work, so our
validator doesn't\r\ncomplain about it. However, it currently fails at
the Elasticsearch\r\nlevel.\r\n\r\nIn a discussion with Fang, we
determined that this is a\r\nvalid use case, so I have
created\r\nhttps://github.com/elastic/elasticsearch/issues/108432 and
determined\r\nnot to tighten the client-side validation since this seems
more like a\r\ncasting \"hole\" that will soon be filled in on the ES
side (though not in\r\n8.14).\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or","sha":"c3e1027b2d5da9361251e3af3304098717193ded"}}]}]
BACKPORT-->

Co-authored-by: Drew Tate <drew.tate@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to the previous minor version (i.e. one version back from main) Feature:ES|QL release_note:skip Skip the PR/issue when compiling release notes Team:ESQL ES|QL related features in Kibana v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants