Use .element() instead of .data() for window range calculations
#13095
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
…nge calculations. This change is in preparation for supporting `STRING` order-by columns in range window functions. In the staging step for executing window range queries, the boundaries of each row's window are calculated. This involves subtracting/adding the `preceding`/`following` values from each order-by column row, and then searching backwards/forwards for the boundary values. The staging step has been using `column_device_view.data()` for accessing the order-by rows, an acceptable approach for when the order-by columns are numeric (e.g. `INT32`). This approach fails when the order-by column is a `STRING`, because `.data()` is not defined for such columns. A better approach would be to use `.element()` to directly access the rows, because it has special handling for `STRING`, among other types, while continuing to work for numeric primitives. In a followup to this change, support for `STRING` order-by columns will be added. Signed-off-by: MithunR <[email protected]>
mythrocks
added
improvement
ttnghia
reviewed
Apr 8, 2023
ttnghia
reviewed
Apr 8, 2023
1. Naming conventions for function/method parameter. 2. More const for identifiers, where applicable.
|
Rerun tests |
|
@mythrocks the world has changed since our GHA migration :) See https://docs.rapids.ai/resources/github-actions/#rerunning-failed-github-actions for how to rerun tests |
ttnghia
reviewed
Apr 13, 2023
ttnghia
reviewed
Apr 13, 2023
ttnghia
approved these changes
Apr 13, 2023
|
/merge |
|
Thank you for the reviews, @ttnghia, @PointKernel. I have merged this change. |
This was referenced Apr 17, 2023
rapids-bot bot
pushed a commit
that referenced
this pull request
Apr 21, 2023
This commit adds support to use `STRING` columns as the order-by column in range-based window functions.
## Context
A range-based window function allows a variable-width window to be defined around each row of an input column. The "range" describes the range of values in an order-by column.
Consider the following input:
```c++
agg_column = { 1, 1, 1, 1, 1, 1, 1 }; // The aggregation column.
oby_column = { "0", "1", "2", "3", "4", "5", "6" }; // The order-by column.
// ^
```
If the range is defined as `[preceding=∞, following=0]`, then for the row at index `2`, the window would be rows `0-2`.
The supported range bounds for `STRING` order-by column are:
1. `UNBOUNDED` (i.e. extending to the beginning/end of the column/group)
2. `CURRENT ROW`, indicating that all equivalent values of the current row in the `oby_column`.
Since `STRING` columns cannot have "delta" values, only `UNBOUNDED` and `CURRENT ROW` based window bounds are supported, unlike for numeric types which could specify a specific scalar delta value as a range.
Depends on #13095.
Authors:
- MithunR (https://github.com/mythrocks)
- Nghia Truong (https://github.com/ttnghia)
Approvers:
- David Wendt (https://github.com/davidwendt)
- Nghia Truong (https://github.com/ttnghia)
URL: #13143
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.
Summary
This change is in preparation for supporting
STRINGorder-by columns in range window functions. It updates how order-by column values are accessed for range-based window functions. No user-facing APIs are altered.Description
In the staging step for executing window range queries, the boundaries of each row's window are calculated. This involves subtracting/adding the
preceding/followingvalues from each order-by column row, and then searching backwards/forwards for the boundary values.The staging step has been using
column_device_view.data()for accessing the order-by rows, an acceptable approach for when the order-by columns are numeric (e.g.INT32).This approach fails when the order-by column is a
STRING, because.data()is not defined for such columns. A better approach would be to use.element()to directly access the rows, because it has special handling forSTRING, among other types, while continuing to work for numeric primitives.Future
In a followup to this change, support for
STRINGorder-by columns will be added.Checklist