Skip to content

Commit 1bc8ce0

Browse files
mihailoale-dbcloud-fan
authored andcommitted
[SPARK-53244][SQL] Don't store dual-run enabled and tentative mode enabled confs during view creation
### What changes were proposed in this pull request? If a `ANALYZER_DUAL_RUN_LEGACY_AND_SINGLE_PASS_RESOLVER` conf is explicitly set during view creation, we store it, but we might not want to. When we query that view, stored value is used, which we forbid in order to avoid any accidental failures. We do the same for `ANALYZER_SINGLE_PASS_RESOLVER_ENABLED_TENTATIVELY`. ### Why are the changes needed? To make single-pass resolver runs more stable. ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Added tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #51972 from mihailoale-db/dontstoreqanconfs. Authored-by: mihailoale-db <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
1 parent b7ada56 commit 1bc8ce0

File tree

2 files changed

+56
-4
lines changed

2 files changed

+56
-4
lines changed

sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -430,18 +430,30 @@ object ViewHelper extends SQLConfHelper with Logging {
430430
"spark.sql.hive.convertMetastoreCtas",
431431
SQLConf.ADDITIONAL_REMOTE_REPOSITORIES.key)
432432

433-
private val configAllowList = Seq(
433+
private val configAllowList = Set(
434434
SQLConf.DISABLE_HINTS.key
435435
)
436436

437+
/**
438+
* Set of single-pass resolver confs that shouldn't be stored during view/UDF/proc creation.
439+
* This is needed to avoid accidental failures in tentative and dual-run modes when querying the
440+
* view.
441+
*/
442+
private val singlePassResolverDenyList = Set(
443+
SQLConf.ANALYZER_SINGLE_PASS_RESOLVER_ENABLED_TENTATIVELY.key,
444+
SQLConf.ANALYZER_DUAL_RUN_LEGACY_AND_SINGLE_PASS_RESOLVER.key
445+
)
446+
437447
/**
438448
* Capture view config either of:
439449
* 1. exists in allowList
440450
* 2. do not exists in denyList
441451
*/
442452
private def shouldCaptureConfig(key: String): Boolean = {
443-
configAllowList.exists(prefix => key.equals(prefix)) ||
444-
!configPrefixDenyList.exists(prefix => key.startsWith(prefix))
453+
configAllowList.contains(key) || (
454+
!configPrefixDenyList.exists(prefix => key.startsWith(prefix)) &&
455+
!singlePassResolverDenyList.contains(key)
456+
)
445457
}
446458

447459
import CatalogTable._

sql/core/src/test/scala/org/apache/spark/sql/analysis/resolver/HybridAnalyzerSuite.scala

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ import org.scalactic.source.Position
2121
import org.scalatest.Tag
2222

2323
import org.apache.spark.sql.{AnalysisException, QueryTest}
24-
import org.apache.spark.sql.catalyst.{ExtendedAnalysisException, QueryPlanningTracker}
24+
import org.apache.spark.sql.catalyst.{
25+
ExtendedAnalysisException,
26+
QueryPlanningTracker,
27+
TableIdentifier
28+
}
2529
import org.apache.spark.sql.catalyst.analysis.{
2630
AnalysisContext,
2731
Analyzer,
@@ -440,7 +444,43 @@ class HybridAnalyzerSuite extends QueryTest with SharedSparkSession {
440444
}
441445
}
442446

447+
test("Tentative mode conf is not stored during view creation when explicitly set") {
448+
withSQLConf(SQLConf.ANALYZER_SINGLE_PASS_RESOLVER_ENABLED_TENTATIVELY.key -> "true") {
449+
validateConfStoredInView(
450+
conf = SQLConf.ANALYZER_SINGLE_PASS_RESOLVER_ENABLED_TENTATIVELY.key,
451+
shouldStore = false
452+
)
453+
}
454+
}
455+
456+
test("Dual-run mode conf is not stored during view creation when explicitly set") {
457+
withSQLConf(SQLConf.ANALYZER_DUAL_RUN_LEGACY_AND_SINGLE_PASS_RESOLVER.key -> "true") {
458+
validateConfStoredInView(
459+
conf = SQLConf.ANALYZER_DUAL_RUN_LEGACY_AND_SINGLE_PASS_RESOLVER.key,
460+
shouldStore = false
461+
)
462+
}
463+
}
464+
465+
test("Single-pass result conf is stored during view creation when explicitly set") {
466+
withSQLConf(SQLConf.ANALYZER_DUAL_RUN_RETURN_SINGLE_PASS_RESULT.key -> "true") {
467+
validateConfStoredInView(
468+
conf = SQLConf.ANALYZER_DUAL_RUN_RETURN_SINGLE_PASS_RESULT.key,
469+
shouldStore = true
470+
)
471+
}
472+
}
473+
443474
private def assertPlansEqual(actualPlan: LogicalPlan, expectedPlan: LogicalPlan) = {
444475
assert(NormalizePlan(actualPlan) == NormalizePlan(expectedPlan))
445476
}
477+
478+
private def validateConfStoredInView(conf: String, shouldStore: Boolean): Unit = {
479+
withView("v1") {
480+
sql("CREATE VIEW v1 AS SELECT 1")
481+
482+
val viewMetadata = spark.sessionState.catalog.getTableMetadata(TableIdentifier("v1"))
483+
assert(viewMetadata.properties.contains(s"view.sqlConfig.$conf") == shouldStore)
484+
}
485+
}
446486
}

0 commit comments

Comments
 (0)