[SPARK-46237][SQL][TESTS] Make `HiveDDLSuite` independently testable #44153

LuciferYang · 2023-12-04T08:26:56Z

What changes were proposed in this pull request?

When I test HiveDDLSuite with

build/sbt "hive/testOnly org.apache.spark.sql.hive.execution.HiveDDLSuite" -Phive

This test throws an error:

[info] - SPARK-34261: Avoid side effect if create exists temporary function *** FAILED *** (4 milliseconds)
[info]   java.util.NoSuchElementException: key not found: default
[info]   at scala.collection.MapOps.default(Map.scala:274)
[info]   at scala.collection.MapOps.default$(Map.scala:273)
[info]   at scala.collection.AbstractMap.default(Map.scala:405)
[info]   at scala.collection.MapOps.apply(Map.scala:176)
[info]   at scala.collection.MapOps.apply$(Map.scala:175)
[info]   at scala.collection.AbstractMap.apply(Map.scala:405)
[info]   at org.apache.spark.sql.hive.execution.HiveDDLSuite.$anonfun$new$445(HiveDDLSuite.scala:3275)
[info]   at org.apache.spark.sql.test.SQLTestUtilsBase.withUserDefinedFunction(SQLTestUtils.scala:256)
[info]   at org.apache.spark.sql.test.SQLTestUtilsBase.withUserDefinedFunction$(SQLTestUtils.scala:254)
[info]   at org.apache.spark.sql.execution.command.DDLSuite.withUserDefinedFunction(DDLSuite.scala:326)
[info]   at org.apache.spark.sql.hive.execution.HiveDDLSuite.$anonfun$new$444(HiveDDLSuite.scala:3267)

I manually printed the content of spark.sparkContext.addedJars, which is an empty Map.

However, when I execute

build/sbt "hive/testOnly org.apache.spark.sql.hive.execution.SQLQuerySuite org.apache.spark.sql.hive.execution.HiveDDLSuite" -Phive

All tests pass, and the content of spark.sparkContext.addedJars is

