milanisvet commented on code in PR #49518:
URL: https://github.com/apache/spark/pull/49518#discussion_r1923954633
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveWithCTE.scala:
##########
@@ -49,17 +51,27 @@ object ResolveWithCTE extends Rule[LogicalPlan] {
plan.resolveOperatorsDownWithPruning(_.containsAllPatterns(CTE)) {
case withCTE @ WithCTE(_, cteDefs) =>
val newCTEDefs = cteDefs.map {
- // `cteDef.recursive` means "presence of a recursive CTERelationRef
under cteDef". The
- // side effect of node substitution below is that after
CTERelationRef substitution
- // its cteDef is no more considered `recursive`. This code path is
common for `cteDef`
- // that were non-recursive from the get go, as well as those that
are no more recursive
- // due to node substitution.
- case cteDef if !cteDef.recursive =>
+ // cteDef in the first case is either recursive and all the
recursive CTERelationRefs
+ // are already substituted to UnionLoopRef in the previous pass, or
it is not recursive
+ // at all. In both cases we need to put it in the map in case it is
resolved.
+ // Second case is performing the substitution of recursive
CTERelationRefs.
+ case cteDef if !cteDef.hasRecursiveCTERelationRef =>
Review Comment:
I suppose now after new changes, introduction of the flag is not needed any
more
--
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: reviews-unsubscr...@spark.apache.org
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
