Support executing collect_list on GPU with windowing. #1548
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
firestarman
force-pushed
the
collect_list
branch
from
2d07f99
to
9791cfe
Compare
firestarman
changed the title
|
Mark as draft since it depends on the support from cuDF native. |
mythrocks
reviewed
Jan 20, 2021
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/AggregateFunctions.scala
Outdated
Show resolved
Hide resolved
12 tasks
firestarman
force-pushed
the
collect_list
branch
2 times, most recently
from
aee4fc6
to
86b80e3
Compare
|
rebased |
|
build |
1 similar comment
|
build |
firestarman
changed the title
Signed-off-by: Firestarman <[email protected]>
Let collect_list with Windowing supports type of array of struct. Signed-off-by: Firestarman <[email protected]>
Signed-off-by: Firestarman <[email protected]>
Signed-off-by: Firestarman <[email protected]>
Signed-off-by: Firestarman <[email protected]>
Add integration tests for collect_list with windowing. Signed-off-by: Firestarman <[email protected]>
firestarman
force-pushed
the
collect_list
branch
from
071910e
to
bfef795
Compare
|
rebased |
Signed-off-by: Firestarman <[email protected]>
revans2
requested changes
Jan 28, 2021
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Firestarman <[email protected]>
Signed-off-by: Firestarman <[email protected]>
|
build |
revans2
reviewed
Jan 29, 2021
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuWindowExpression.scala
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/TypeChecks.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/AggregateFunctions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/AggregateFunctions.scala
Outdated
Show resolved
Hide resolved
revans2
reviewed
Jan 29, 2021
Signed-off-by: Firestarman <[email protected]>
|
cudf nightly with PR rapidsai/cudf#7189 always fail, so the pre-merge check will fail too. |
Signed-off-by: Firestarman <[email protected]>
The fix is in review rapidsai/cudf#7266 |
revans2
reviewed
Feb 4, 2021
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
| TypeSig.ARRAY.nested(TypeSig.commonCudfTypes + TypeSig.DECIMAL + TypeSig.STRUCT), | ||
| TypeSig.ARRAY.nested(TypeSig.all), | ||
| Seq(ParamCheck("input", | ||
| (TypeSig.commonCudfTypes + TypeSig.DECIMAL).nested() + TypeSig.STRUCT, |
There was a problem hiding this comment.
again I think longer is clearer, but in this case because there is only one nested type I think it is okay. So we don't support doing a collect list on a struct that also contains a struct?
There was a problem hiding this comment.
This should be supported. If it's not working, it might be a bug in libcudf. I'd be happy to investigate.
There was a problem hiding this comment.
Updated it, and the struct of struct is not covered in integration tests, so do not add it here. Shall we need it ?
There was a problem hiding this comment.
if yes, i will add it in a following PR
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/AggregateFunctions.scala
Show resolved
Hide resolved
|
cudf side is ready now |
Signed-off-by: Firestarman <[email protected]>
|
build |
revans2
approved these changes
Feb 5, 2021
There was a problem hiding this comment.
@mythrocks are you OK with a follow on PR to add in struct or struct support?
Sounds good. |
nartal1
pushed a commit
to nartal1/spark-rapids
that referenced
this pull request
Jun 9, 2021
Signed-off-by: Firestarman <[email protected]>
nartal1
pushed a commit
to nartal1/spark-rapids
that referenced
this pull request
Jun 9, 2021
Signed-off-by: Firestarman <[email protected]>
tgravescs
pushed a commit
to tgravescs/spark-rapids
that referenced
this pull request
Nov 30, 2023
…IDIA#1548) Signed-off-by: spark-rapids automation <[email protected]>
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.
This PR is to support running
collect_liston GPU with windowing.This PR depends on the PR rapidsai/cudf#7189 .
Signed-off-by: Firestarman [email protected]