subject:"\[GitHub\] spark issue #20023\: \[SPARK\-22036\]\[SQL\] Decimal multiplication with high prec..."

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86271/
Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86271 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86271/testReport)**
 for PR 20023 at commit 
[`cf3b372`](https://github.com/apache/spark/commit/cf3b372ef4d2d4c798bd448f9cf2338269b6ce43).
 * This patch **fails from timeout after a configured wait of \`300m\`**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86277/
Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86277 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86277/testReport)**
 for PR 20023 at commit 
[`2b66098`](https://github.com/apache/spark/commit/2b6609876aa76da5a9169b11142782613ea39ab7).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86276/
Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86276 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86276/testReport)**
 for PR 20023 at commit 
[`7653e6d`](https://github.com/apache/spark/commit/7653e6d5c427c80f925a5992b01c80a8f2d58969).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86272/
Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86272 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86272/testReport)**
 for PR 20023 at commit 
[`03644fe`](https://github.com/apache/spark/commit/03644fe117f56b745c344beb6ea08af5045c2dbd).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86277 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86277/testReport)**
 for PR 20023 at commit 
[`2b66098`](https://github.com/apache/spark/commit/2b6609876aa76da5a9169b11142782613ea39ab7).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86276 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86276/testReport)**
 for PR 20023 at commit 
[`7653e6d`](https://github.com/apache/spark/commit/7653e6d5c427c80f925a5992b01c80a8f2d58969).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86272 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86272/testReport)**
 for PR 20023 at commit 
[`03644fe`](https://github.com/apache/spark/commit/03644fe117f56b745c344beb6ea08af5045c2dbd).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-17 Thread cloud-fan

Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/20023
  
LGTM.

One thing we can improve is the golden file test framework. I found we 
sometimes repeat the test cases with a config on and off. We should write the 
test cases once and list the configs we wanna try, and ask the test framework 
to do it. This can be a follow-up.

@mgaido91 thanks for your great work!


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86271 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86271/testReport)**
 for PR 20023 at commit 
[`cf3b372`](https://github.com/apache/spark/commit/cf3b372ef4d2d4c798bd448f9cf2338269b6ce43).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-17 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
the test failure is unrelated. A proof is that I just updated the migration 
section in the last commit and the previous one was passing all the tests.

@cloud-fan @gatorsmile any more comments?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-17 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
Jenkins, retest this please


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86260/
Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86260 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86260/testReport)**
 for PR 20023 at commit 
[`cf3b372`](https://github.com/apache/spark/commit/cf3b372ef4d2d4c798bd448f9cf2338269b6ce43).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86260 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86260/testReport)**
 for PR 20023 at commit 
[`cf3b372`](https://github.com/apache/spark/commit/cf3b372ef4d2d4c798bd448f9cf2338269b6ce43).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-16 Thread AmplabJenkins

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-16 Thread AmplabJenkins

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86182/
Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-16 Thread SparkQA

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86182 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86182/testReport)**
 for PR 20023 at commit 
[`090659f`](https://github.com/apache/spark/commit/090659fe5f2471462ada0d54c0c855d9fe4aba7e).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-16 Thread SparkQA

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86182 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86182/testReport)**
 for PR 20023 at commit 
[`090659f`](https://github.com/apache/spark/commit/090659fe5f2471462ada0d54c0c855d9fe4aba7e).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-16 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@gatorsmile in the migration guide should I put this change in the section 
Spark 2.2 -> Spark 2.3? Will this change be in Spark 2.3?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-16 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
Update the PR description to explain the related difference between Hive 
and MS SQL Server and also document the solution this PR uses.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-16 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
Since this introduces a behavior change, please update the [migration guide 
of Spark 
SQL](https://spark.apache.org/docs/latest/sql-programming-guide.html#migration-guide)


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86138/
Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86138 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86138/testReport)**
 for PR 20023 at commit 
[`1f36cf6`](https://github.com/apache/spark/commit/1f36cf6bcf784a04d77b2b5153cd504e6155c875).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86138 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86138/testReport)**
 for PR 20023 at commit 
[`1f36cf6`](https://github.com/apache/spark/commit/1f36cf6bcf784a04d77b2b5153cd504e6155c875).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86137/
Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86137 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86137/testReport)**
 for PR 20023 at commit 
[`519571d`](https://github.com/apache/spark/commit/519571d5df6cecc9095bb2029c370fb52c9f6b16).
 * This patch **fails to build**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #86137 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86137/testReport)**
 for PR 20023 at commit 
[`519571d`](https://github.com/apache/spark/commit/519571d5df6cecc9095bb2029c370fb52c9f6b16).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-15 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
sorry @gatorsmile, but I can't follow you. Why do you say it is different 
from the doc? I can't see any difference, sorry. May you explain me please? 
Thanks.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-15 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
@mgaido91 At least it is different from the doc 
https://docs.microsoft.com/en-us/sql/t-sql/data-types/precision-scale-and-length-transact-sql


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-13 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@gatorsmile as far as I know, Hive implementation is consistent with MS SQL 
Server 2006 and the implementation is taken by the description of its behavior 
in MSDN blogs. DB2 implementation I think is not suitable for any big data 
system, since it is very likely to fail at runtime wasting a lot of 
computational time in big data applications.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-13 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
The proposal of this PR is to follow Hive instead of MS SQL Server and DB2. 
I read the comments in  HIVE-15331. Nobody pointed out why Hive decided to 
introduce new behaviors which are actually inconsistent with MS SQL Server. 
Looks weird to me. Anybody knew the reason?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-12 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
any more comments @gatorsmile @cloud-fan @dongjoon-hyun @viirya @hvanhovell 
?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-10 Thread AmplabJenkins

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-10 Thread AmplabJenkins

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85914/
Test PASSed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-10 Thread SparkQA

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #85914 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85914/testReport)**
 for PR 20023 at commit 
[`ecd6a2a`](https://github.com/apache/spark/commit/ecd6a2a0d01283b0d3a9582c93618ff1c2a772f7).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2018-01-10 Thread SparkQA

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #85914 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85914/testReport)**
 for PR 20023 at commit 
[`ecd6a2a`](https://github.com/apache/spark/commit/ecd6a2a0d01283b0d3a9582c93618ff1c2a772f7).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85863/
Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Build finished. Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #85863 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85863/testReport)**
 for PR 20023 at commit 
[`6701a54`](https://github.com/apache/spark/commit/6701a54068145994e10b8dd38d9a38a1be1f3674).
 * This patch **fails Spark unit tests**.
 * This patch **does not merge cleanly**.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85864/
Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/20023
  
Merged build finished. Test FAILed.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #85864 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85864/testReport)**
 for PR 20023 at commit 
[`20616fd`](https://github.com/apache/spark/commit/20616fdfc1a75ea9ae0ec531ce72d8c722facb31).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #85864 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85864/testReport)**
 for PR 20023 at commit 
[`20616fd`](https://github.com/apache/spark/commit/20616fdfc1a75ea9ae0ec531ce72d8c722facb31).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/20023
  
**[Test build #85863 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85863/testReport)**
 for PR 20023 at commit 
[`6701a54`](https://github.com/apache/spark/commit/6701a54068145994e10b8dd38d9a38a1be1f3674).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-30 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
thanks @gatorsmile, do you have any suggestion for the conf name?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-29 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
Sounds OK to me. Add a conf for this behavior change and also document the 
behavior change in migration section?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-29 Thread cloud-fan

Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/20023
  
hmmm, even we can add many modes like hive mode, postgres mode, etc. we 
should still make the default/native mode reasonable. It seems more reasonable 
to truncate the values instead of returning null, in the case of precision 
lose. @gatorsmile WDYT? Overflow is another story and we should still return 
null, but introduce a strict mode to throw exception for this case.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-29 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@cloud-fan thanks for your feedback. Honestly I don't have a string opinion 
on how we should behave in that case. That's why I wanted some feedbacks from 
the community on that point, while this PR covers only the other case, ie. 
rounding in case of precision loss in accordance to Hive and SQLServer's 
behavior.

Anyway, IIUC now this PR is blocked by #18853 to introduce 
`spark.sql.typeCoercion.mode` and use that property to decide whether to keep 
the previous behavior or the one proposed here.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-29 Thread cloud-fan

Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/20023
  
I think for big data, throwing exception during runtime is pretty bad. I 
think it's ok to be incompatible with SQL standard for some cases, and return 
null.

Personally I feel it's ok to follow SQL standard and truncate if precision 
lose, but we should not follow SQL standard to throw exception when overflow, 
at least this should not be the default behavior.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-26 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@gatorsmile created #20084 for adding tests. Thanks.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-25 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
The problem found by this PR is just one of the issues that returns NULL 
when SPARK SQL is unable to process it. Below is another example. I believe we 
can find more.

```SQL
SELECT CAST('a' AS TIMESTAMP)
```

Before we deciding how to fix these issues (one by one or as a whole), we 
need to do more investigation and identify all of them. We also need to clearly 
document our current behaviors and then our users can know what is the result 
they can expect. 

Yeah! Please go ahead to create a new PR for adding more tests. Thanks!


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-25 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
Thanks @gatorsmile. Then should I create a follow up PR for #20008 in order 
to cover the cases 2 and 3 before going on with this PR or can we go on with 
this PR and the test cases added in this PR?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-24 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
Following ANSI SQL compliance sounds good to me. However, many details are 
vendor-specific. That means, the query results still varies even if we can be 
100% ANSI SQL compliant. 

To avoid frequently introducing behavior breaking changes, we can also 
introduce a new mode `strict` for `spark.sql.typeCoercion.mode`. (Hive is also 
not 100% ANSI SQL compliant) Instead of inventing a completely new one, we can 
try to follow one of the mainstream open-source databases. For example, 
Postgres. 

Before introducing the new mode, we first need to understand the difference 
between Spark SQL and the other. That is the reason why we need to write the 
test cases first. Then, we can run them against different systems. This PR 
clearly shows the current test cases do not cover the scenarios of 2 and 3. 



---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-24 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
Thanks for your analysis @gatorsmile. Actually the rule you specified for 
Oracle is what it uses when casting, rather then when doing arithmetic 
operations. Yes DB2 has rather different rules to define the output type of 
operations. Anyway, we can have a behavior practically identical to DB2 by 
changing the value of `MINIMUM_ADJUSTED_SCALE` to 31. Therefore, I'd propose, 
instead of using the configuration you pointed out, to use a configuration for 
the `MINIMUM_ADJUSTED_SCALE`, changing which we can have both the behavior of 
Hive and SQLServer and the one of DB2. What do you think?

The reason why I am suggesting this is that my first concern is not Hive 
compliance, but SQL standard compliance. Indeed, as you con see from the 
summary, on point 1 there is not a uniform behavior (but this is OK to SQL 
standard since it gives freedom). But on point 2 we are the only ones who are 
not compliant to SQL standard. And having this behavior by default doesn't look 
the right thing to do IMHO. On point 3, only we and Hive are not compliant. 
Thus I think also that should be changed. But in that case, we can't use the 
same flag, because it would be inconsistent. What do you think?

I can understand and agree that loosing precision looks scary. But to me 
returning `NULL` is even more scary if possible: indeed, `NULL` is what should 
be returned if either if the two operands are `NULL`. Thus queries running on 
other DBs which relies on this might return very bad result. For instance, 
let's think to a report where we join a prices table and a sold_product table 
per country. In this use case, we can assume that if the result is `NULL`, it 
means that there was no sold product in that country and then coalesce the 
output of the multiplication to 0. This would work well on any DB but Spark. 
With my proposal of tuning the `MINIMUM_ADJUSTED_SCALE`, each customer can 
decide (query by query) how much precision loss they can tolerate. And if we 
agree to change point 3 behavior to the SQL standard, in case of it is not 
possible to meet their desires we throw an exception, giving them the choice 
about what to do: allow more precision loss, change their input data type, et
 c. etc. This is the safer way IMHO.

I would ne happy to help improving test cases. May I just kindly ask you 
how you meant to do that? What would you like to be tested more? Would you like 
me to add more test cases in scope of this PR or to open a new one for that?

Thank you for your time reading my long messages. I just want to take the 
best choice and give you all the elements I have to decide for the best all 
together.
Thank you.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-24 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023

Thanks for your careful summary!

We do not have a SQLCA. Thus, it is hard for us to send a warning message
back like [what DB2
does.](https://www.ibm.com/support/knowledgecenter/SSEPEK_10.0.0/sqlref/src/tpc/db2z_decimalmultiplication.html).
Silently losing the precision looks scary to me. Oracle sounds like following
the rule, [`If a value exceeds the precision, then Oracle returns an error. If
a value exceeds the scale, then Oracle rounds
it.`](https://docs.oracle.com/en/database/oracle/oracle-database/12.2/sqlrf/Data-Types.html#GUID-9401BC04-81C4-4CD5-99E7-C5E25C83F608)

SQL ANSI 2011 does not document many details. For example, [the result type
of DB2's
division](https://www.ibm.com/support/knowledgecenter/en/SSEPEK_10.0.0/sqlref/src/tpc/db2z_decimaldivision.html)
is different from either our existing rule or the rule you changed. The rule
you mentioned above about DB2 is just for multiplification.

I am not sure whether we can finalize our default type cocersion rule
`DecimalPrecision` now. However, for Hive compliance, we can add a new rule
after we introduce the new conf `spark.sql.typeCoercion.mode`. See the PR
https://github.com/apache/spark/pull/18853 for details. The new behavior will
be corrected if and only if `spark.sql.typeCoercion.mode` is set to `hive`.

Could you first improve the test cases added in
https://github.com/apache/spark/pull/20008 ?

---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-23 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@gatorsmile that is scenario 3. I will explain you why and after I will do 
and errata corrige of the summary I did in my last e-mail, because I made a 
mistake about how DB2 computes the result precision and scale, sorry for that.

Anyway, what you showed is an example of point 3 because DB2 computes the 
result type as `DECIMAL( MIN(31, p1 + p2), MIN(31, s1 + s2) )`. Therefore, in 
your case the result type was `DECIMAL(31, 31)`. Since your result had more 
than 0 significant digits, it was out out of the range of the representable 
values and an overflow exception was thrown.
You can reproduce case 2 as follows:
```
db2 => create table decimals_test (id int, a decimal(31,31), b 
decimal(31,31))
DB2I  The SQL command completed successfully.
db2 => insert into decimals_test values(1, 0.12345678912345678912345689, 
0.12345678912345678912345) 
DB2I  The SQL command completed successfully.
db2 => select a*b from dd

1
-
 .0152415787806736785461049526020
```
As you can see a truncation occurred.

Now, let me amend my table to summarize the behavior of the many DBs:
1. **Rules to determine precision and scale**
 - *Hive, SQLServer (and Spark after the PR)*: I won't include the
exact formulas, anyway the relevant part is that in case of precision
higher that the maximum value, we use the maximum available value (38) as
precision and the maximum between the needed scale (computing according the
relevant formula) and a minimum value guaranteed for the scale which is 6.
 - *DB2*: computes the result type as `DECIMAL( MIN(31, p1 + p2), 
MIN(31, s1 + s2) )`.
 - *Postgres and Oracle*: NA
 - *SQL ANSI 2011*: no indication
 - *Spark now*: if the precision needed is more than 38, use 38 as
precision; use the needed scale without any adjustment.

  2. **Behavior in case of precision loss but result in the range of the
representable values**
 - *Oracle, Hive, SQLServer (and Spark after the PR)*: round the result.
 - *DB2*: truncates the result (and sets a warning flag).
 - *Postgres*: NA, it has infinite precision...
 - *SQL ANSI 2011*: either truncate or round the value.
 - *Spark now*: returns NULL.

  3. **Behavior in case of result out of the range of the representable
values (i.e overflow)**
 - *Oracle, DB2, SQLServer*: throw an exception.
 - *Postgres*: NA, they have nearly infinite precision...
 - *SQL ANSI 2011*: an exception should be raised
 - *Spark now, Hive*: return NULL (for Hive, there is a open ticket to
make it compliant to the SQL standard).



---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-22 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
```
db2 => create table decimals_test(id int, a decimal(31,18), b 
decimal(31,18))
DB2I  The SQL command completed successfully.
db2 => insert into decimals_test values (1, 2.33, 1.123456789123456789)
DB2I  The SQL command completed successfully.
db2 => select a * b from decimals_test

1
-
SQL0802N  Arithmetic overflow or other arithmetic exception occurred.  
SQLSTATE=22003
```

I might not get your point. Above is the result I got. This is your 
scenario 3 or 2?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-22 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@gatorsmile I answered to your comments about DB2 in the e-mail.

@cloud-fan that would help, but not solve the problem. It would just make 
the problem being generated by bigger numbers.

As you can see from the e-mail, DB2 behavior is actually in accordance to 
SQL standards and the other DBs, it just have a smaller maximum precision. And 
the case of throwing an exception is point 3 of my e-mails and it is out of 
scope of this PR, because I think we best discuss before which is the right 
approach in that case and then I can eventually create a PR.
Therefore, also DB2 behavior is aligned to the other SQL engines, the SQL 
standard and Spark is the only one which is currently behaving differently.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-22 Thread cloud-fan

Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/20023
  
it looks like other databases are very careful about precision lose. 
However, following DB2 and throwing exception is pretty bad for big data 
applications, but returning null is also bad as it violates SQL standard.

A new proposal: can we increase the max decimal precision to 76 and keep 
max scale as 38? Then we can avoid precision lose IIUC.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-21 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
Regarding the rules for deciding precision and scales, DB2 z/OS also has 
its own rules: 
https://www.ibm.com/support/knowledgecenter/en/SSEPEK_10.0.0/sqlref/src/tpc/db2z_witharithmeticoperators.html

Could you compare it with MS SQL Server?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-21 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
In DB2, `a * b` will just stop with an error message: SQL0802N Arithmetic 
overflow or other arithmetic exception occurred.

Thus, it might be more straightforward for us to follow what DB2 does.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-21 Thread mgaido91

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@gatorsmile, please refer to the [e-mail to the dev mail 
list](https://mail-archives.apache.org/mod_mbox/spark-dev/201712.mbox/%3CCAEorWNAJ4TxJR9NBcgSFMD_VxTg8qVxusjP%2BAJP-x%2BJV9zH-yA%40mail.gmail.com%3E)
 for further details. I run the script I added to the tests in this PR, the 
results are:

 - Hive behaves exactly as Spark after this PR;
 - SQLServer the same, even though on additions and subtractions it seems 
to maintain one more precision digit in some cases (I am running SQLServer 
2017, since Hive implementation, and therefore this too, are inspired to 
SQLServer2005, there might have been a small behavior change in this case). 
Anyway, differently from Hive and Spark it throws an exception in case 3 
described in the email (it is compliant to SQL standard, point 3 of the email 
is out of scope of this PR, I will create another PR for it once we agree on 
how to handle that case);
 - Oracle and Postgres have nearly infinite precision. Thus it is nearly 
impossible to provoke a rounding on them. If we force a precision loss on them 
(point 3 of the email, out of scope of this PR) they throw an exception 
(compliant to SQL standard and SQLServer);

Here you are the outputs of the queries.

**Hive 2.3.0 (same as Spark after PR)**

```
0: jdbc:hive2://localhost:1> create table decimals_test(id int, a 
decimal(38,18), b decimal(38,18));
No rows affected (2.085 seconds)
0: jdbc:hive2://localhost:1> insert into decimals_test values (1, 
100.0, 999.0), (2, 12345.123, 12345.123), (3, 0.1234567891011, 1234.1), (4, 
123456789123456789.0, 1.123456789123456789);
No rows affected (14.054 seconds)
0: jdbc:hive2://localhost:1> select id, a+b, a-b, a*b, a/b from 
decimals_test order by id;

