You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/08/16 19:56:31 UTC
[GitHub] [spark] revans2 opened a new pull request, #37540: [SPARK-40089][SQL ] Fix sorting for some Decimal types
revans2 opened a new pull request, #37540:
URL: https://github.com/apache/spark/pull/37540
### What changes were proposed in this pull request?
This fixes https://issues.apache.org/jira/browse/SPARK-40089 where the prefix can overflow in some cases and the code assumes that the overflow is always on the negative side, not the positive side.
### Why are the changes needed?
This adds a check when the overflow does happen to know what is the proper prefix to return.
### Does this PR introduce _any_ user-facing change?
No, unless you consider getting the sort order correct a user facing change.
### How was this patch tested?
I tested manually with the file in the JIRA and I added a small unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ulysses-you commented on pull request #37540: [SPARK-40089][SQL] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1221871439
lgtm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #37540: [SPARK-40089][SQL] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #37540: [SPARK-40089][SQL] Fix sorting for some Decimal types
URL: https://github.com/apache/spark/pull/37540
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] ulysses-you commented on pull request #37540: [SPARK-40089][SQL ] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1217373842
good catch ! cc @HyukjinKwon @dongjoon-hyun @cloud-fan
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #37540: [SPARK-40089][SQL] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1222030048
thanks, merging to master/3.3/3.2/3.1!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #37540: [SPARK-40089][SQL ] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1218701323
> Is there a good way to kick the build to retry?
you can push an empty commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] revans2 commented on pull request #37540: [SPARK-40089][SQL] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
revans2 commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1220971184
@cloud-fan could you please take another look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #37540: [SPARK-40089][SQL ] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37540:
URL: https://github.com/apache/spark/pull/37540#discussion_r947497593
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/SortSuite.scala:
##########
@@ -114,6 +114,22 @@ class SortSuite extends SparkPlanTest with SharedSparkSession {
sortAnswers = false)
}
+ test("SPARK-40089: decimal values sort correctly") {
+ val input = Seq(
+ BigDecimal("999999999999999999.50"),
+ BigDecimal("1.11"),
+ BigDecimal("999999999999999999.49")
+ )
+ spark.conf.set("spark.sql.shuffle.partitions", "1")
Review Comment:
nit: use `withSQLConf` which will restore the conf value at the end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #37540: [SPARK-40089][SQL ] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1217515345
good catch! Is this a long-standing bug? cc @hvanhovell @somani @maryannxue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] revans2 commented on pull request #37540: [SPARK-40089][SQL ] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
revans2 commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1218107148
> good catch! Is this a long-standing bug? cc @hvanhovell @somani @maryannxue
According to git blame it has been at least 7 years (Spark 1.5).
https://issues.apache.org/jira/browse/SPARK-9529
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] revans2 commented on pull request #37540: [SPARK-40089][SQL ] Fix sorting for some Decimal types
Posted by GitBox <gi...@apache.org>.
revans2 commented on PR #37540:
URL: https://github.com/apache/spark/pull/37540#issuecomment-1218499775
Is there a good way to kick the build to retry? the failure appears totally unrelated to this patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org