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/28 12:54:51 UTC
[GitHub] [spark] peter-toth opened a new pull request, #37696: [SPARK-40247][SQL] Fix BitSet equality check
peter-toth opened a new pull request, #37696:
URL: https://github.com/apache/spark/pull/37696
### What changes were proposed in this pull request?
Spark's `BitSet` doesn't implement `equals()` and `hashCode()` but it is used in `FileSourceScanExec` for bucket pruning.
### Why are the changes needed?
Without proper equality check reuse issues can occur.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added new UT.
--
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] peter-toth commented on pull request #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
peter-toth commented on PR #37696:
URL: https://github.com/apache/spark/pull/37696#issuecomment-1229844888
cc @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 #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #37696:
URL: https://github.com/apache/spark/pull/37696#issuecomment-1229885366
thanks, merging to master/3.3 (it's a mistake not implementing equals/hashCode)
--
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 #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37696:
URL: https://github.com/apache/spark/pull/37696#discussion_r956968957
##########
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala:
##########
@@ -4003,6 +4003,31 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
+ test("SPARK-40247: Fix BitSet equals") {
+ withTable("td") {
+ testData
+ .withColumn("bucket", $"key" % 3)
+ .write
+ .mode(SaveMode.Overwrite)
+ .bucketBy(2, "bucket")
+ .format("parquet")
+ .saveAsTable("td")
+ val df = sql(
+ """
+ |SELECT t1.key, t2.key, t3.key
Review Comment:
do we return wrong result before this PR?
--
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] peter-toth commented on a diff in pull request #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
peter-toth commented on code in PR #37696:
URL: https://github.com/apache/spark/pull/37696#discussion_r956977360
##########
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala:
##########
@@ -4003,6 +4003,31 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
+ test("SPARK-40247: Fix BitSet equals") {
+ withTable("td") {
+ testData
+ .withColumn("bucket", $"key" % 3)
+ .write
+ .mode(SaveMode.Overwrite)
+ .bucketBy(2, "bucket")
+ .format("parquet")
+ .saveAsTable("td")
+ val df = sql(
+ """
+ |SELECT t1.key, t2.key, t3.key
Review Comment:
No, just exchange reuse didn't happen.
--
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] peter-toth commented on a diff in pull request #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
peter-toth commented on code in PR #37696:
URL: https://github.com/apache/spark/pull/37696#discussion_r956977360
##########
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala:
##########
@@ -4003,6 +4003,31 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
+ test("SPARK-40247: Fix BitSet equals") {
+ withTable("td") {
+ testData
+ .withColumn("bucket", $"key" % 3)
+ .write
+ .mode(SaveMode.Overwrite)
+ .bucketBy(2, "bucket")
+ .format("parquet")
+ .saveAsTable("td")
+ val df = sql(
+ """
+ |SELECT t1.key, t2.key, t3.key
Review Comment:
No, only exchange reuse didn't happen.
--
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 #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37696:
URL: https://github.com/apache/spark/pull/37696#discussion_r956978506
##########
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala:
##########
@@ -4003,6 +4003,31 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
+ test("SPARK-40247: Fix BitSet equals") {
+ withTable("td") {
+ testData
+ .withColumn("bucket", $"key" % 3)
+ .write
+ .mode(SaveMode.Overwrite)
+ .bucketBy(2, "bucket")
+ .format("parquet")
+ .saveAsTable("td")
+ val df = sql(
+ """
+ |SELECT t1.key, t2.key, t3.key
Review Comment:
ah, so this is just a perf improvement.
--
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 #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #37696: [SPARK-40247][SQL] Fix BitSet equality check
URL: https://github.com/apache/spark/pull/37696
--
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] peter-toth commented on pull request #37696: [SPARK-40247][SQL] Fix BitSet equality check
Posted by GitBox <gi...@apache.org>.
peter-toth commented on PR #37696:
URL: https://github.com/apache/spark/pull/37696#issuecomment-1229887254
> good catch! which spark version starts to have this bug?
SPARK-23803 added in Spark 2.4 that `optionalBucketSet: Option[BitSet]` field to `FileSourceScanExec`.
> thanks, merging to master/3.3 (it's a mistake not implementing equals/hashCode)
Thanks for the review.
--
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