Support DECIMAL order-by for RANGE window functions #11645
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CUDF grouped RANGE window functions currently support only integral types and timestamps as the ORDER BY (OBY) column. This commit adds support for DECIMAL types (i.e. decimal32, decimal64, and decimal128) columns to by used as the ORDER BY column in RANGE window functions.
mythrocks
added
feature request
github-actions
bot
added
CMake
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-22.10 #11645 +/- ##
===============================================
Coverage ? 86.42%
===============================================
Files ? 145
Lines ? 23014
Branches ? 0
===============================================
Hits ? 19890
Misses ? 3124
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: MithunR <[email protected]>
Plus, scaling.
Signed-off-by: MithunR <[email protected]>
|
With the benefit of hindsight, I see that I could have also supported decimal range bounds with scales lower than that of the order-by column, simply by scaling in the other direction. The fear at the time was the loss of precision when scaling to a higher scale, but that should also apply in the other direction. I am not rocking the boat now. I'll raise an issue to tackle that separately from this PR. |
mythrocks
added
3 - Ready for Review
mythrocks
changed the title
davidwendt
requested changes
Sep 12, 2022
Also, fixed grammar error in a code comment.
hyperbolic2346
requested changes
Sep 12, 2022
1. Better documentation of how decimals are compared. 2. enable_if_t<> => CUDF_ENABLE_IF(). 3. Better names for test typedefs.
davidwendt
reviewed
Sep 12, 2022
davidwendt
reviewed
Sep 12, 2022
hyperbolic2346
approved these changes
Sep 13, 2022
davidwendt
approved these changes
Sep 13, 2022
jlowe
reviewed
Sep 13, 2022
1. Removed extra whitespaces, unnecessary comments. 2. Fixed copyright date. 3. Reused `big()` helper function. 4. Updated `Scalar.equals()` to include DECIMAL128. Signed-off-by: MithunR <[email protected]>
jlowe
approved these changes
Sep 13, 2022
|
Thank you for the reviews, all. I'll merge this change after CI is done cranking. |
|
@gpucibot merge |
This was referenced Sep 14, 2022
Merged
rapids-bot bot
pushed a commit
that referenced
this pull request
Sep 19, 2022
…11710) As a follow-up to #11645, this change includes `DECIMAL` among the list of data-types that may be used in the order-by column for `RANGE`-based window functions, listed in the exception message. Authors: - MithunR (https://github.com/mythrocks) Approvers: - Gera Shegalov (https://github.com/gerashegalov) - Nghia Truong (https://github.com/ttnghia) URL: #11710
3 tasks
rapids-bot bot
pushed a commit
that referenced
this pull request
Jun 20, 2023
This commit adds support for `FLOAT32` and `FLOAT64` order-by columns for RANGE-based window functions. ## Background Up until this commit, order-by columns for RANGE window functions were allowed to be integral numerics, timestamps, or strings (for unbounded/current rows). With this commit, window functions will be permitted to run on floating point value ranges. E.g. This supports windows defined with floating point deltas, like `rows with values exceeding the current row by no more than 3.14f`. This is in the same vein as the support for `DECIMAL` (#11645) and `STRING` (#13143). Authors: - MithunR (https://github.com/mythrocks) Approvers: - Nghia Truong (https://github.com/ttnghia) - David Wendt (https://github.com/davidwendt) URL: #13512
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
CUDF grouped RANGE window functions currently support only
integral types and timestamps as the ORDER BY (OBY) column.
This commit adds support for DECIMAL types (i.e. decimal32,
decimal64, and decimal128) to be used as the ORDER BY
column in RANGE window functions.
This feature allows
spark-rapidsto address NVIDIA/spark-rapids#6400.Checklist