cloud-fan commented on code in PR #58229: URL: https://github.com/apache/spark/pull/58229#discussion_r4057248704
########## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ConvertViewToMaterializedCTE.scala: ########## @@ -0,0 +1,190 @@ +/* + * 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.optimizer + +import scala.collection.mutable + +import org.apache.spark.sql.catalyst.TableIdentifier +import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute} +import org.apache.spark.sql.catalyst.plans.logical._ +import org.apache.spark.sql.catalyst.rules.Rule +import org.apache.spark.sql.internal.SQLConf + +/** + * Rewrites multiple references to the same view into a single `CTERelationDef` with multiple + * `CTERelationRef`s, so that the view's underlying plan is computed once (through exchange + * reuse at the physical layer) instead of once per reference. + * + * The rule runs in `FinishAnalysis`, immediately before `EliminateView`: after `EliminateView` + * no `View` nodes remain and every reference site holds an independent copy of the view's plan. + * + * A converted definition always sets `forceSkipInline = true`; otherwise `InlineCTE` would + * immediately flatten it back into duplicated subtrees (the definition body is deterministic + * in every case we convert), making the rule a no-op. + * + * Only deterministic, batch views are eligible: a multi-reference CTE guarantees that its + * definition is evaluated exactly once (even for non-deterministic definitions), while + * multiple references to a non-deterministic view are evaluated independently today. + * Converting such views would change query results. + * + * A view body may contain correlated subqueries whose outer references resolve to relations + * inside the same body (e.g. `t WHERE x IN (SELECT y FROM s WHERE s.k = t.k)`). The view is + * analyzed standalone when it is created, so an outer reference that does not resolve inside + * the body fails view analysis and can never escape to the outer query. The converted + * definition contains the whole body, so internal correlations resolve within it and these + * bodies are safe to convert. `InlineCTE`'s rejection of boundary-crossing outer references + * is only a generic safety net for non-view `forceSkipInline` producers. + * + * Each reference site gains a shuffle boundary added by `ReplaceCTERefWithRepartition` and + * deduplicated by exchange reuse, so the conversion trades recomputation for a + * shuffle plus reuse; it is therefore gated behind + * [[SQLConf.CONVERT_VIEW_TO_MATERIALIZED_CTE]] and off by default. + */ +object ConvertViewToMaterializedCTE extends Rule[LogicalPlan] { + + override def apply(plan: LogicalPlan): LogicalPlan = { + if (!SQLConf.get.getConf(SQLConf.CONVERT_VIEW_TO_MATERIALIZED_CTE)) return plan + val occurrences = plan.collectWithSubqueries { case v: View => v } + if (occurrences.length < 2) return plan + + // Group occurrences of the same view by their canonicalized body and captured SQL + // configs. Occurrences of one view differ only in renewed expression ids, which + // canonicalization normalizes away. + val qualifiedGroups = occurrences.groupBy(groupKey).values.filter(qualifies) + if (qualifiedGroups.isEmpty) return plan + // Match by identifier during the transform, not by the full group key: the key embeds + // the canonicalized body, and by the time an outer view is visited in the bottom-up + // traversal, nested views inside its body have already been rewritten into + // `CTERelationRef`s, so the body no longer canonicalizes to the key computed here. + // An identifier mapping to more than one qualified group would mean occurrences whose + // canonicalized bodies diverge; refuse conversion rather than rewrite all of them + // against whichever definition happens to be visited first. + val qualifiedIdentifiers = qualifiedGroups Review Comment: **Non-blocking (P2):** `qualifiedIdentifiers` is computed after non-qualifying groups have been discarded, but the later transform rewrites every `View` with an admitted identifier. If two occurrences form a qualifying group and a third same-identifier occurrence has a different or nondeterministic body, that singleton is excluded here but can still be rebound to the pair's CTE definition, changing the query result. This is reachable when repeated temporary-view lookups observe a replacement during analysis. Please admit an identifier only when every occurrence belongs to the one qualified group, and cover the qualifying-pair plus divergent-singleton case. -- 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]
