This is an automated email from the ASF dual-hosted git repository.
yao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 7b6077a02fc3 [SPARK-46584][SQL][TESTS] Remove invalid
attachCleanupResourceChecker in JoinSuite
7b6077a02fc3 is described below
commit 7b6077a02fc3e619465fb21511ea16e71e6d4c7e
Author: zml1206 <zhuml1...@gmail.com>
AuthorDate: Thu Jan 4 10:32:46 2024 +0800
[SPARK-46584][SQL][TESTS] Remove invalid attachCleanupResourceChecker in
JoinSuite
### What changes were proposed in this pull request?
Remove `attachCleanupResourceChecker` in `JoinSuite`.
### Why are the changes needed?
`attachCleanupResourceChecker` is invalid:
1. The matching of `SortExec` needs to be in `QueryExecution.executePlan`
not `QueryExecution.sparkPlan`, The correct way is
`foreachUp(df.queryExecution.executedPlan){f()}`.
2. `Mockito` counts the number of function calls, only for objects after
`spy`. Calls to the original object are not counted. eg
```
test() {
val data = new java.util.ArrayList[String]()
val _data = spy(data)
data.add("a");
data.add("b");
data.add("b");
_data.add("b")
verify(_data, times(0)).add("a")
verify(_data, times(1)).add("b")
}
```
Therefore, when using `df.queryExecution.executedPlan` correctly to match,
count is always 0.
3. Not all `SortMergeJoin` joinTypes will trigger `cleanupResources()`,
such as 'full outer join'.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Local test, update `attachCleanupResourceChecker` atLeastOnce to nerver, ut
is still successful.
```
verify(sortExec, atLeastOnce).cleanupResources()
verify(sortExec.rowSorter, atLeastOnce).cleanupResources()
```
to
```
verify(sortExec, never).cleanupResources()
verify(sortExec.rowSorter, never).cleanupResources()
```
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #44573 from zml1206/SPARK-21492.
Authored-by: zml1206 <zhuml1...@gmail.com>
Signed-off-by: Kent Yao <y...@apache.org>
---
.../test/scala/org/apache/spark/sql/JoinSuite.scala | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala
index 909a05ce26f7..f31f60e8df56 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala
@@ -22,8 +22,6 @@ import java.util.Locale
import scala.collection.mutable.ListBuffer
import scala.jdk.CollectionConverters._
-import org.mockito.Mockito._
-
import org.apache.spark.TestUtils.{assertNotSpilled, assertSpilled}
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
@@ -44,23 +42,6 @@ import org.apache.spark.tags.SlowSQLTest
class JoinSuite extends QueryTest with SharedSparkSession with
AdaptiveSparkPlanHelper {
import testImplicits._
- private def attachCleanupResourceChecker(plan: SparkPlan): Unit = {
- // SPARK-21492: Check cleanupResources are finally triggered in SortExec
node for every
- // test case
- plan.foreachUp {
- case s: SortExec =>
- val sortExec = spy[SortExec](s)
- verify(sortExec, atLeastOnce).cleanupResources()
- verify(sortExec.rowSorter, atLeastOnce).cleanupResources()
- case _ =>
- }
- }
-
- override protected def checkAnswer(df: => DataFrame, rows: Seq[Row]): Unit =
{
- attachCleanupResourceChecker(df.queryExecution.sparkPlan)
- super.checkAnswer(df, rows)
- }
-
setupTestData()
def statisticSizeInByte(df: DataFrame): BigInt = {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
