zabetak commented on code in PR #2966:
URL: https://github.com/apache/hive/pull/2966#discussion_r857739177


##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java:
##########
@@ -1257,15 +1249,26 @@ public Void visitCall(RexCall call) {
     }
 
     public boolean hasDisjunction(RexNode node) {
-      // clear the state
-      inNegation = false;
-      hasDisjunction = false;
-
       node.accept(this);
       return hasDisjunction;
     }
   }
 
+  /**
+   * Find disjunction (OR) in an expression (at any level of nesting).

Review Comment:
   `Find` implies that we are going to return a disjunctive expression. 
Suggestion:
   `Returns whether the expression has disjunctions (OR) at any level of 
nesting.`



##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java:
##########
@@ -1257,15 +1249,26 @@ public Void visitCall(RexCall call) {
     }
 
     public boolean hasDisjunction(RexNode node) {
-      // clear the state
-      inNegation = false;
-      hasDisjunction = false;
-
       node.accept(this);
       return hasDisjunction;
     }
   }
 
+  /**
+   * Find disjunction (OR) in an expression (at any level of nesting).
+   *
+   * Example 1: OR(=($0, $1), IS NOT NULL($2))):INTEGER (OR in the top-level 
expression)
+   * Example 2: NOT(AND(=($0, $1), IS NOT NULL($2))
+   *   this is equivalent to OR((<>($0, $1), IS NULL($2))
+   * Example 3: AND(OR(=($0, $1), IS NOT NULL($2)))) (OR in inner expression)
+   *
+   * @param node the expression where to look for disjunctions.
+   * @return true if the given expressions contains a disjunction, false 
otherwise.
+   */
+  public static boolean hasDisjuction(RexNode node) {
+    return new DisjunctivePredicatesFinder().hasDisjunction(node);

Review Comment:
   nit to get rid of the `DisjunctivePredicateFinder#hasDisjunction` method:
   ```
   DisjunctivePredicateFinder finder = new DisjunctivePredicateFinder();
   node.accept(finder);
   return finder.hasDisjunction;
   ```



##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java:
##########
@@ -1214,6 +1214,58 @@ public FixNullabilityShuttle(RexBuilder rexBuilder,
     }
   }
 
+  /**
+   * Find disjunction (OR) in an expression (at any level of nesting).
+   *
+   * Example 1: OR(=($0, $1), IS NOT NULL($2))):INTEGER (OR in the top-level 
expression)
+   * Example 2: NOT(AND(=($0, $1), IS NOT NULL($2))
+   *   this is equivalent to OR((<>($0, $1), IS NULL($2))
+   * Example 3: AND(OR(=($0, $1), IS NOT NULL($2)))) (OR in inner expression)
+   */

Review Comment:
   I had in mind compiling with `-Pjavadoc` profile and checking for new errors 
in this class. Actually I am afraid of `<>` symbols as well as `$`. Don't 
remember if it is fine to use them like that.



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