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

Jump to bottom

Plugin should throw same arithmetic exceptions as Spark part1 #5354

Merged
merged 13 commits into from
May 11, 2022
Merged

Plugin should throw same arithmetic exceptions as Spark part1 #5354

merged 13 commits into from
May 11, 2022

Conversation

HaoYang670
Copy link
Collaborator

This is the first PR for #5196.
In this PR we:

  1. move RapidsErrorUtils from 311until330-nondb to 311until320-nondb and 320until330-nondb.
  2. Add divByZeroError and divOverflowError into RapidsErrorUtils
  3. update some tests

There are other arithmetic exceptions that we need to update. I will file more follow-up PRs to fix them.

@HaoYang670
modify shim layers
3965922 
update tests

Signed-off-by: remzi <13716567376yh@gmail.com>
@HaoYang670
correct the path
442263f 
Signed-off-by: remzi <13716567376yh@gmail.com>
@jlowe jlowe added this to the Apr 18 - Apr 29 milestone Apr 28, 2022
jlowe
jlowe reviewed Apr 28, 2022
View reviewed changes
Copy link
Contributor

@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.

This needs to be updated to resolve the merge conflict but otherwise lgtm.

There are other arithmetic exceptions that we need to update. I will file more follow-up PRs to fix them.

If there are a number of followup items, it may be useful to enumerate what needs to be done in #5196 as a checklist so it's clear what has been done and what is yet to be done.

@sameerz sameerz added the task Work required that improves the product but is not user facing label Apr 28, 2022
@HaoYang670
Copy link
Collaborator Author

build

@HaoYang670
fix compile error
1bacccd 
Signed-off-by: remzi <13716567376yh@gmail.com>
@HaoYang670
Copy link
Collaborator Author

build

@HaoYang670
fix compile error
8868ac9 
Signed-off-by: remzi <13716567376yh@gmail.com>
@HaoYang670
Copy link
Collaborator Author

build

@HaoYang670 HaoYang670 mentioned this pull request Apr 29, 2022
Closed
14 tasks
@HaoYang670
Copy link
Collaborator Author

If there are a number of followup items, it may be useful to enumerate what needs to be done in #5196 as a checklist so it's clear what has been done and what is yet to be done.

Done! #5196 (comment)

HaoYang670 and others added 4 commits April 30, 2022 06:31
@HaoYang670 @jlowe
Update sql-plugin/src/main/scala/org/apache/spark/sql/rapids/arithmet…
cd7db65 
…ic.scala

Co-authored-by: Jason Lowe <jlowe@nvidia.com>
@HaoYang670 @jlowe
Update sql-plugin/src/main/scala/org/apache/spark/sql/rapids/arithmet…
9ef94d9 
…ic.scala

Co-authored-by: Jason Lowe <jlowe@nvidia.com>
@HaoYang670 @jlowe
Update sql-plugin/src/main/scala/org/apache/spark/sql/rapids/arithmet…
b30ee64 
…ic.scala

Co-authored-by: Jason Lowe <jlowe@nvidia.com>
@HaoYang670 @jlowe
Update sql-plugin/src/main/scala/org/apache/spark/sql/rapids/arithmet…
b239247 
…ic.scala

Co-authored-by: Jason Lowe <jlowe@nvidia.com>
@HaoYang670
Copy link
Collaborator Author

build

jlowe
jlowe previously approved these changes May 2, 2022
View reviewed changes
HaoYang670 added 2 commits May 3, 2022 08:54
@HaoYang670
resolve conflict
807f920 
Signed-off-by: remzi <13716567376yh@gmail.com>
@HaoYang670
Merge remote-tracking branch 'origin/throw_same_exceptions_part1' int…
84710a1 
…o throw_same_exceptions_part1
@HaoYang670
resolve conflict
a5176b4 
Signed-off-by: remzi <13716567376yh@gmail.com>
@HaoYang670
Copy link
Collaborator Author

build

@HaoYang670
update the shim layer of 321cdh
cd1520b 
Signed-off-by: remzi <13716567376yh@gmail.com>
@HaoYang670
Copy link
Collaborator Author

build

1 similar comment
@jlowe
Copy link
Contributor

jlowe commented May 4, 2022

build

@HaoYang670
Copy link
Collaborator Author

Why is there such an error /home/ubuntu/spark-rapids/sql-plugin/src/main/320until330-noncdh/scala/com/nvidia/spark/rapids/shims/OrcShims.scala:22: not found: type OrcShims320untilAllBase?
Do I modify the pom.xml in a wrong way?
https://github.com/NVIDIA/spark-rapids/runs/6269757840?check_suite_focus=true#step:2:3836

@jlowe
Copy link
Contributor

jlowe commented May 5, 2022

Why is there such an error

It's unrelated to this PR. The problem was introduced by #5346 and fixed by #5414.

@jlowe
Copy link
Contributor

jlowe commented May 5, 2022

build

@HaoYang670 HaoYang670 self-assigned this May 6, 2022
@HaoYang670
Copy link
Collaborator Author

Could we merge this PR, as the potential merge conflicts are high?

@jlowe jlowe merged commit 29fd1bf into NVIDIA:branch-22.06 May 11, 2022
@tgravescs tgravescs mentioned this pull request May 12, 2022
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jlowe jlowe jlowe approved these changes

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

@tgravescs tgravescs Awaiting requested review from tgravescs tgravescs is a code owner

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

Assignees

@HaoYang670 HaoYang670

Labels
task Work required that improves the product but is not user facing
Projects
None yet
Milestone
May 2 - May 20
Development

Successfully merging this pull request may close these issues.
3 participants
@HaoYang670 @jlowe @sameerz