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.

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

[SPARK-39889][SQL] Use different error classes for numeric/interval divided by 0 #37313

Closed
wants to merge 3 commits into from

Conversation

gengliangwang
Copy link
Member

What changes were proposed in this pull request?

Currently, when numbers are divided by 0 under ANSI mode, the error message is like

[DIVIDE_BY_ZERO] Division by zero. Use `try_divide` to tolerate divisor being 0 and return NULL instead. If necessary set "ansi_mode" to "false" (except for ANSI interval type) to bypass this error.

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: "INTERVAL_DIVIDED_BY_ZERO"

Why are the changes needed?

For better error messages

Does this PR introduce any user-facing change?

Yes, Use different error classes for numeric/interval divided by 0. After changes, the error messages are simpler and more clear.

How was this patch tested?

UT

@gengliangwang
Copy link
Member Author

cc @entong as well

@@ -210,6 +210,12 @@
"<message>"
]
},
"INTERVAL_DIVIDED_BY_ZERO" : {
"message" : [
"Division by zero."
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't try_divide help here? The TryDivide expression has some examples with intervals:

> SELECT _FUNC_(interval 2 month, 2);
0-1
> SELECT _FUNC_(interval 2 month, 0);
NULL

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense. I have updated the message. Thanks!

@MaxGekk
Copy link
Member

MaxGekk commented Jul 28, 2022

+1, LGTM. Merging to master (try to 3.3).
Thank you, @gengliangwang and @cloud-fan for review.

@MaxGekk MaxGekk closed this in 5420562 Jul 28, 2022
@MaxGekk
Copy link
Member

MaxGekk commented Jul 28, 2022

@gengliangwang It conflicts w/ 3.3. It is up to you to backport it to branch-3.3.

gengliangwang added a commit that referenced this pull request Aug 1, 2022
…rithmetic overflow

### What changes were proposed in this pull request?

Similar with #37313, currently, when  arithmetic overflow errors happen under ANSI mode, the error messages are like
```
[ARITHMETIC_OVERFLOW] long overflow. Use 'try_multiply' to tolerate overflow and return NULL instead. If necessary set spark.sql.ansi.enabled to "false"
```

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: `INTERVAL_ARITHMETIC_OVERFLOW`

### Why are the changes needed?

For better error messages

### Does this PR introduce _any_ user-facing change?

Yes, Use different error classes for arithmetic overflows of numeric/interval.. After changes, the error messages are simpler and more clear.

### How was this patch tested?

UT

Closes #37337 from gengliangwang/improveOverflowMsg.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
@gengliangwang
Copy link
Member Author

@MaxGekk Thanks, I will keep it on master branch for now.

gengliangwang added a commit to gengliangwang/spark that referenced this pull request Aug 2, 2022
…rithmetic overflow

### What changes were proposed in this pull request?

Similar with apache#37313, currently, when  arithmetic overflow errors happen under ANSI mode, the error messages are like
```
[ARITHMETIC_OVERFLOW] long overflow. Use 'try_multiply' to tolerate overflow and return NULL instead. If necessary set spark.sql.ansi.enabled to "false"
```

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: `INTERVAL_ARITHMETIC_OVERFLOW`

### Why are the changes needed?

For better error messages

### Does this PR introduce _any_ user-facing change?

Yes, Use different error classes for arithmetic overflows of numeric/interval.. After changes, the error messages are simpler and more clear.

### How was this patch tested?

UT

Closes apache#37337 from gengliangwang/improveOverflowMsg.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
gengliangwang added a commit that referenced this pull request Aug 2, 2022
…rithmetic overflow

### What changes were proposed in this pull request?

Similar with #37313, currently, when  arithmetic overflow errors happen under ANSI mode, the error messages are like
```
[ARITHMETIC_OVERFLOW] long overflow. Use 'try_multiply' to tolerate overflow and return NULL instead. If necessary set spark.sql.ansi.enabled to "false"
```

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: `INTERVAL_ARITHMETIC_OVERFLOW`

### Why are the changes needed?

For better error messages

### Does this PR introduce _any_ user-facing change?

Yes, Use different error classes for arithmetic overflows of numeric/interval.. After changes, the error messages are simpler and more clear.

### How was this patch tested?

UT

Closes #37374 from gengliangwang/SPARK-39917.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants