cloud-fan commented on code in PR #36121:
URL: https://github.com/apache/spark/pull/36121#discussion_r847529275


##########
sql/catalyst/src/main/scala-2.12/org/apache/spark/sql/catalyst/expressions/ExpressionSet.scala:
##########
@@ -55,13 +55,22 @@ object ExpressionSet {
  * For non-deterministic expressions, they are always considered as not 
contained in the [[Set]].
  * On adding a non-deterministic expression, simply append it to the original 
expressions.
  * This is consistent with how we define `semanticEquals` between two 
expressions.
+ *
+ * The constructor of this class is protected so caller can only initialize an 
Expression from
+ * empty, then build it using `add` and `remove` methods. So every instance of 
this class holds the
+ * invariant that:
+ * 1. Every expr `e` in `baseSet` satisfies `e.deterministic && 
e.canonicalized == e`
+ * 2. Every deterministic expr `e` in `originals` satisfies that 
`e.canonicalized` is already
+ *    accessed.
  */
 class ExpressionSet protected(
-    private val baseSet: mutable.Set[Expression] = new mutable.HashSet,
-    private val originals: mutable.Buffer[Expression] = new ArrayBuffer)
-  extends Iterable[Expression] {
+  private val baseSet: mutable.Set[Expression] = new mutable.HashSet,

Review Comment:
   nit: 4 spaces indentation here.



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