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


##########
ql/src/java/org/apache/hadoop/hive/ql/QueryProperties.java:
##########
@@ -49,6 +49,10 @@ public class QueryProperties {
   boolean hasJoinFollowedByGroupBy = false;
   boolean hasPTF = false;
   boolean hasWindowing = false;
+  boolean hasQualify = false;
+

Review Comment:
   nit: space not necessary



##########
ql/src/java/org/apache/hadoop/hive/ql/QueryProperties.java:
##########
@@ -302,6 +330,10 @@ public void clear() {
     hasJoinFollowedByGroupBy = false;
     hasPTF = false;
     hasWindowing = false;
+    hasQualify = false;
+

Review Comment:
   nit: space not necessary



##########
ql/src/test/queries/clientnegative/cbo_fallback_skip_with_except.q:
##########
@@ -0,0 +1,2 @@
+set hive.cbo.fallback.strategy=CONSERVATIVE;
+select cast(0 as bigint) = '1' except select cast(1 as bigint) = '1';

Review Comment:
   Let's just keep this q test and remove the qualify which is more involved.



##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -683,15 +683,19 @@ void doPhase1QBExpr(ASTNode ast, QBExpr qbexpr, String 
id, String alias, boolean
         qbexpr.setOpcode(QBExpr.Opcode.UNION);
         break;
       case HiveParser.TOK_INTERSECTALL:
+        queryProperties.setHasIntersect(true);
         qbexpr.setOpcode(QBExpr.Opcode.INTERSECTALL);
         break;
       case HiveParser.TOK_INTERSECTDISTINCT:
+        queryProperties.setHasIntersect(true);
         qbexpr.setOpcode(QBExpr.Opcode.INTERSECT);
         break;
       case HiveParser.TOK_EXCEPTALL:
+        queryProperties.setHasExcept(true);
         qbexpr.setOpcode(QBExpr.Opcode.EXCEPTALL);
         break;
       case HiveParser.TOK_EXCEPTDISTINCT:
+        queryProperties.setHasExcept(true);

Review Comment:
   Can you please add a few unit tests for the analyzer to test that the query 
properties are set correctly for the different types of queries. You can get 
some inspiration from `TestSemanticAnalyzer` and similar classes.



-- 
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: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to