Map(default -> Map(spark://localhost:54875/jars/SPARK-21101-1.0.jar -> 1701676986594, spark://localhost:54875/jars/hive-contrib-2.3.9.jar -> 1701676944590, spark://localhost:54875/jars/TestUDTF.jar -> 1701676921340))

The reason why this failure is not reproduced in the GitHub Action test is because SQLQuerySuite is indeed executed before HiveDDLSuite.

So in the current PR, I change to use .get("default").foreach(_.remove(k)) that the remove operation is only performed when .get("default") is not None.

Why are the changes needed?

Make HiveDDLSuite independently testable.

Does this PR introduce any user-facing change?

No, just for test

How was this patch tested?

Pass Github Actions
Manual check HiveDDLSuite with this pr and all test passed

Was this patch authored or co-authored using generative AI tooling?

No

LuciferYang · 2023-12-04T08:41:13Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

      val jar = spark.asInstanceOf[TestHiveSparkSession].getHiveFile(jarName).toURI.toString
      spark.sparkContext.allAddedJars.keys.find(_.contains(jarName))
-        .foreach(spark.sparkContext.addedJars("default").remove)
+        .foreach(k => spark.sparkContext.addedJars.get("default") match {


@HyukjinKwon

I noticed that this logic was added in #41495. If I'm not mistaken, this is a defensive deletion to avoid including TestUDTF.jar in default during testing? However, I found that running only this test:

build/sbt "hive/testOnly org.apache.spark.sql.hive.execution.HiveDDLSuite" -Phive

throws an error:

[info] - SPARK-34261: Avoid side effect if create exists temporary function *** FAILED *** (4 milliseconds) [info] java.util.NoSuchElementException: key not found: default [info] at scala.collection.MapOps.default(Map.scala:274) [info] at scala.collection.MapOps.default$(Map.scala:273) [info] at scala.collection.AbstractMap.default(Map.scala:405) [info] at scala.collection.MapOps.apply(Map.scala:176) [info] at scala.collection.MapOps.apply$(Map.scala:175) [info] at scala.collection.AbstractMap.apply(Map.scala:405) [info] at org.apache.spark.sql.hive.execution.HiveDDLSuite.$anonfun$new$445(HiveDDLSuite.scala:3275) [info] at org.apache.spark.sql.test.SQLTestUtilsBase.withUserDefinedFunction(SQLTestUtils.scala:256) [info] at org.apache.spark.sql.test.SQLTestUtilsBase.withUserDefinedFunction$(SQLTestUtils.scala:254) [info] at org.apache.spark.sql.execution.command.DDLSuite.withUserDefinedFunction(DDLSuite.scala:326) [info] at org.apache.spark.sql.hive.execution.HiveDDLSuite.$anonfun$new$444(HiveDDLSuite.scala:3267)

I manually printed the contents of spark.sparkContext.addedJars and it's an empty Map.

But when I execute:

build/sbt "hive/testOnly org.apache.spark.sql.hive.execution.SQLQuerySuite org.apache.spark.sql.hive.execution.HiveDDLSuite" -Phive

all tests pass, and the content of spark.sparkContext.addedJars is:

Map(default -> Map(spark://localhost:54875/jars/SPARK-21101-1.0.jar -> 1701676986594, spark://localhost:54875/jars/hive-contrib-2.3.9.jar -> 1701676944590, spark://localhost:54875/jars/TestUDTF.jar -> 1701676921340))

In GitHub Action tests, SQLQuerySuite does indeed execute before HiveDDLSuite, so the failure is not reproduced.

So in this PR, I added a case match to only execute remove when default is not None.

I didn't directly clear all the contents in spark.sparkContext.allAddedJars after SQLQuerySuite in this PR, because I'm not sure about the impact of this behavior.

Do you think this is ok? Or do you have any better suggestions?

The change seems fine. Can we do as below though?

foreach(k => spark.sparkContext.addedJars.get("default").foreach(_.remove(k)))

fine to me ~

dongjoon-hyun

+1, LGTM.

dongjoon-hyun · 2023-12-04T16:53:20Z

Merged to master.

LuciferYang · 2023-12-04T16:53:29Z

Thanks @dongjoon-hyun @HyukjinKwon

### What changes were proposed in this pull request? When I test `HiveDDLSuite` with ``` build/sbt "hive/testOnly org.apache.spark.sql.hive.execution.HiveDDLSuite" -Phive ``` This test throws an error: ``` [info] - SPARK-34261: Avoid side effect if create exists temporary function *** FAILED *** (4 milliseconds) [info] java.util.NoSuchElementException: key not found: default [info] at scala.collection.MapOps.default(Map.scala:274) [info] at scala.collection.MapOps.default$(Map.scala:273) [info] at scala.collection.AbstractMap.default(Map.scala:405) [info] at scala.collection.MapOps.apply(Map.scala:176) [info] at scala.collection.MapOps.apply$(Map.scala:175) [info] at scala.collection.AbstractMap.apply(Map.scala:405) [info] at org.apache.spark.sql.hive.execution.HiveDDLSuite.$anonfun$new$445(HiveDDLSuite.scala:3275) [info] at org.apache.spark.sql.test.SQLTestUtilsBase.withUserDefinedFunction(SQLTestUtils.scala:256) [info] at org.apache.spark.sql.test.SQLTestUtilsBase.withUserDefinedFunction$(SQLTestUtils.scala:254) [info] at org.apache.spark.sql.execution.command.DDLSuite.withUserDefinedFunction(DDLSuite.scala:326) [info] at org.apache.spark.sql.hive.execution.HiveDDLSuite.$anonfun$new$444(HiveDDLSuite.scala:3267) ``` I manually printed the content of `spark.sparkContext.addedJars`, which is an empty `Map`. However, when I execute ``` build/sbt "hive/testOnly org.apache.spark.sql.hive.execution.SQLQuerySuite org.apache.spark.sql.hive.execution.HiveDDLSuite" -Phive ``` All tests pass, and the content of `spark.sparkContext.addedJars` is ``` Map(default -> Map(spark://localhost:54875/jars/SPARK-21101-1.0.jar -> 1701676986594, spark://localhost:54875/jars/hive-contrib-2.3.9.jar -> 1701676944590, spark://localhost:54875/jars/TestUDTF.jar -> 1701676921340)) ``` The reason why this failure is not reproduced in the GitHub Action test is because `SQLQuerySuite` is indeed executed before `HiveDDLSuite`. So in the current PR, I change to use `.get("default").foreach(_.remove(k))` that the remove operation is only performed when `.get("default")` is not `None`. ### Why are the changes needed? Make `HiveDDLSuite` independently testable. ### Does this PR introduce _any_ user-facing change? No, just for test ### How was this patch tested? - Pass Github Actions - Manual check `HiveDDLSuite` with this pr and all test passed ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#44153 from LuciferYang/HiveDDLSuite. Authored-by: yangjie01 <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

init

cd286ec

LuciferYang marked this pull request as draft December 4, 2023 08:27

github-actions bot added the SQL label Dec 4, 2023

LuciferYang commented Dec 4, 2023

View reviewed changes

LuciferYang changed the title ~~[SPARK-46237][SQL][TESTS] Fix test failed of HiveDDLSuite~~ [SPARK-46237][SQL][TESTS] Make HiveDDLSuite independently testable Dec 4, 2023

LuciferYang assigned LuciferYang and unassigned LuciferYang Dec 4, 2023

LuciferYang marked this pull request as ready for review December 4, 2023 08:51

address comments

47e164e

HyukjinKwon approved these changes Dec 4, 2023

View reviewed changes

dongjoon-hyun approved these changes Dec 4, 2023

View reviewed changes

dongjoon-hyun closed this in 37d19b9 Dec 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-46237][SQL][TESTS] Make `HiveDDLSuite` independently testable #44153

[SPARK-46237][SQL][TESTS] Make `HiveDDLSuite` independently testable #44153

Uh oh!

LuciferYang commented Dec 4, 2023 •

edited

Loading

Uh oh!

LuciferYang Dec 4, 2023 •

edited

Loading

Uh oh!

HyukjinKwon Dec 4, 2023

Uh oh!

LuciferYang Dec 4, 2023

Uh oh!

dongjoon-hyun left a comment

Uh oh!

dongjoon-hyun commented Dec 4, 2023

Uh oh!

LuciferYang commented Dec 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SPARK-46237][SQL][TESTS] Make HiveDDLSuite independently testable #44153

[SPARK-46237][SQL][TESTS] Make HiveDDLSuite independently testable #44153

Uh oh!

Conversation

LuciferYang commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

LuciferYang Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Dec 4, 2023

Choose a reason for hiding this comment

Uh oh!

LuciferYang Dec 4, 2023

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Dec 4, 2023

Uh oh!

LuciferYang commented Dec 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SPARK-46237][SQL][TESTS] Make `HiveDDLSuite` independently testable #44153

[SPARK-46237][SQL][TESTS] Make `HiveDDLSuite` independently testable #44153

LuciferYang commented Dec 4, 2023 •

edited

Loading

LuciferYang Dec 4, 2023 •

edited

Loading