-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-38786][SQL][TEST] Bug in StatisticsSuite 'change stats after add/drop partition command' #36075
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…dd/drop partition command'
sunchao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @kazuyukitanimura !
|
Committed to master, thanks. |
…dd/drop partition command' ### What changes were proposed in this pull request? https://github.com/apache/spark/blob/cbffc12f90e45d33e651e38cf886d7ab4bcf96da/sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala#L979 It should be `partDir2` instead of `partDir1`. Looks like it is a copy paste bug. ### Why are the changes needed? Due to this test bug, the drop command was dropping a wrong (`partDir1`) underlying file in the test. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Added extra underlying file location check. Closes #36075 from kazuyukitanimura/SPARK-38786. Authored-by: Kazuyuki Tanimura <[email protected]> Signed-off-by: Chao Sun <[email protected]> (cherry picked from commit a6b04f0) Signed-off-by: Dongjoon Hyun <[email protected]>
|
Thank you, @kazuyukitanimura , @sunchao , @huaxingao . Since the original bug was introduced at SPARK-33770 and was backported to old branches, I'll test and backport this to branch-3.3/3.2/3.1 too. |
|
cc @MaxGekk |
…dd/drop partition command' ### What changes were proposed in this pull request? https://github.com/apache/spark/blob/cbffc12f90e45d33e651e38cf886d7ab4bcf96da/sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala#L979 It should be `partDir2` instead of `partDir1`. Looks like it is a copy paste bug. ### Why are the changes needed? Due to this test bug, the drop command was dropping a wrong (`partDir1`) underlying file in the test. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Added extra underlying file location check. Closes #36075 from kazuyukitanimura/SPARK-38786. Authored-by: Kazuyuki Tanimura <[email protected]> Signed-off-by: Chao Sun <[email protected]> (cherry picked from commit a6b04f0) Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit a52a245) Signed-off-by: Dongjoon Hyun <[email protected]>
HyukjinKwon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM2
…dd/drop partition command' ### What changes were proposed in this pull request? https://github.com/apache/spark/blob/cbffc12f90e45d33e651e38cf886d7ab4bcf96da/sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala#L979 It should be `partDir2` instead of `partDir1`. Looks like it is a copy paste bug. ### Why are the changes needed? Due to this test bug, the drop command was dropping a wrong (`partDir1`) underlying file in the test. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Added extra underlying file location check. Closes #36075 from kazuyukitanimura/SPARK-38786. Authored-by: Kazuyuki Tanimura <[email protected]> Signed-off-by: Chao Sun <[email protected]> (cherry picked from commit a6b04f0) Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit a52a245) Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 04161ac) Signed-off-by: Dongjoon Hyun <[email protected]>
…dd/drop partition command' (apache#1385) ### What changes were proposed in this pull request? https://github.com/apache/spark/blob/cbffc12f90e45d33e651e38cf886d7ab4bcf96da/sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala#L979 It should be `partDir2` instead of `partDir1`. Looks like it is a copy paste bug. ### Why are the changes needed? Due to this test bug, the drop command was dropping a wrong (`partDir1`) underlying file in the test. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Added extra underlying file location check. Closes apache#36075 from kazuyukitanimura/SPARK-38786. Authored-by: Kazuyuki Tanimura <[email protected]> Signed-off-by: Chao Sun <[email protected]>
What changes were proposed in this pull request?
spark/sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala
Line 979 in cbffc12
It should be
partDir2instead ofpartDir1. Looks like it is a copy paste bug.Why are the changes needed?
Due to this test bug, the drop command was dropping a wrong (
partDir1) underlying file in the test.Does this PR introduce any user-facing change?
No
How was this patch tested?
Added extra underlying file location check.