cloud-fan commented on code in PR #58856:
URL: https://github.com/apache/spark/pull/58856#discussion_r4113741401
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/Resolver.scala:
##########
@@ -78,6 +78,10 @@ import org.apache.spark.sql.errors.{QueryCompilationErrors,
QueryErrorsBase}
* relation operators in [[MetadataResolver]].
* @param externalRelationResolution An optional [[RelationResolution]] with
to override the default
* one. The default is constructed using
[[Resolver.createRelationResolution]].
+ * @param hintResolutionRules Single-pass hint resolution rules, applied
before relation metadata
+ * is resolved. Defaults to the fixed-point Analyzer's
`injectHintResolutionRule` sequence;
Review Comment:
**Nit (P3):** `hintResolutionRules` does not default to the Analyzer's
registered sequence here: the constructor below defaults it to `Seq.empty`, and
only HybridAnalyzer supplies `singlePassHintResolutionRules` explicitly. Please
describe the direct constructor default separately from the Analyzer-level
default that HybridAnalyzer propagates.
**Verification:**
- **Inspection:** Compare the parameter text with the constructor default
expression and HybridAnalyzer call site and confirm that each default is
attributed to its actual owner.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/HintResolutionRunner.scala:
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.analysis.resolver
+
+import org.apache.spark.sql.catalyst.SQLConfHelper
+import org.apache.spark.sql.catalyst.analysis.ResolveHints
+import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
+import org.apache.spark.sql.catalyst.plans.logical.{AnalysisHelper,
LogicalPlan, UnresolvedWith}
+import org.apache.spark.sql.catalyst.rules.{Rule, RuleExecutor}
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
+import org.apache.spark.sql.catalyst.trees.TreePattern.{PLAN_EXPRESSION,
UNRESOLVED_WITH}
+import org.apache.spark.sql.internal.SQLConf
+
+/**
+ * Utility wrapper on top of [[RuleExecutor]], used to apply the hint
resolution rules injected
+ * through `SparkSessionExtensions.injectHintResolutionRule` on the unresolved
plan.
+ *
+ * The fixed-point Analyzer runs these rules in its "Hints" batch, which
completes before the
+ * "Resolution" batch, so the rules always observe an unresolved plan. The
single-pass Resolver
+ * has to run them before the [[MetadataResolver]] pre-pass for the same
reason: the pre-pass
+ * resolves relation metadata eagerly (which, for path-based relations, lists
files), therefore
+ * rules that rewrite [[UnresolvedRelation]]s have to run ahead of it.
+ *
+ * The "Disable Hints" batch precedes the "Hints" batch, exactly like in the
fixed-point Analyzer,
+ * so that `spark.sql.optimizer.disableHints` keeps hiding the hints from the
injected rules.
+ *
+ * The rules are applied on the main plan, the CTE definitions and the
subquery plans. The
+ * fixed-point Analyzer covers those same subtrees, through two different
mechanisms:
+ * - `CTESubstitution` merges the CTE definitions into the main plan in the
"Substitution" batch,
+ * which runs before the "Hints" batch, so the rules observe them as part
of the main plan;
+ * - subquery plans are resolved by `ResolveSubquery` re-entering the whole
Analyzer through
+ * `executeSameContext`, which replays every batch, "Hints" included.
+ *
+ * View bodies are covered for the same reason: the fixed-point Analyzer
resolves them by
+ * re-entering the Analyzer (`ViewResolution.resolve` is called with
`executeSameContext`), and the
+ * single-pass Resolver calls [[Resolver.lookupMetadataAndResolve]], and
therefore this runner, once
+ * per unresolved [[View]].
+ */
+class HintResolutionRunner(hintResolutionRules: Seq[Rule[LogicalPlan]])
+ extends ResolverMetricTracker
+ with SQLConfHelper {
+
+ private lazy val hintResolver = new RuleExecutor[LogicalPlan] {
+ override def batches: Seq[Batch] =
+ Seq(
+ Batch("Disable Hints", Once, new ResolveHints.DisableHints),
+ Batch(
+ "Hints",
+ FixedPoint(
+ conf.analyzerMaxIterations,
+ errorOnExceed = true,
+ maxIterationsSetting = SQLConf.ANALYZER_MAX_ITERATIONS.key
+ ),
+ hintResolutionRules: _*
+ )
+ )
+ }
+
+ /**
+ * Applies the hint resolution rules by first recursing into the CTE
definitions and the
+ * subqueries and then applying the rules on the entire plan. No rules are
injected by default,
+ * in which case the plan is returned as is and the [[RuleExecutor]] is
never constructed
+ * (`hintResolver` is lazy). That also skips the "Disable Hints" batch,
which only matters for
+ * plans that still carry an [[UnresolvedHint]] - a shape the single-pass
Resolver does not
+ * support in the first place.
+ *
+ * The recursion needs [[AnalysisHelper.allowInvokingTransformsInAnalyzer]]
to rewrite the inner
+ * plans, so the rules are invoked within its scope, like the rewrite rules
in [[PlanRewriter]].
+ */
+ def resolveWithSubqueries(plan: LogicalPlan): LogicalPlan = {
+ if (hintResolutionRules.isEmpty) {
+ plan
+ } else {
+ recordProfile("resolveWithSubqueries") {
+ AnalysisHelper.allowInvokingTransformsInAnalyzer {
+ doResolveWithSubqueries(plan)
+ }
+ }
+ }
+ }
+
+ private def doResolveWithSubqueries(plan: LogicalPlan): LogicalPlan = {
+ val planWithResolvedCteDefinitions = resolveCteDefinitions(plan)
+
+ val planWithResolvedSubqueries =
+ planWithResolvedCteDefinitions.transformAllExpressionsWithPruning(
+ _.containsPattern(PLAN_EXPRESSION)
+ ) {
+ case subqueryExpression: SubqueryExpression =>
+
subqueryExpression.withNewPlan(doResolveWithSubqueries(subqueryExpression.plan))
Review Comment:
**Non-blocking (P2):** This recursive call fully applies the injected batch
to each current subquery before the containing plan reaches `hintResolver`,
which is the reverse of fixed-point Analyzer's Hints-before-ResolveSubquery
order. It also fixes the subquery inventory before outer rules run, so a
`SubqueryExpression` created by an injected rule later reaches
`Resolver.resolve` without another hint pass. Order-sensitive extensions can
therefore produce a different forced-single-pass plan or dual-run mismatch, and
rule-created subqueries can resolve relations without the registered rewrite.
**Verification:**
- **Regression:** Exercise an order-sensitive injected rule and prove that
the containing plan observes an unprocessed subquery in both analyzer paths.
- **Regression:** Exercise an injected rule that creates a subquery whose
relation requires a subsequent injected rewrite, and prove the nested rewrite
occurs before metadata lookup.
- **Compatibility:** Preserve existing behavior for main plans, pre-existing
nested CTEs and subqueries, fixed-point iteration limits, disabled hints, and
CTE alias origin and tags.
--
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]
