s-maling-celonis commented on code in PR #58856: URL: https://github.com/apache/spark/pull/58856#discussion_r4119751313
########## 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: Addressed in 9e80eaf4cc7: the hint batch now runs on the containing plan before CTE definitions and subquery plans, including subqueries the rules themselves create. Covered in `HintResolutionRunnerSuite` and under both analyzers in `SparkSessionExtensionSuite`. ########## 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: Addressed in 9e80eaf4cc7: the constructor parameter is documented as defaulting to an empty sequence, with `HybridAnalyzer` passing `Analyzer.singlePassHintResolutionRules` as the Analyzer-level default. -- 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]
