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