[SPARK-35162][SQL] New SQL functions: TRY_ADD/TRY_DIVIDE #32292

gengliangwang · 2021-04-22T09:15:45Z

What changes were proposed in this pull request?

Add New SQL functions:

TRY_ADD
TRY_DIVIDE

These expressions are identical to the following expression under ANSI mode except that it returns null if error occurs:

ADD
DIVIDE

Note: it is easy to add other expressions like TRY_SUBTRACT/TRY_MULTIPLY but let's control the number of these new expressions and just add TRY_ADD and TRY_DIVIDE for now.

Why are the changes needed?

Users can manage to finish queries without interruptions in ANSI mode.
Users can get NULLs instead of unreasonable results if overflow occurs when ANSI mode is off.
For example, the behavior of the following SQL operations is unreasonable:

2147483647 + 2 => -2147483647

With the new safe version SQL functions:

TRY_ADD(2147483647, 2) => null

Note: We should only add new expressions to important operators, instead of adding new safe expressions for all the expressions that can throw errors.

Does this PR introduce any user-facing change?

Yes, new SQL functions: TRY_ADD/TRY_DIVIDE

How was this patch tested?

Unit test

gengliangwang · 2021-04-22T09:16:33Z

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

I will fix the sql/toString method. This is still WIP.

SparkQA · 2021-04-22T09:25:55Z

Test build #137800 has finished for PR 32292 at commit eb5a07d.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
s\"$
case class Try(child: Expression) extends UnaryExpression

SparkQA · 2021-04-22T10:21:20Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42329/

SparkQA · 2021-04-22T16:06:50Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42343/

SparkQA · 2021-04-22T16:06:51Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42343/

SparkQA · 2021-04-22T17:28:25Z

Test build #137813 has finished for PR 32292 at commit 153f147.

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

SparkQA · 2021-05-11T18:37:46Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42914/

SparkQA · 2021-05-11T18:37:48Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42914/

SparkQA · 2021-05-11T19:06:30Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42915/

SparkQA · 2021-05-11T19:06:31Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42915/

SparkQA · 2021-05-11T20:48:01Z

Test build #138391 has finished for PR 32292 at commit a1d5742.

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

SparkQA · 2021-05-11T21:55:01Z

Test build #138392 has finished for PR 32292 at commit 75dee4a.

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

SparkQA · 2021-05-12T13:23:04Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42974/

SparkQA · 2021-05-12T13:23:05Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42974/

gengliangwang · 2021-05-12T14:06:02Z

@cloud-fan @maropu Sorry for the delay. This one is ready for review now.

SparkQA · 2021-05-12T14:47:05Z

Test build #138453 has finished for PR 32292 at commit 9b671f1.

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

SparkQA · 2021-05-12T15:04:19Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42976/

SparkQA · 2021-05-12T18:24:09Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42989/

SparkQA · 2021-05-12T18:27:26Z

Test build #138455 has finished for PR 32292 at commit d1c4e92.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class TryAdd(left: Expression, right: Expression, child: Expression)
case class TryDivide(left: Expression, right: Expression, child: Expression)

SparkQA · 2021-05-12T18:30:25Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42989/

SparkQA · 2021-05-12T21:59:45Z

Test build #138468 has finished for PR 32292 at commit 60b4514.

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

maropu

Just out of curiosity; Any reason to pick up try_add+try_divide instead of try_add+try_multiple?

maropu · 2021-05-13T02:24:01Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

    expression[CaseWhen]("when"),

+    expression[TryAdd]("try_add"),
+    expression[TryDivide]("try_divide"),


If these functions exist in the math function group, how about moving them after L376?

maropu · 2021-05-13T02:25:17Z

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

+import org.apache.spark.sql.catalyst.expressions.codegen.Block._
+import org.apache.spark.sql.types.DataType
+
+private[catalyst] case class TryEval(child: Expression)


we need private[catalyst] here? I think catalyst package itself looks private.

maropu · 2021-05-13T02:26:42Z

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