+-+---+---+++
| id  |  _c1  |  _c2
  |_c3 |_c4 |

+-+---+---+++
| 1   | 1099.0| -899.0  
  | 99900.00   | 0.100100   |
| 2   | 24690.24600   | 0E-17   
  | 152402061.885129   | 1.00   |
| 3   | 1234.2234567891011| -1233.9765432108989 
  | 152.358023 | 0.000100   |
| 4   | 123456789123456790.12345678912345679  | 
123456789123456787.87654321087654321  | 138698367904130467.515623  | 
109890109097814272.043109  |

+-+---+---+++
```

**SQLServer 2017**

```
1> create table decimals_test(id int, a decimal(38,18), b decimal(38,18));
2> insert into decimals_test values (1, 100.0, 999.0), (2, 12345.123, 
12345.123), (3, 0.1234567891011, 1234.1), (4, 123456789123456789.0, 
1.123456789123456789);
3> select id, a+b, a-b, a*b, a/b from decimals_test order by id;
4> GO

(4 rows affected)
id  

   
---  
 
 

  1  1099.00  
-899.00 99900.00
  .100100
  2 24690.246000  
.00 152402061.885129
 1.00
  3  1234.22345678910110 
-1233.97654321089890   152.358023   
   .000100
  4123456789123456790.123456789123456789
