-
Notifications
You must be signed in to change notification settings - Fork 155
Add earliest and latest in aggregation and window function #3640
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
|
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.
In SPL doc, these functions are applied to _raw
field, but in our impl it's just an alias of min/max?
Signed-off-by: xinyual <[email protected]>
Add the doc. |
In aggregation, they just calculate the latest/earliest time. Since we implement time related column as string, we can directly reuse min/max for them. Also in SPL, _raw is regarded as a string field. |
Please fix IT |
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Already fix it by fixing the doc. |
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
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.
please make sure the cases you added could pass in doc-test with calcite enabled in local then change the os>
to PPL>
core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java
Outdated
Show resolved
Hide resolved
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
Signed-off-by: xinyual <[email protected]>
"1970-01-18 20:22:32", | ||
"2018-08-19 00:00:00")); |
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 am afraid the current implementation may be wrong, the current implementation treat the earliest
and latest
as window function with default window frame. IMO, the behavior is odd. Please check again if they require to use other window frame instead default.
Description
This pr add latest/earliest as two aggregation function, it will calculate the earliest/latest time for the time related column. Since we support time related type as string (timestamp/time/date), currently we directly reuse max/min to do it.
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
#3639
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.