HyukjinKwon commented on a change in pull request #34470:
URL: https://github.com/apache/spark/pull/34470#discussion_r769254236



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala
##########
@@ -84,6 +84,13 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
     AttributeSet.fromAttributeSets(expressions.map(_.references)) -- 
producedAttributes
   }
 
+  /**
+   * Returns true when the all the expressions in the current node as well as 
all of its children
+   * are deterministic
+   */
+  lazy val deterministic: Boolean = expressions.forall(_.deterministic) &&

Review comment:
       So if we optimize something, that should always happen in optimizer with 
logical plans ... right?
   
   If we can do something with physical plans, we will have to add another 
argument for every non deterministic plan e.g.)
   
   ```diff
   case class Sample(
       lowerBound: Double,
       upperBound: Double,
       withReplacement: Boolean,
       seed: Long,
   +   deterministic: Boolean,
       child: LogicalPlan) extends UnaryNode {
   
   ```
   
   ```diff
   case class SampleExec(
       lowerBound: Double,
       upperBound: Double,
       withReplacement: Boolean,
       seed: Long,
   +   deterministic: Boolean,
       child: SparkPlan) extends UnaryExecNode with CodegenSupport {
   ```
   
   which is pretty much different from how we do in `Expression`.
   
   Otherwise, we will have to recalculate it for each plan, etc.
   




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to