-
Notifications
You must be signed in to change notification settings - Fork 245
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Perform explicit UnsafeRow projection in ColumnarToRow transition #5274
Conversation
Signed-off-by: Jason Lowe <jlowe@nvidia.com>
Keeping this as a draft until this gets some at-scale performance tests |
build |
Premerge failed due to transitive Cloudera dependency, rekicking the build. |
build |
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.
Minor nits
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuRowToColumnarExec.scala
Show resolved
Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuRowToColumnarExec.scala
Show resolved
Hide resolved
build |
The latest code changes do not appear to have a significant performance impact. Thanks to @mattahrens for running some performance tests. This should now be ready for review. |
build |
@revans2 @tgravescs if one of you could take a look that would be great, as it needs a codeowner approval as well. |
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.
looking good, just minor comments.
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.
Just a minor nit on top of what @abellina found
sql-plugin/src/main/java/com/nvidia/spark/rapids/UnsafeRowToColumnarBatchIterator.java
Show resolved
Hide resolved
build |
Fixes #5189.
The row to columnar transition was assuming that every row coming in was an
UnsafeRow
but that is not always the case. This adds an explicit unsafe projection to the generated unsafe row iterator to handle those cases.