[SPARK-4593] [SQL] return null when divider is 0 of Double type #3443

adrian-wang · 2014-11-25T06:57:33Z

SELECT max(1/0) FROM src
would return a very large number, which is obviously not right.
For hive-0.12, hive would return Infinity for 1/0, while for hive-0.13.1, it is NULL for 1/0.
I think it is better to keep our behavior with newer Hive version.
This PR ensures that when the divider is 0, the result of expression should be NULL, same with hive-0.13.1

SparkQA · 2014-11-25T07:05:09Z

Test build #23821 has started for PR 3443 at commit 2dfe50f.

This patch merges cleanly.

SparkQA · 2014-11-25T08:12:53Z

Test build #23821 has finished for PR 3443 at commit 2dfe50f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-25T08:12:56Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23821/
Test PASSed.

ueshin · 2014-11-25T16:29:41Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala

I think expression specific behavior should not be in f2 method, which is a general purpose method for binary expression of fractional type.

ueshin · 2014-11-25T16:42:10Z

Divide.nullable should be true if it returns null when the divider is 0.

ueshin · 2014-11-25T16:46:15Z

What do you think about IntegralType?
Currently I think it throws ArithmeticException when the divider is 0.

adrian-wang · 2014-11-26T02:40:53Z

@ueshin Thanks for your reminding, I made a mistake here.

SparkQA · 2014-11-26T02:42:33Z

Test build #23865 has started for PR 3443 at commit cf28c58.

This patch merges cleanly.

SparkQA · 2014-11-26T02:43:44Z

Test build #23865 has finished for PR 3443 at commit cf28c58.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-26T02:43:45Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23865/
Test FAILed.

SparkQA · 2014-11-26T03:00:14Z

Test build #23867 has started for PR 3443 at commit 22ecd9a.

This patch merges cleanly.

SparkQA · 2014-11-26T04:08:26Z

Test build #23867 has finished for PR 3443 at commit 22ecd9a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-26T04:08:29Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23867/
Test PASSed.

ueshin · 2014-11-27T02:42:27Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala

I'm wondering we should check equality between the evaluated right value and zero here because evaluation right.eval(input) happens twice, here and in f2 or i2.
Should be at around _.div(_, _) or _.quot(_, _)?

SparkQA · 2014-11-27T09:00:05Z

Test build #23920 has started for PR 3443 at commit cee92bd.

This patch merges cleanly.

SparkQA · 2014-11-27T10:04:47Z

Test build #23920 has finished for PR 3443 at commit cee92bd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-27T10:04:50Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23920/
Test PASSed.

SparkQA · 2014-11-27T10:40:15Z

Test build #23922 has started for PR 3443 at commit 6f5716f.

This patch merges cleanly.

SparkQA · 2014-11-27T11:50:30Z

Test build #23922 has finished for PR 3443 at commit 6f5716f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-27T11:50:33Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23922/
Test PASSed.

marmbrus · 2014-12-01T21:21:52Z

Can you add some test cases to the ExpressionEvaluationSuite?

SparkQA · 2014-12-02T03:00:08Z

Test build #24018 has started for PR 3443 at commit 36236a5.

This patch merges cleanly.

SparkQA · 2014-12-02T03:10:03Z

Test build #24018 has finished for PR 3443 at commit 36236a5.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-12-02T03:10:05Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24018/
Test FAILed.

SparkQA · 2014-12-02T06:55:24Z

Test build #24030 has started for PR 3443 at commit 2e98677.

This patch merges cleanly.

SparkQA · 2014-12-02T07:58:59Z

Test build #24030 has finished for PR 3443 at commit 2e98677.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-12-02T07:59:02Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24030/
Test PASSed.

SELECT max(1/0) FROM src would return a very large number, which is obviously not right. For hive-0.12, hive would return `Infinity` for 1/0, while for hive-0.13.1, it is `NULL` for 1/0. I think it is better to keep our behavior with newer Hive version. This PR ensures that when the divider is 0, the result of expression should be NULL, same with hive-0.13.1 Author: Daoyuan Wang <[email protected]> Closes #3443 from adrian-wang/div and squashes the following commits: 2e98677 [Daoyuan Wang] fix code gen for divide 0 85c28ba [Daoyuan Wang] temp 36236a5 [Daoyuan Wang] add test cases 6f5716f [Daoyuan Wang] fix comments cee92bd [Daoyuan Wang] avoid evaluation 2 times 22ecd9a [Daoyuan Wang] fix style cf28c58 [Daoyuan Wang] divide fix 2dfe50f [Daoyuan Wang] return null when divider is 0 of Double type (cherry picked from commit f6df609) Signed-off-by: Michael Armbrust <[email protected]>

marmbrus · 2014-12-02T22:23:26Z

Thanks for fixing this! I've merged to master and 1.2.

This is a follow-up of SPARK-4593 (#3443). Author: Takuya UESHIN <[email protected]> Closes #3581 from ueshin/issues/SPARK-4720 and squashes the following commits: c3959d4 [Takuya UESHIN] Make Remainder return null if the divider is 0.

return null when divider is 0 of Double type

2dfe50f

ueshin reviewed Nov 25, 2014
View reviewed changes

divide fix

cf28c58

fix style

22ecd9a

ueshin reviewed Nov 27, 2014
View reviewed changes

avoid evaluation 2 times

cee92bd

fix comments

6f5716f

add test cases

36236a5

adrian-wang added 2 commits December 1, 2014 21:32

temp

85c28ba

fix code gen for divide 0

2e98677

asfgit closed this in f6df609 Dec 2, 2014

ueshin mentioned this pull request Dec 3, 2014

[SPARK-4720][SQL] Remainder should also return null if the divider is 0. #3581

Closed

[SPARK-4593] [SQL] return null when divider is 0 of Double type #3443

[SPARK-4593] [SQL] return null when divider is 0 of Double type #3443

Uh oh!

Conversation

adrian-wang commented Nov 25, 2014

Uh oh!

SparkQA commented Nov 25, 2014

Uh oh!

SparkQA commented Nov 25, 2014

Uh oh!

AmplabJenkins commented Nov 25, 2014

Uh oh!

ueshin Nov 25, 2014

Choose a reason for hiding this comment

Uh oh!

ueshin commented Nov 25, 2014

Uh oh!

ueshin commented Nov 25, 2014

Uh oh!

adrian-wang commented Nov 26, 2014

Uh oh!

SparkQA commented Nov 26, 2014

Uh oh!

SparkQA commented Nov 26, 2014

Uh oh!

AmplabJenkins commented Nov 26, 2014

Uh oh!

SparkQA commented Nov 26, 2014

Uh oh!

SparkQA commented Nov 26, 2014

Uh oh!

AmplabJenkins commented Nov 26, 2014

Uh oh!

ueshin Nov 27, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 27, 2014

Uh oh!

SparkQA commented Nov 27, 2014

Uh oh!

AmplabJenkins commented Nov 27, 2014

Uh oh!

SparkQA commented Nov 27, 2014

Uh oh!

SparkQA commented Nov 27, 2014

Uh oh!

AmplabJenkins commented Nov 27, 2014

Uh oh!

marmbrus commented Dec 1, 2014

Uh oh!

SparkQA commented Dec 2, 2014

Uh oh!

SparkQA commented Dec 2, 2014

Uh oh!

AmplabJenkins commented Dec 2, 2014

Uh oh!

SparkQA commented Dec 2, 2014

Uh oh!

SparkQA commented Dec 2, 2014

Uh oh!

AmplabJenkins commented Dec 2, 2014

Uh oh!

marmbrus commented Dec 2, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants