-
Notifications
You must be signed in to change notification settings - Fork 240
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
support GpuSortArray #2616
support GpuSortArray #2616
Conversation
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/collectionOperations.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/collectionOperations.scala
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
Outdated
Show resolved
Hide resolved
TypeSig.ARRAY.nested(TypeSig.all)), | ||
("ascendingOrder", TypeSig.BOOLEAN, TypeSig.BOOLEAN)), | ||
(in, conf, p, r) => new BinaryExprMeta[SortArray](in, conf, p, r) { | ||
override def convertToGpu(lhs: Expression, rhs: Expression): GpuExpression = { |
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.
If we don't support literal arrays we have to tag it here so we fall back to the CPU.
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 think we can support literal arrays. And I made up the corresponding test.
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.
It gets hard to test this because if both parts are literals spark will optimize it before we ever see it and replace the entire thing with a literal array that is already sorted. There is a kind of hidden config to disable this, but it would take some digging to find it.
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.
What you wrote is good enough for me. The test is likely not testing what we expect it to, but the code looks like it should work fine so I am OK with it.
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/org/apache/spark/sql/rapids/collectionOperations.scala
Outdated
Show resolved
Hide resolved
Co-authored-by: Gera Shegalov <gshegalov@nvidia.com>
Signed-off-by: sperlingxx <lovedreamf@gmail.com>
build |
Closes #2557. This PR is to support sort_array function on GPU with cuDF method ColumnVector.listSortRows. Signed-off-by: sperlingxx <lovedreamf@gmail.com> Co-authored-by: Gera Shegalov <gshegalov@nvidia.com>
Closes #2557.
This PR is to support sort_array function on GPU with cuDF method
ColumnVector.listSortRows
.