-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
[ESQL] Mark date_diff as requiring all three arguments #108834
[ESQL] Mark date_diff as requiring all three arguments #108834
Conversation
Pinging @elastic/es-analytical-engine (Team:Analytics) |
Hi @not-napoleon, I've created a changelog YAML for you. |
…-diff-missing-arg' into esql-fix-date-diff-missing-arg
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.
I guess a test could invoke date_diff with just 2 args and observe the parsing exception, but that's a bit superfluous, so it LGTM.
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.
LGTM
Since this is a bug, please backport it to 8.14. |
💚 Backport successful
|
Resolves elastic#108383 I didn't see a good place to add a test for this; if there is one, let me know and I'm happy to add it. This fixes a bug where we had incorrectly marked date_diff as having an optional argument, which resulted in an NPE when that argument was not provided. In fact, all three of date_diff's arguments are required. After this change, failing to provide one will cause a parse exception, as one would expect.
…8874) Resolves #108383 I didn't see a good place to add a test for this; if there is one, let me know and I'm happy to add it. This fixes a bug where we had incorrectly marked date_diff as having an optional argument, which resulted in an NPE when that argument was not provided. In fact, all three of date_diff's arguments are required. After this change, failing to provide one will cause a parse exception, as one would expect.
Resolves #108383
I didn't see a good place to add a test for this; if there is one, let me know and I'm happy to add it.
This fixes a bug where we had incorrectly marked
date_diff
as having an optional argument, which resulted in an NPE when that argument was not provided. In fact, all three ofdate_diff
's arguments are required. After this change, failing to provide one will cause a parse exception, as one would expect.