cloud-fan commented on code in PR #49518:
URL: https://github.com/apache/spark/pull/49518#discussion_r1923141166
##########
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:
maybe we should still put a boolean flag in `CTERelationDef` to indicate
that this def is declared with the keyword `RECURSIVE`, and we only perform
validation for it.
--
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