+  examples = """
+    Examples:
+      > SELECT _FUNC_(1, 2);
+       3


How about adding an overflow example here?

maropu · 2021-05-13T02:27:20Z

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

+      > SELECT _FUNC_(3, 2);
+       1.5
+      > SELECT _FUNC_(2L, 2L);
+       1.0


How about adding a null example here?

gengliangwang · 2021-05-13T04:13:12Z

Just out of curiosity; Any reason to pick up try_add+try_divide instead of try_add+try_multiple?

IMO, divide by 0 error is more common in ETL/ML jobs than integral multiply overflow. I also talked to @bart-samwel, from his experience on Bigquery, the most desired functions are try_cast and try_divide.
We can add TRY_SUBTRACT/TRY_MULTIPLY if many users want it. Before that, let's be cautious in adding such new functions.

maropu

Thank you for the explanation, @gengliangwang . Looks fine to me.

SparkQA · 2021-05-13T05:11:14Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43010/

SparkQA · 2021-05-13T05:11:15Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43010/

SparkQA · 2021-05-13T06:37:43Z

Test build #138490 has finished for PR 32292 at commit 774bda1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class TryEval(child: Expression) extends UnaryExpression with NullIntolerant

SparkQA · 2021-05-13T08:25:27Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43022/

SparkQA · 2021-05-13T08:25:28Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43022/

SparkQA · 2021-05-13T12:05:37Z

Test build #138502 has finished for PR 32292 at commit 91bdc4c.

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

gengliangwang · 2021-05-13T14:25:32Z

Merging to master. @maropu Thanks for the review.

…e as well ### What changes were proposed in this pull request? In #32292, `try_arithmetic.sql` is only tested with ANSI mode off. We should test it under ANSI mode as well. ### Why are the changes needed? Improve test coverage ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Unit test Closes #34795 from gengliangwang/try_arithmetic.sql. Authored-by: Gengliang Wang <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>

gengliangwang commented Apr 22, 2021

View reviewed changes

gengliangwang requested review from cloud-fan and hvanhovell April 22, 2021 09:16

github-actions bot added the SQL label Apr 22, 2021

gengliangwang force-pushed the try_add branch from eb5a07d to 153f147 Compare April 22, 2021 14:29

gengliangwang changed the title ~~[WIP][SPARK-35162][SQL] New SQL functions: TRY_ADD/TRY_SUBTRACT/TRY_MULTIPLY/TRY_DIVIDE/TRY_DIV~~ [WIP][SPARK-35162][SQL] New SQL functions: TRY_ADD/TRY_DIVIDE Apr 23, 2021

gengliangwang added 4 commits May 11, 2021 22:08

add try_add/try_subtract/try_multiply/try_divide/try_div

7f86af7

rename

aca8975

add test

c0ea9bd

remove expressions

a1d5742

gengliangwang force-pushed the try_add branch from 153f147 to a1d5742 Compare May 11, 2021 17:32

update

75dee4a

gengliangwang added 2 commits May 12, 2021 20:15

refactor

4eed565

remove code

483affb

RuntimeReplaceable

d1c4e92

gengliangwang force-pushed the try_add branch from 9b671f1 to d1c4e92 Compare May 12, 2021 13:54

gengliangwang changed the title ~~[WIP][SPARK-35162][SQL] New SQL functions: TRY_ADD/TRY_DIVIDE~~ [SPARK-35162][SQL] New SQL functions: TRY_ADD/TRY_DIVIDE May 12, 2021

update sql-expression-schema.md

60b4514

maropu reviewed May 13, 2021

View reviewed changes

address comments

774bda1

maropu approved these changes May 13, 2021

View reviewed changes

try_add

91bdc4c

gengliangwang closed this in 02c99f1 May 13, 2021

gengliangwang mentioned this pull request Dec 3, 2021

[SPARK-35162][TESTS][FOLLOWUP] Test try_arithmetic.sql under ANSI mode as well #34795

Closed

[SPARK-35162][SQL] New SQL functions: TRY_ADD/TRY_DIVIDE #32292

[SPARK-35162][SQL] New SQL functions: TRY_ADD/TRY_DIVIDE #32292

Uh oh!

Conversation

gengliangwang commented Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 22, 2021

Uh oh!

SparkQA commented Apr 22, 2021

Uh oh!

SparkQA commented Apr 22, 2021

Uh oh!

SparkQA commented Apr 22, 2021

Uh oh!

SparkQA commented Apr 22, 2021

Uh oh!

SparkQA commented May 11, 2021

Uh oh!

SparkQA commented May 11, 2021

Uh oh!

SparkQA commented May 11, 2021

Uh oh!

SparkQA commented May 11, 2021

Uh oh!

SparkQA commented May 11, 2021

Uh oh!

SparkQA commented May 11, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

gengliangwang commented May 12, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

SparkQA commented May 12, 2021

Uh oh!

maropu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gengliangwang commented May 13, 2021

Uh oh!

maropu left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 13, 2021

Uh oh!

SparkQA commented May 13, 2021

gengliangwang commented Apr 22, 2021 •

edited

Loading