peter-toth commented on code in PR #50757:
URL: https://github.com/apache/spark/pull/50757#discussion_r2069403852


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -103,13 +103,21 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
     AttributeSet(expressions) -- producedAttributes)
 
   /**
-   * Returns true when the all the expressions in the current node as well as 
all of its children
+   * Returns true when the current node (all the expressions in it) is 
deterministic.
+   */
+  def deterministicNode: Boolean = _deterministicNode()
+
+  private val _deterministicNode =
+    new BestEffortLazyVal[JBoolean](() => expressions.forall(_.deterministic))
+
+  /**
+   * Returns true when all the expressions in the current node as well as all 
of its children
    * are deterministic
    */
   def deterministic: Boolean = _deterministic()
 
-  private val _deterministic = new BestEffortLazyVal[JBoolean](() =>
-    expressions.forall(_.deterministic) && children.forall(_.deterministic))
+  private val _deterministic =
+    new BestEffortLazyVal[JBoolean](() => deterministicNode && 
children.forall(_.deterministic))

Review Comment:
   `Expression.hasIndeterminism` kind of tracking in 
https://github.com/apache/spark/pull/50029 seems fragile to me because plan 
nodes can create new attributes and those should have their own 
`hasIndeterminism` logic implemented.
   E.g. if you check `UnionBase` then you will see that its output attributes 
are either inherited from the first leg or a new `AttributeReference` is 
created, but in both cases they should take into account `hasIndeterminism` of 
all legs, which is missing from https://github.com/apache/spark/pull/50029.



-- 
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

Reply via email to