You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mgaido91 <gi...@git.apache.org> on 2018/01/22 14:34:52 UTC
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
GitHub user mgaido91 opened a pull request:
https://github.com/apache/spark/pull/20350
[SPARK-23179][SQL] Support option to throw exception if overflow occurs
## What changes were proposed in this pull request?
SQL ANSI 2011 states that in case of overflow during arithmetic operations, an exception should be thrown. This is what most of the SQL DBs do (eg. SQLServer, DB2). Hive currently returns NULL (as Spark does) but HIVE-18291 is open to be SQL compliant.
The PR introduce an option to decide which behavior Spark should follow, ie. returning NULL on overflow or throwing an exception.
## How was this patch tested?
added UTs
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mgaido91/spark SPARK-23179
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20350.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20350
----
commit 449b69c0804622cc747d280d28415e93144ca272
Author: Marco Gaido <ma...@...>
Date: 2018-01-22T14:32:04Z
[SPARK-23179][SQL] Support option to throw exception if overflow occurs
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2537/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86524/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #87841 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87841/testReport)** for PR 20350 at commit [`bd8b645`](https://github.com/apache/spark/commit/bd8b645b09ee81a524da462225f950ec514bd350).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #92087 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92087/testReport)** for PR 20350 at commit [`069b861`](https://github.com/apache/spark/commit/069b86163e03739ef5fa6925c8933521125a1ff0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86484 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86484/testReport)** for PR 20350 at commit [`449b69c`](https://github.com/apache/spark/commit/449b69c0804622cc747d280d28415e93144ca272).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class CheckOverflow(`
* `final class Decimal extends Ordered[Decimal] with Serializable with Logging `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87841/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/985/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #87794 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87794/testReport)** for PR 20350 at commit [`bd8b645`](https://github.com/apache/spark/commit/bd8b645b09ee81a524da462225f950ec514bd350).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86533 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86533/testReport)** for PR 20350 at commit [`2c8e2c7`](https://github.com/apache/spark/commit/2c8e2c75c4ef99155f205b71b60b92a8521318d1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #89642 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89642/testReport)** for PR 20350 at commit [`d6bc7e9`](https://github.com/apache/spark/commit/d6bc7e9bd4176c229df09b37121388462a750a97).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #89673 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89673/testReport)** for PR 20350 at commit [`aa84034`](https://github.com/apache/spark/commit/aa84034bd60413057738500564a9714dfa4b4192).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #93072 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93072/testReport)** for PR 20350 at commit [`069b861`](https://github.com/apache/spark/commit/069b86163e03739ef5fa6925c8933521125a1ff0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
the error is unrelated, and I am seeing it frequently throughout the code. It seems something caused the flakiness to increase for this test. There is already a ticket for it: SPARK-23369, but it is vecoming more and more important to fix it. It would be great also to check what increased the flakiness...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93072/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by bersprockets <gi...@git.apache.org>.
Github user bersprockets commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163269198
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -1074,6 +1074,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val DECIMAL_OPERATIONS_NULL_ON_OVERFLOW =
+ buildConf("spark.sql.decimalOperations.nullOnOverflow")
+ .internal()
+ .doc("When true (default), if an overflow on a decimal occurs, then NULL is returned. " +
+ "Spark's older versions and Hive behave in this way. If turned to false, SQL ANSI 2011 " +
+ "specification, will be followed instead: an arithmetic exception is thrown. This is " +
+ "what most of the SQL databases do.")
--- End diff --
Tiny nit:
If turned to false, SQL ANSI 2011 specification, will be followed instead
This should be
If turned to false, SQL ANSI 2011 specification will be followed instead
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163008946
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
@@ -237,14 +238,26 @@ final class Decimal extends Ordered[Decimal] with Serializable {
/**
* Create new `Decimal` with given precision and scale.
*
- * @return a non-null `Decimal` value if successful or `null` if overflow would occur.
+ * @return a non-null `Decimal` value if successful. Otherwise, if `nullOnOverflow` is true, null
+ * is returned; if `nullOnOverflow` is false, an `ArithmeticException` is thrown.
*/
private[sql] def toPrecision(
precision: Int,
scale: Int,
- roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = {
+ roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP,
+ nullOnOverflow: Boolean = true): Decimal = {
val copy = clone()
- if (copy.changePrecision(precision, scale, roundMode)) copy else null
+ if (copy.changePrecision(precision, scale, roundMode)) {
+ copy
+ } else {
+ val message = s"$toDebugString cannot be represented as Decimal($precision, $scale)."
+ if (nullOnOverflow) {
+ logWarning(s"$message NULL is returned.")
--- End diff --
I see your point. And I agree with you. But I wanted to put some traces of what was happening What about using DEBUG as log level? In this case most of the time we are not logging anything, but if we want to check is an overflow is happening we can. What do you think?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20350
Thanks for your contributions! Could you ping us again after 2.3 release?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86484 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86484/testReport)** for PR 20350 at commit [`449b69c`](https://github.com/apache/spark/commit/449b69c0804622cc747d280d28415e93144ca272).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/103/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/110/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/136/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/318/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #93072 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93072/testReport)** for PR 20350 at commit [`069b861`](https://github.com/apache/spark/commit/069b86163e03739ef5fa6925c8933521125a1ff0).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86533 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86533/testReport)** for PR 20350 at commit [`2c8e2c7`](https://github.com/apache/spark/commit/2c8e2c75c4ef99155f205b71b60b92a8521318d1).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163184915
--- Diff: sql/core/src/test/resources/sql-tests/inputs/decimalArithmeticOperations.sql ---
@@ -49,7 +49,6 @@ select 1e35 / 0.1;
-- arithmetic operations causing a precision loss are truncated
select 123456789123456789.1234567890 * 1.123456789123456789;
-select 0.001 / 9876543210987654321098765432109876543.2
--- End diff --
yes, unfortunately I missed it somehow previously...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/1195/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
any more comments @cloud-fan @gatorsmile @hvanhovell?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #89673 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89673/testReport)** for PR 20350 at commit [`aa84034`](https://github.com/apache/spark/commit/aa84034bd60413057738500564a9714dfa4b4192).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89642/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86488 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86488/testReport)** for PR 20350 at commit [`fcd665e`](https://github.com/apache/spark/commit/fcd665e4a23b21b4ff6e112687ef802ccbc23d0f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
Jenkins, retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/661/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20350
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86528 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86528/testReport)** for PR 20350 at commit [`c73471d`](https://github.com/apache/spark/commit/c73471d167c502a8d5a0eae93ec2afa23282879a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87154/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/144/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/107/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #87841 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87841/testReport)** for PR 20350 at commit [`bd8b645`](https://github.com/apache/spark/commit/bd8b645b09ee81a524da462225f950ec514bd350).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r182820983
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/decimalExpressions.scala ---
@@ -80,30 +80,44 @@ case class PromotePrecision(child: Expression) extends UnaryExpression {
/**
* Rounds the decimal to given scale and check whether the decimal can fit in provided precision
- * or not, returns null if not.
+ * or not. If not, if `nullOnOverflow` is `true`, it returns `null`; otherwise an
+ * `ArithmeticException` is thrown.
*/
-case class CheckOverflow(child: Expression, dataType: DecimalType) extends UnaryExpression {
+case class CheckOverflow(
+ child: Expression,
+ dataType: DecimalType,
+ nullOnOverflow: Boolean) extends UnaryExpression {
override def nullable: Boolean = true
override def nullSafeEval(input: Any): Any =
- input.asInstanceOf[Decimal].toPrecision(dataType.precision, dataType.scale)
+ input.asInstanceOf[Decimal].toPrecision(
+ dataType.precision,
+ dataType.scale,
+ Decimal.ROUND_HALF_UP,
+ nullOnOverflow)
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
nullSafeCodeGen(ctx, ev, eval => {
val tmp = ctx.freshName("tmp")
+ val onOverflow = if (nullOnOverflow) {
--- End diff --
Why are you not just calling `Decimal.toPrecision` here? There seems to be very little value in code generating this (no specialization).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20350
Sure, will do the review in the next few days.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92087/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/4212/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
thanks for you comment @hvanhovell. Do you have other ones?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20350
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93401/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
@cloud-fan @gatorsmile @hvanhovell this has been stuck for a while now, do you happen to have time to ewview it again in the next days? Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #92087 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92087/testReport)** for PR 20350 at commit [`069b861`](https://github.com/apache/spark/commit/069b86163e03739ef5fa6925c8933521125a1ff0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #87154 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87154/testReport)** for PR 20350 at commit [`bd8b645`](https://github.com/apache/spark/commit/bd8b645b09ee81a524da462225f950ec514bd350).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r162959471
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
@@ -237,14 +238,26 @@ final class Decimal extends Ordered[Decimal] with Serializable {
/**
* Create new `Decimal` with given precision and scale.
*
- * @return a non-null `Decimal` value if successful or `null` if overflow would occur.
+ * @return a non-null `Decimal` value if successful. Otherwise, if `nullOnOverflow` is true, null
+ * is returned; if `nullOnOverflow` is false, an `ArithmeticException` is thrown.
*/
private[sql] def toPrecision(
precision: Int,
scale: Int,
- roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = {
+ roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP,
+ nullOnOverflow: Boolean = true): Decimal = {
val copy = clone()
- if (copy.changePrecision(precision, scale, roundMode)) copy else null
+ if (copy.changePrecision(precision, scale, roundMode)) {
+ copy
+ } else {
+ val message = s"$toDebugString cannot be represented as Decimal($precision, $scale)."
+ if (nullOnOverflow) {
+ logWarning(s"$message NULL is returned.")
--- End diff --
If we hit it often, the result we get is quite useless. I added it only to notify the user of something which is an unexpected/undesired situation and now happens silently. I think it is bad that the user cannot know if a NULL is a result of an operation involving NULLs or the result of an overflow.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86484/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/140/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86524 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86524/testReport)** for PR 20350 at commit [`610a595`](https://github.com/apache/spark/commit/610a595bf61721c38edfaf29dcc161e363319423).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163134873
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
@@ -237,14 +238,26 @@ final class Decimal extends Ordered[Decimal] with Serializable {
/**
* Create new `Decimal` with given precision and scale.
*
- * @return a non-null `Decimal` value if successful or `null` if overflow would occur.
+ * @return a non-null `Decimal` value if successful. Otherwise, if `nullOnOverflow` is true, null
+ * is returned; if `nullOnOverflow` is false, an `ArithmeticException` is thrown.
*/
private[sql] def toPrecision(
precision: Int,
scale: Int,
- roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = {
+ roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP,
+ nullOnOverflow: Boolean = true): Decimal = {
val copy = clone()
- if (copy.changePrecision(precision, scale, roundMode)) copy else null
+ if (copy.changePrecision(precision, scale, roundMode)) {
+ copy
+ } else {
+ def message = s"$toDebugString cannot be represented as Decimal($precision, $scale)."
+ if (nullOnOverflow) {
+ if (log.isDebugEnabled) logDebug(s"$message NULL is returned.")
+ null
--- End diff --
Do we really need to log for this?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
cc @gatorsmile @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86495/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163134537
--- Diff: sql/core/src/test/resources/sql-tests/inputs/decimalArithmeticOperations.sql ---
@@ -49,7 +49,6 @@ select 1e35 / 0.1;
-- arithmetic operations causing a precision loss are truncated
select 123456789123456789.1234567890 * 1.123456789123456789;
-select 0.001 / 9876543210987654321098765432109876543.2
--- End diff --
I think it is missing a `;` before...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
kindly ping @gatorsmile @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86495 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86495/testReport)** for PR 20350 at commit [`fcd665e`](https://github.com/apache/spark/commit/fcd665e4a23b21b4ff6e112687ef802ccbc23d0f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86495 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86495/testReport)** for PR 20350 at commit [`fcd665e`](https://github.com/apache/spark/commit/fcd665e4a23b21b4ff6e112687ef802ccbc23d0f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
sure, thanks @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
kindly ping @gatorsmile @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86528 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86528/testReport)** for PR 20350 at commit [`c73471d`](https://github.com/apache/spark/commit/c73471d167c502a8d5a0eae93ec2afa23282879a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `final class Decimal extends Ordered[Decimal] with Serializable `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89673/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86533/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163007885
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
@@ -237,14 +238,26 @@ final class Decimal extends Ordered[Decimal] with Serializable {
/**
* Create new `Decimal` with given precision and scale.
*
- * @return a non-null `Decimal` value if successful or `null` if overflow would occur.
+ * @return a non-null `Decimal` value if successful. Otherwise, if `nullOnOverflow` is true, null
+ * is returned; if `nullOnOverflow` is false, an `ArithmeticException` is thrown.
*/
private[sql] def toPrecision(
precision: Int,
scale: Int,
- roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = {
+ roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP,
+ nullOnOverflow: Boolean = true): Decimal = {
val copy = clone()
- if (copy.changePrecision(precision, scale, roundMode)) copy else null
+ if (copy.changePrecision(precision, scale, roundMode)) {
+ copy
+ } else {
+ val message = s"$toDebugString cannot be represented as Decimal($precision, $scale)."
+ if (nullOnOverflow) {
+ logWarning(s"$message NULL is returned.")
--- End diff --
I agree that a result becomes less useful if we return nulls often. My problem is more that if we process a million non convertible decimals we log the same message a million times, which is going to cause a significant regression. Moreover this is logged on the executor, an end-user typically does not look at those logs (there is also no reason to do so since the job does not throw an error).
My suggestion would be to not log at all, or just log once. I prefer not to log at all.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #89642 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89642/testReport)** for PR 20350 at commit [`d6bc7e9`](https://github.com/apache/spark/commit/d6bc7e9bd4176c229df09b37121388462a750a97).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2560/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163184979
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
@@ -237,14 +238,26 @@ final class Decimal extends Ordered[Decimal] with Serializable {
/**
* Create new `Decimal` with given precision and scale.
*
- * @return a non-null `Decimal` value if successful or `null` if overflow would occur.
+ * @return a non-null `Decimal` value if successful. Otherwise, if `nullOnOverflow` is true, null
+ * is returned; if `nullOnOverflow` is false, an `ArithmeticException` is thrown.
*/
private[sql] def toPrecision(
precision: Int,
scale: Int,
- roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = {
+ roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP,
+ nullOnOverflow: Boolean = true): Decimal = {
val copy = clone()
- if (copy.changePrecision(precision, scale, roundMode)) copy else null
+ if (copy.changePrecision(precision, scale, roundMode)) {
+ copy
+ } else {
+ def message = s"$toDebugString cannot be represented as Decimal($precision, $scale)."
+ if (nullOnOverflow) {
+ if (log.isDebugEnabled) logDebug(s"$message NULL is returned.")
+ null
--- End diff --
since also @hvanhovell was suggesting that this is not necessary, even though I think it would be good to have it, I am removing it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/1159/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87794/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #87154 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87154/testReport)** for PR 20350 at commit [`bd8b645`](https://github.com/apache/spark/commit/bd8b645b09ee81a524da462225f950ec514bd350).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #93401 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93401/testReport)** for PR 20350 at commit [`069b861`](https://github.com/apache/spark/commit/069b86163e03739ef5fa6925c8933521125a1ff0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86488 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86488/testReport)** for PR 20350 at commit [`fcd665e`](https://github.com/apache/spark/commit/fcd665e4a23b21b4ff6e112687ef802ccbc23d0f).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r162957968
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
@@ -237,14 +238,26 @@ final class Decimal extends Ordered[Decimal] with Serializable {
/**
* Create new `Decimal` with given precision and scale.
*
- * @return a non-null `Decimal` value if successful or `null` if overflow would occur.
+ * @return a non-null `Decimal` value if successful. Otherwise, if `nullOnOverflow` is true, null
+ * is returned; if `nullOnOverflow` is false, an `ArithmeticException` is thrown.
*/
private[sql] def toPrecision(
precision: Int,
scale: Int,
- roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = {
+ roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP,
+ nullOnOverflow: Boolean = true): Decimal = {
val copy = clone()
- if (copy.changePrecision(precision, scale, roundMode)) copy else null
+ if (copy.changePrecision(precision, scale, roundMode)) {
+ copy
+ } else {
+ val message = s"$toDebugString cannot be represented as Decimal($precision, $scale)."
+ if (nullOnOverflow) {
+ logWarning(s"$message NULL is returned.")
--- End diff --
I am not sure if we should log this message. If we hit this often we'll end up with huge logs.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #93401 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93401/testReport)** for PR 20350 at commit [`069b861`](https://github.com/apache/spark/commit/069b86163e03739ef5fa6925c8933521125a1ff0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #86524 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86524/testReport)** for PR 20350 at commit [`610a595`](https://github.com/apache/spark/commit/610a595bf61721c38edfaf29dcc161e363319423).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86528/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
sorry @gatorsmile, now that RC for 2.3 has passed the vote, do you happen to have time to look at this? Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86488/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20350
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20350: [SPARK-23179][SQL] Support option to throw except...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/20350#discussion_r163013386
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
@@ -237,14 +238,26 @@ final class Decimal extends Ordered[Decimal] with Serializable {
/**
* Create new `Decimal` with given precision and scale.
*
- * @return a non-null `Decimal` value if successful or `null` if overflow would occur.
+ * @return a non-null `Decimal` value if successful. Otherwise, if `nullOnOverflow` is true, null
+ * is returned; if `nullOnOverflow` is false, an `ArithmeticException` is thrown.
*/
private[sql] def toPrecision(
precision: Int,
scale: Int,
- roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = {
+ roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP,
+ nullOnOverflow: Boolean = true): Decimal = {
val copy = clone()
- if (copy.changePrecision(precision, scale, roundMode)) copy else null
+ if (copy.changePrecision(precision, scale, roundMode)) {
+ copy
+ } else {
+ val message = s"$toDebugString cannot be represented as Decimal($precision, $scale)."
+ if (nullOnOverflow) {
+ logWarning(s"$message NULL is returned.")
--- End diff --
I am ok with using debug/trace level logging. Can you make sure we do not construct the message unless we are logging or throwing the exception (changing `val` into `def` should be enough)?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20350
**[Test build #87794 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87794/testReport)** for PR 20350 at commit [`bd8b645`](https://github.com/apache/spark/commit/bd8b645b09ee81a524da462225f950ec514bd350).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20350
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20350: [SPARK-23179][SQL] Support option to throw exception if ...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20350
My understanding from https://github.com/apache/spark/pull/21499#issuecomment-397010222 is that the plan you have in mind is to have this in 3.0 and not in 2.4 @gatorsmile , am I right? If this is the case, shall I close this now and reopen once 2.4 is out? Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org