knkarthik01 commented on code in PR #59743:
URL: https://github.com/apache/spark/pull/59743#discussion_r4383647381
##########
sql/core/src/test/scala/org/apache/spark/sql/SparkSessionExtensionSuite.scala:
##########
@@ -513,6 +514,52 @@ class SparkSessionExtensionSuite extends PlanTest with
AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-60483: use the operator default list ahead of user extensions") {
+ val session = SparkSession.builder()
+ .master("local[1]")
+ .config(DEFAULT_EXTENSIONS_LIST, classOf[MyExtensions].getCanonicalName)
+ .config(SPARK_SESSION_EXTENSIONS.key,
classOf[MyExtensions2].getCanonicalName)
+ .getOrCreate()
+ try {
+ val orderedRules = Seq(MyRule(session), MyRule2(session))
+ val orderedCheckRules = Seq(MyCheckRule(session), MyCheckRule2(session))
+
assert(session.sessionState.analyzer.extendedResolutionRules.containsSlice(orderedRules))
+
assert(session.sessionState.analyzer.extendedCheckRules.containsSlice(orderedCheckRules))
+ assert(session.sessionState.planner.strategies.containsSlice(
+ Seq(MySparkStrategy(session), MySparkStrategy2(session))))
+ } finally {
+ stop(session)
+ }
+ }
+
+ test("SPARK-60483: use the operator default list when user extensions are
unset") {
+ val session = SparkSession.builder()
+ .master("local[1]")
+ .config(DEFAULT_EXTENSIONS_LIST, classOf[MyExtensions].getCanonicalName)
+ .getOrCreate()
+ try {
+
assert(session.sessionState.analyzer.extendedResolutionRules.contains(MyRule(session)))
+
assert(session.sessionState.planner.strategies.contains(MySparkStrategy(session)))
+ } finally {
+ stop(session)
+ }
+ }
+
+ test("SPARK-60483: keep an extension listed by both the operator and the
user") {
+ // Applied twice, matching the existing contract for duplicated user
extensions.
Review Comment:
Adding one data point on the duplicate case @sarutak raised. On Spark 4.0.2
with Iceberg 1.10.0, I set `spark.sql.extensions` to
`IcebergSparkSessionExtensions` listed twice, which is what the migration case
produces with this PR. Then I ran `ALTER TABLE ... WRITE ORDERED BY`, `CALL
system.rewrite_data_files` and `MERGE INTO`. All three worked, and the table
ended up the same as with Iceberg listed once. So keeping concatenation and
documenting it, as suggested above, looks safe at least for Iceberg. I also ran
this branch locally and `SparkSessionExtensionSuite` passes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