123456789123456787.876543210876543211138698367904130467.515623  
  109890109097814272.043109
```

**Postgres and Oracle**

```
postgres=# create table decimals_test(id int, a decimal(38,18), b 
decimal(38,18));
CREATE TABLE
postgres=# insert into decimals_test values (1, 100.0, 999.0), (2, 
12345.123, 12345.123), (3, 0.1234567891011, 1234.1), (4, 123456789123456789.0, 
1.123456789123456789);
INSERT 0 4
postgres=# select id, a+b, a-b, a*b, a/b from decimals_te

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-21 Thread gatorsmile

Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/20023
  
Thanks for your efforts! This change needs more careful review and 
investigation. 

Could you post the outputs of Oracle, DB2, SQL server and Hive? Are their 
results consistent?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-20 Thread viirya

Github user viirya commented on the issue:

https://github.com/apache/spark/pull/20023
  
I think that we should be careful on suddenly changing behavior.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-20 Thread dongjoon-hyun

Github user dongjoon-hyun commented on the issue:

https://github.com/apache/spark/pull/20023
  
I thought adding more cases into `decimals.sql`. :) But, for now, never 
mind about that~


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
Thank you for your review @dongjoon-hyun. I think what we can do is add 
more test to the whitelist in `HiveCompatibilitySuite`, updating them according 
to HIVE-15331. Were you thinking to this or something different? Thanks.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@hvanhovell, as far as 1 is regarded, I was referring to [this 
comment](https://github.com/apache/spark/pull/19449#pullrequestreview-67789784) 
and [this PR](https://github.com/apache/spark/pull/18568) where it is 
explicitly stated that using `SQLConf` here is not safe and it shouldn't be 
done. Let me know if I am missing something.
I am sorry, but I think I haven't fully understood what you meant by 

> Or you can wire up the rules using the SessionStateBuilder

may I kindly ask you if you could elaborate this sentence a bit more? Thank 
you very much.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-20 Thread dongjoon-hyun

Github user dongjoon-hyun commented on the issue:

https://github.com/apache/spark/pull/20023
  
Thank you for pining me, @mgaido91 . The approach of PR looks good to me.
BTW, do we need to borrow more Hive 2.2 test cases?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-20 Thread hvanhovell

Github user hvanhovell commented on the issue:

https://github.com/apache/spark/pull/20023
  
I don't fully agree :)...

1. You can use `SQLConf.get` for this. Or you can wire up the rules using 
the `SessionStateBuilder`.
2. I am reluctant to change this for a minor version. I don't think the 
Hive approach is very good for UX.
3. People are also going to complain if do change it.




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
thanks for looking at this @hvanhovell. The reasons why I didn't introduce 
a configuration variable for this behavior are:

 1. As far as I know, currently there is no way to read reliably a 
configuration in catalyst;
 2. Also in Hive, the behavior was changed without introducing any 
configuration to switch back to the previous behavior;
 3. Many people are complaining about the current Spark behavior in the 
JIRA and therefore it seems that the previous behavior is neither desired nor 
useful to users.

Let me know if you don't agree with these arguments. Thanks.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-20 Thread hvanhovell

Github user hvanhovell commented on the issue:

https://github.com/apache/spark/pull/20023
  
In am generally in favor of following the SQL standard. How about we do 
this. Let's make the standard behavior the default, and add a flag to revert to 
the old behavior. This allows us to ease users into the new behavior, and for 
us it can provide some data points on when we can remove the old behavior. I 
hope we can remove this for Spark 2.4 or later.

At the end of the day it will be a bit more work, as I'd definitely would 
make an effort to isolate the the two behaviors as much as possible.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

Github user mgaido91 commented on the issue:

https://github.com/apache/spark/pull/20023
  
@cloud-fan yes, Hive changed and most important at the moment we are not 
compliant with SQL standard. So currently Spark is returning results which are 
different from Hive and not compliant with SQL standard. This is why I proposed 
this change.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...

2017-12-20 Thread cloud-fan

Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/20023
  
Ideally we should not change behaviors as possible as we can, but since 
this behavior is from Hive and Hive also changed it, might be OK to follow Hive 
and also change it? cc @hvanhovell  too


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark issue #20023: [SPARK-22036][SQL] Decimal multiplication with high prec...