cloud-fan commented on a change in pull request #28318:
URL: https://github.com/apache/spark/pull/28318#discussion_r414580897
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -31,53 +31,44 @@ object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
LegacyBehaviorPolicy.withName(SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_POLICY))
match {
case LegacyBehaviorPolicy.EXCEPTION =>
- assertNoNameConflictsInCTE(plan, inTraverse = false)
- traverseAndSubstituteCTE(plan, inTraverse = false)
+ assertNoNameConflictsInCTE(plan)
+ traverseAndSubstituteCTE(plan)
case LegacyBehaviorPolicy.LEGACY =>
legacyTraverseAndSubstituteCTE(plan)
case LegacyBehaviorPolicy.CORRECTED =>
- traverseAndSubstituteCTE(plan, inTraverse = false)
+ traverseAndSubstituteCTE(plan)
}
}
/**
* Check the plan to be traversed has naming conflicts in nested CTE or not,
traverse through
- * child, innerChildren and subquery for the current plan.
+ * child, innerChildren and subquery expressions for the current plan.
*/
private def assertNoNameConflictsInCTE(
plan: LogicalPlan,
- inTraverse: Boolean,
- cteNames: Set[String] = Set.empty): Unit = {
- plan.foreach {
+ outerCTERelationNames: Set[String] = Set.empty,
+ namesInSubqueries: Set[String] = Set.empty): Unit = {
+ plan match {
case w @ With(child, relations) =>
val newNames = relations.map {
case (cteName, _) =>
- if (cteNames.contains(cteName)) {
+ if (outerCTERelationNames.contains(cteName)) {
throw new AnalysisException(s"Name $cteName is ambiguous in
nested CTE. " +
s"Please set ${LEGACY_CTE_PRECEDENCE_POLICY.key} to CORRECTED
so that name " +
"defined in inner CTE takes precedence. If set it to LEGACY,
outer CTE " +
"definitions will take precedence. See more details in
SPARK-28228.")
} else {
cteName
}
- }.toSet
- child.transformExpressions {
- case e: SubqueryExpression =>
- assertNoNameConflictsInCTE(e.plan, inTraverse = true, cteNames ++
newNames)
- e
- }
- w.innerChildren.foreach { p =>
- assertNoNameConflictsInCTE(p, inTraverse = true, cteNames ++
newNames)
- }
-
- case other if inTraverse =>
- other.transformExpressions {
- case e: SubqueryExpression =>
- assertNoNameConflictsInCTE(e.plan, inTraverse = true, cteNames)
- e
- }
+ }.toSet ++ namesInSubqueries
+ assertNoNameConflictsInCTE(child, outerCTERelationNames, newNames)
+ w.innerChildren.foreach(assertNoNameConflictsInCTE(_, newNames,
newNames))
- case _ =>
+ case other =>
+ other.subqueries.foreach(
+ assertNoNameConflictsInCTE(_, namesInSubqueries, namesInSubqueries))
Review comment:
shouldn't this be `assertNoNameConflictsInCTE(_, namesInSubqueries,
Nil)`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
