viirya commented on a change in pull request #28407:
URL: https://github.com/apache/spark/pull/28407#discussion_r417825103



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -159,17 +146,36 @@ object CTESubstitution extends Rule[LogicalPlan] {
     }
   }
 
+  private def resolveCTERelations(
+      relations: Seq[(String, SubqueryAlias)],
+      isLegacy: Boolean): Seq[(String, LogicalPlan)] = {
+    val resolvedCTERelations = new mutable.ArrayBuffer[(String, 
LogicalPlan)](relations.size)
+    for ((name, relation) <- relations) {
+      val innerCTEResolved = if (isLegacy) {
+        // In legacy mode, outer CTE relations take precedence, so substitute 
relations later.
+        relation
+      } else {
+        // A CTE definition might contain an inner CTE that has priority, so 
traverse and
+        // substitute CTE defined in `relation` first.
+        traverseAndSubstituteCTE(relation)
+      }
+      // CTE definition can reference a previous one
+      resolvedCTERelations += (name -> substituteCTE(innerCTEResolved, 
resolvedCTERelations))

Review comment:
       For legacy case, `innerCTEResolved` might contain an inner `WITH`, but 
seems `substituteCTE` doesn't remove `WITH`.
   
   Then in later `substituteCTE`s, will we result some `WITH`s in the final 
query plan ?
   
   
   
   
   
   




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to