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] Convert string to datetime when the other size of an arithmetic operator is date_period or time_duration #108455
base: main
Are you sure you want to change the base?
Conversation
Pinging @elastic/es-analytical-engine (Team:Analytics) |
Hi @fang-xing-esql, I've created a changelog YAML for you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @fang-xing-esql , looks good!
I'd increase test coverage a bit, though; maybe we could also add an analyzer/verifier test that confirms that arbitrary functions producing a (foldable) string can't be used, so that e.g. mv_concat("2024", "-04", "-01") + 1 day
is invalid.
@@ -0,0 +1,6 @@ | |||
pr: 108455 | |||
summary: "[ES|QL] Convert string to datetime when the other size of an arithmetic\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
summary: "[ES|QL] Convert string to datetime when the other size of an arithmetic\ | |
summary: "[ES|QL] Convert string to datetime when the other side of an arithmetic\ |
You'll probably need to update the typo in the PR title as well, otherwise the bot may not honor any changes to the changelog.
implicitCastingArithmeticOperation | ||
required_capability: string_literal_auto_casting_to_datetime_add_sub | ||
from employees | ||
| eval x = 1 day + "2024-01-01", y = 1 year + "2024-04-01" + 1 month, z = "2024-01-01" + 3600 seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Maybe let's add a test for
to_datetime(null) + 1 day
(and other durations/periods) as well. - This does not cover subtractions, let's add a test for those as well.
- Optionally, we could also add tests for expressions like
"2024-04-01" + (1 year + 1 day)
and expressions where the literal datetime has a an hour/minute/second as well, although this is getting into paranoid territory.
Resolves : #108432
eval x = 1 day + "2024-01-01"
this will work after this change. Users don't need to call to_dt explicitly like thiseval x = 1 day + to_dt("2024-01-01")