Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update GpuIf to support expressions with side effects #4358

Merged
merged 15 commits into from
Dec 17, 2021
Merged

Update GpuIf to support expressions with side effects #4358

merged 15 commits into from
Dec 17, 2021

Conversation

andygrove
Copy link
Contributor

@andygrove andygrove commented Dec 14, 2021 •
edited
Loading

Signed-off-by: Andy Grove andygrove@nvidia.com

This partially addresses #3849.

There will be a follow-on PR to apply this to GpuCaseWhen since this is much more involved.

@andygrove
Update GpuIf to support expressions with side effects
2baf9eb 
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove
prep for review
b236f39 
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove andygrove added this to the Dec 13 - Jan 7 milestone Dec 14, 2021
@andygrove andygrove self-assigned this Dec 14, 2021
@andygrove
improve checks for all true/false
c1943f0 
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove
Copy link
Contributor Author

build

andygrove and others added 2 commits December 15, 2021 08:23
@andygrove @firestarman
Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalE…
96567bb 
…xpressions.scala

Co-authored-by: Liangcai Li <firestarmanllc@gmail.com>
@andygrove @firestarman
Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalE…
0c706f4 
…xpressions.scala

Co-authored-by: Liangcai Li <firestarmanllc@gmail.com>
@andygrove
Copy link
Contributor Author

build

@andygrove
Copy link
Contributor Author

@firestarman Thanks for the suggestions. That resolved the test failures I was seeing.

@andygrove
fix resource leak and add test for CAST
ee83339 
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove andygrove changed the title WIP: Update GpuIf to support expressions with side effects Update GpuIf to support expressions with side effects Dec 15, 2021
@andygrove andygrove marked this pull request as ready for review December 15, 2021 18:03
@andygrove andygrove requested a review from jlowe December 15, 2021 18:06
@andygrove
Copy link
Contributor Author

build

jlowe
jlowe reviewed Dec 15, 2021
View reviewed changes
sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuUserDefinedFunction.scala Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Outdated Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Outdated Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Outdated Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Show resolved Hide resolved
sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Outdated Show resolved Hide resolved
andygrove and others added 3 commits December 15, 2021 17:15
@andygrove @jlowe
Update sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalE…
54488eb 
…xpressions.scala

Co-authored-by: Jason Lowe <jlowe@nvidia.com>
@andygrove
partially address PR review feedback
7b87af6 
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove
Merge branch 'gpu-if-side-effect' of github.com:andygrove/spark-rapid…
d6d724f 
…s into gpu-if-side-effect
jlowe
jlowe previously approved these changes Dec 16, 2021
View reviewed changes
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit, lgtm.

sql-plugin/src/main/scala/com/nvidia/spark/rapids/conditionalExpressions.scala Outdated Show resolved Hide resolved
@andygrove
Change gather signature to return ColumnVector. Also add missing import.
bee0643 
Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove
Copy link
Contributor Author

build

@jlowe jlowe mentioned this pull request Dec 17, 2021
Closed
@andygrove andygrove merged commit c07e1e2 into NVIDIA:branch-22.02 Dec 17, 2021
@andygrove andygrove deleted the gpu-if-side-effect branch December 17, 2021 22:24
@NvTimLiu NvTimLiu mentioned this pull request Dec 20, 2021
Closed
@andygrove andygrove mentioned this pull request Dec 21, 2021
Merged
@sameerz sameerz added the bug Something isn't working label Dec 29, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@firestarman firestarman firestarman left review comments

@jlowe jlowe jlowe approved these changes

Assignees

@andygrove andygrove

Labels
bug Something isn't working
Projects
None yet
Milestone
Dec 13 - Jan 7
Development

Successfully merging this pull request may close these issues.
4 participants
@andygrove @jlowe @firestarman @sameerz