You are viewing a plain text version of this content. The canonical link for it is here.

Posted to gitbox@hive.apache.org by "Indhumathi27 (via GitHub)" <gi...@apache.org> on 2025/01/16 13:18:09 UTC

[PR] HIVE-28705: Fix Data Inconsistency Due to missing 'IS NOT? TRUE/FALSE parsing in Partition Filter Pruning [hive]

Indhumathi27 opened a new pull request, #5614:
URL: https://github.com/apache/hive/pull/5614

### What changes were proposed in this pull request?
Added missing support for parsing '**IS NOT? TRUE/FALSE**' conditions in partition filter pruning. Specifically, it handles cases where partition filters with boolean expressions such as IS NOT TRUE or IS NOT FALSE are involved, ensuring correct partition pruning behaviour.

### Why are the changes needed?
These changes address data inconsistencies in queries involving partition filter pushdown to the Hive Metastore (HMS), especially when dealing with boolean expressions like **IS NOT? TRUE** or **IS NOT? FALSE**. Previously, the partition pruning logic failed to correctly interpret these expressions, leading to incorrect results and inefficient query execution. With the new parsing logic, partition pruning now handles these complex boolean conditions properly, ensuring accurate data retrieval and better query performance.

### Does this PR introduce _any_ user-facing change?
No

### Is the change a dependency upgrade?
No

### How was this patch tested?
Test scenarios added

--
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-unsubscribe@hive.apache.org

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

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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947839147


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");
+    if (isNot) expectedValue = !expectedValue;
+
+    // Return the node based on the expected boolean value (negated or not)
+    return expectedValue ? innerNode : negateTree(innerNode);
+  }
+
+  private TreeNode negateTree(TreeNode node) {
+    if (node instanceof LeafNode) {
+      // Negate leaf nodes directly
+      return negateLeafNode((LeafNode) node);
+    } else if (node != null) {
+      // Negate logical nodes (AND/OR) recursively
+      TreeNode negatedLeft = negateTree(node.getLhs());
+      TreeNode negatedRight = negateTree(node.getRhs());
+      LogicalOperator negatedOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      return combineNodes(negatedLeft, negatedOperator, negatedRight);
+    }
+    throw new IllegalArgumentException("Unknown TreeNode type");
+  }
+
+  private LeafNode negateLeafNode(LeafNode leaf) {
+    LeafNode negatedLeaf = new LeafNode();
+    negatedLeaf.keyName = leaf.keyName;
+
+    // Invert the operator for the leaf node
+    negatedLeaf.operator = invertOperator(leaf.operator);
+    negatedLeaf.value = leaf.value;
+    return negatedLeaf;
+  }
+
+  private TreeNode combineNodes(TreeNode lhs, LogicalOperator operator, TreeNode rhs) {
+    // Create a logical node combining two child nodes
+    return new TreeNode(lhs, operator, rhs);
+  }
+
+  @Override
+  public TreeNode visitInConditionWithBoolean(PartitionFilterParser.InConditionWithBooleanContext ctx) {
+    // Extract the values from the constant sequence
+    List<Object> values = visitConstantSeq(ctx.constantSeq());
+
+    // Determine if the condition is a NOT IN by checking the presence of the NOT keyword
+    boolean isNotIn = ctx.NOT() != null;
+
+    // Extract the key name (identifier)
+    String keyName = (String) visit(ctx.key);
+
+    // Extract the boolean literal (TRUE or FALSE)
+    boolean isTrue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");
+
+    // If the boolean literal is TRUE, the condition should be IN, otherwise NOT IN
+    if (isTrue) {
+      return buildInCondition(keyName, values, isNotIn);
+    } else {
+      return buildInCondition(keyName, values, !isNotIn); // Flip isNotIn for FALSE
+    }
+  }
+
+  private TreeNode buildInCondition(String keyName, List<Object> values, boolean isNotIn) {
+    // Create nodes based on the values and operator (EQUALS or NOTEQUALS)
+    List<LeafNode> nodes = values.stream()
+            .map(value -> {
+              LeafNode leafNode = new LeafNode();
+              leafNode.keyName = keyName;
+              leafNode.value = value;
+              // Set operator based on whether it is NOT IN or IN
+              leafNode.operator = isNotIn ? Operator.NOTEQUALS : Operator.EQUALS;
+              return leafNode;
+            })
+            .collect(Collectors.toList());
+
+    // Combine the nodes using AND for NOT IN and OR for IN
+    return buildInTreeFromNodes(nodes, isNotIn ? LogicalOperator.AND : LogicalOperator.OR);
+  }
+
+  private TreeNode buildInTreeFromNodes(List<LeafNode> nodes, LogicalOperator operator) {

Review Comment:
   how is that different from `buildTreeFromNodes`?



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "kokila-19 (via GitHub)" <gi...@apache.org>.

kokila-19 commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1945236171


##########
ql/src/test/queries/clientpositive/casewhen_isnot_boolean_partition.q:
##########
@@ -0,0 +1,77 @@
+create table t1(id int) partitioned by (month int, year string);
+insert into t1 select 2,2,'2020';
+insert into t1 select 3,3,'2024';
+insert into t1 select 4,4,'2020';
+
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;
+
+set hive.cbo.enable=false;
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;

Review Comment:
   Can you add test case for validating the third expression you have added in PartitionFilter.g4 
   `    | LPAREN key=identifier RPAREN NOT? IN LPAREN values=constantSeq RPAREN IS NOT? booleanLiteral  # inConditionWithBoolean`
   ?
   



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1948494423


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:

Review Comment:
   booleanLiteral is just a string variable name which is used in G4 file



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2647966520

   Thanks @Indhumathi27 for the review and suggestions. I have handled the same


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2635995325

   @deniskuzZ @okumin 
   Can you help to review and merge this PR. Thanks


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947834310


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");

Review Comment:
   use `Boolean.parseBoolean()`
   ````
   String booleanLiteral = ctx.booleanLiteral().getText();
   
   return (isNot && Boolean.parseBoolean(booleanLiteral)) ?
       negateTree(innerNode) : innerNode;
   ````



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2652480673

   @deniskuzZ Pipeline is green. Can you help to merge. Thanks


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "okumin (via GitHub)" <gi...@apache.org>.

okumin commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2618393173

   @Indhumathi27 By the way, you may update either the title of this pull request or the title of the JIRA ticket
   https://issues.apache.org/jira/browse/HIVE-28705


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.

sonarqubecloud[bot] commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2648757209

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614)
   
   


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947838369


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");
+    if (isNot) expectedValue = !expectedValue;
+
+    // Return the node based on the expected boolean value (negated or not)
+    return expectedValue ? innerNode : negateTree(innerNode);
+  }
+
+  private TreeNode negateTree(TreeNode node) {
+    if (node instanceof LeafNode) {
+      // Negate leaf nodes directly
+      return negateLeafNode((LeafNode) node);
+    } else if (node != null) {
+      // Negate logical nodes (AND/OR) recursively
+      TreeNode negatedLeft = negateTree(node.getLhs());
+      TreeNode negatedRight = negateTree(node.getRhs());
+      LogicalOperator negatedOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      return combineNodes(negatedLeft, negatedOperator, negatedRight);
+    }
+    throw new IllegalArgumentException("Unknown TreeNode type");
+  }
+
+  private LeafNode negateLeafNode(LeafNode leaf) {
+    LeafNode negatedLeaf = new LeafNode();
+    negatedLeaf.keyName = leaf.keyName;
+
+    // Invert the operator for the leaf node
+    negatedLeaf.operator = invertOperator(leaf.operator);
+    negatedLeaf.value = leaf.value;
+    return negatedLeaf;
+  }
+
+  private TreeNode combineNodes(TreeNode lhs, LogicalOperator operator, TreeNode rhs) {
+    // Create a logical node combining two child nodes
+    return new TreeNode(lhs, operator, rhs);
+  }
+
+  @Override
+  public TreeNode visitInConditionWithBoolean(PartitionFilterParser.InConditionWithBooleanContext ctx) {

Review Comment:
   `visitInConditionWithBoolean` is mostly a duplicate of the existing `visitInCondition`, instead of making the code generic and overriding `isPositive` flag, you created multiple code duplicates



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.

sonarqubecloud[bot] commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2652060936

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614)
   
   


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "okumin (via GitHub)" <gi...@apache.org>.

okumin commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2618153564

   I'm willing to review this PR but cannot immediately do so. I haven't used HMS for years and need more time to familiarize myself.


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1949032895


##########
ql/src/test/queries/clientpositive/casewhen_isnot_boolean_partition.q:
##########
@@ -0,0 +1,77 @@
+create table t1(id int) partitioned by (month int, year string);
+insert into t1 select 2,2,'2020';
+insert into t1 select 3,3,'2024';
+insert into t1 select 4,4,'2020';
+
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;
+
+set hive.cbo.enable=false;
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;

Review Comment:
   Added those scenarios



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.

sonarqubecloud[bot] commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2596583733

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614)
   
   


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.

sonarqubecloud[bot] commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2647879200

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [7 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614)
   
   


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2650259124

   /piper build pr


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1948981279


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -132,9 +132,11 @@ public TreeNode visitBetweenCondition(PartitionFilterParser.BetweenConditionCont
 
   @Override
   public TreeNode visitInCondition(PartitionFilterParser.InConditionContext ctx) {
-    List<Object> values = visitConstantSeq(ctx.constantSeq());
     boolean isPositive = ctx.NOT() == null;
-    String keyName = (String) visit(ctx.key);
+    return buildInCondition(visitConstantSeq(ctx.constantSeq()), (String) visit(ctx.key), isPositive);
+  }
+
+  private TreeNode buildInCondition(List<Object> values, String keyName, boolean isPositive) {

Review Comment:
   should we reverse the order of args, first `keyName` and then `values`?



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947834358


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {

Review Comment:
   `visitBooleanWrappedExpression`



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947832638


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:

Review Comment:
   you've defined `booleanLiteral` as TRUE/FALSE, how could it be something else?`
   https://github.com/apache/hive/pull/5614/files#diff-4f2ef6e04f868805ddc84ef7500193d1004c01a821805d1593c073f0e6e65c20R93
   
   ````
   return (isNegated && Boolean.parseBoolean(booleanLiteral)) ? 
       negateTreeNode(exprNode) : exprNode;
   ````



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947840629


##########
ql/src/test/queries/clientpositive/casewhen_isnot_boolean_partition.q:
##########
@@ -0,0 +1,77 @@
+create table t1(id int) partitioned by (month int, year string);
+insert into t1 select 2,2,'2020';
+insert into t1 select 3,3,'2024';
+insert into t1 select 4,4,'2020';
+
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;
+
+set hive.cbo.enable=false;
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;

Review Comment:
   how about `NOT IN (constantSeq) IS NOT booleanLiteral` ?



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2602747449

   @ayushtkn @deniskuzZ @okumin Can you help to review the PR


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1949033382


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:

Review Comment:
   Got it. Handled the comments



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1948980217


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -132,9 +132,11 @@ public TreeNode visitBetweenCondition(PartitionFilterParser.BetweenConditionCont
 
   @Override
   public TreeNode visitInCondition(PartitionFilterParser.InConditionContext ctx) {
-    List<Object> values = visitConstantSeq(ctx.constantSeq());
     boolean isPositive = ctx.NOT() == null;
-    String keyName = (String) visit(ctx.key);
+    return buildInCondition(visitConstantSeq(ctx.constantSeq()), (String) visit(ctx.key), isPositive);

Review Comment:
   please keep `keyName` and `values` as local vars. 



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947834310


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");

Review Comment:
   use `Boolean.parseBoolean()`



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947836377


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");
+    if (isNot) expectedValue = !expectedValue;
+
+    // Return the node based on the expected boolean value (negated or not)
+    return expectedValue ? innerNode : negateTree(innerNode);
+  }
+
+  private TreeNode negateTree(TreeNode node) {

Review Comment:
   `negateTree` seems to be an exact copy of `negateTreeNode`, except it doesn't modify the LeafNode, but creates a negated clone.  
   Why do we need both, can we refactor?



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");
+    if (isNot) expectedValue = !expectedValue;
+
+    // Return the node based on the expected boolean value (negated or not)
+    return expectedValue ? innerNode : negateTree(innerNode);
+  }
+
+  private TreeNode negateTree(TreeNode node) {

Review Comment:
   `negateTree` seems to be an exact copy of `negateTreeNode`, except it doesn't modify the LeafNode, but creates a negated clone.  
   
   Why do we need both, can we refactor?



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947832638


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:

Review Comment:
   you've defined `booleanLiteral` as TRUE/FALSE, how could it be something else?`
   https://github.com/apache/hive/pull/5614/files#diff-4f2ef6e04f868805ddc84ef7500193d1004c01a821805d1593c073f0e6e65c20R93



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947831043


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {

Review Comment:
   could we rename `visitBooleanCondition` 



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.

sonarqubecloud[bot] commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2650623347

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5614&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5614&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5614&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5614)
   
   


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on PR #5614:
URL: https://github.com/apache/hive/pull/5614#issuecomment-2617963751

   @zabetak Can you help to review the PR. Thanks


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "Indhumathi27 (via GitHub)" <gi...@apache.org>.

Indhumathi27 commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1946765616


##########
ql/src/test/queries/clientpositive/casewhen_isnot_boolean_partition.q:
##########
@@ -0,0 +1,77 @@
+create table t1(id int) partitioned by (month int, year string);
+insert into t1 select 2,2,'2020';
+insert into t1 select 3,3,'2024';
+insert into t1 select 4,4,'2020';
+
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;
+
+set hive.cbo.enable=false;
+select id from t1 where (CASE WHEN (month=3) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3) THEN false else true END);
+select id from t1 where (CASE WHEN (year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 and year='2024') THEN false else true END);
+
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN true else false END);
+select id from t1 where (CASE WHEN (month=3 or month=4) THEN false else true END);
+
+select id from t1 where (month=3) is not true;
+select id from t1 where (month=3) is true;
+select id from t1 where (month=3) is not false;
+select id from t1 where (month=3) is false;
+
+select id from t1 where (month=3 or month=4) is not true;
+select id from t1 where (month=3 or month=4) is true;
+select id from t1 where (month=3 or month=4) is not false;
+select id from t1 where (month=3 or month=4) is false;
+
+select id from t1 where (month=3 and id=3) is not true;
+select id from t1 where (month=3 and id=3) is true;
+select id from t1 where (month=3 and id=3) is not false;
+select id from t1 where (month=3 and id=3) is false;
+
+select id from t1 where (month=3) and (id=3) is not true;
+select id from t1 where (month=3) and (id=3) is true;
+select id from t1 where (month=3) and (id=3) is not false;
+select id from t1 where (month=3) and (id=3) is false;
+
+select id from t1 where ((month=3 or month=4) and year='2024') is not true;
+select id from t1 where ((month=3 or month=4) and year='2024') is true;
+select id from t1 where ((month=3 or month=4) and year='2024') is not false;
+select id from t1 where ((month=3 or month=4) and year='2024') is false;

Review Comment:
   CASE WHEN (month=3 or month=4) THEN true else false END) - this case will cover this scenario. It is added in the qfile



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ merged PR #5614:
URL: https://github.com/apache/hive/pull/5614


-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947832638


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:

Review Comment:
   you've defined `booleanLiteral` as TRUE/FALSE, how could it be something else?



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1948704261


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:

Review Comment:
   isn't it restricted in G4 file to just TRUE/FALSE?



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947834310


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");

Review Comment:
   use `Boolean.parseBoolean()`
   ````
   // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
   String booleanLiteral = ctx.booleanLiteral().getText();
   
   // Return the node based on the expected boolean value (negated or not)
   return (isNot && Boolean.parseBoolean(booleanLiteral)) ?
       negateTree(innerNode) : innerNode;
   ````



-- 
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-unsubscribe@hive.apache.org

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


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

Re: [PR] HIVE-28705: Fix Data Inconsistency due to missing 'IS NOT? TRUE/FALSE' parsing in Partition Filter Pruning [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.

deniskuzZ commented on code in PR #5614:
URL: https://github.com/apache/hive/pull/5614#discussion_r1947836377


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/PartFilterVisitor.java:
##########
@@ -262,4 +262,163 @@ public String visitQuotedIdentifier(PartitionFilterParser.QuotedIdentifierContex
     return StringUtils.replace(ctx.getText().substring(1, ctx.getText().length() -1 ), "``", "`");
   }
 
+  @Override
+  public TreeNode visitConditionIsBoolean(PartitionFilterParser.ConditionIsBooleanContext ctx) {
+    TreeNode exprNode = (TreeNode) visit(ctx.expression());
+    boolean isNegated = ctx.NOT() != null;  // Check for negation (NOT)
+
+    // Retrieve the boolean literal (TRUE/FALSE) and handle negation
+    String booleanLiteral = ctx.booleanLiteral().getText();
+    switch (booleanLiteral.toUpperCase()) {
+      case "TRUE":
+        // For TRUE: return expression directly if not negated, otherwise negate it
+        return isNegated ? negateTreeNode(exprNode) : exprNode;
+      case "FALSE":
+        // For FALSE: return negated expression if not negated, otherwise return as is
+        return isNegated ? exprNode : negateTreeNode(exprNode);
+      default:
+        throw new ParseCancellationException("Unexpected boolean literal: " + booleanLiteral);
+    }
+  }
+
+  private TreeNode negateTreeNode(TreeNode node) {
+    if (node == null) {
+      throw new IllegalArgumentException("TreeNode cannot be null.");
+    }
+
+    if (node instanceof LeafNode) {
+      // If the node is a leaf, negate the operator
+      LeafNode leaf = (LeafNode) node;
+      leaf.operator = invertOperator(leaf.operator);
+      return leaf;
+    } else {
+      // Negate logical operators (AND/OR)
+      LogicalOperator newOperator = (node.getAndOr() == LogicalOperator.AND)
+              ? LogicalOperator.OR
+              : LogicalOperator.AND;
+      TreeNode leftNegated = negateTreeNode(node.getLhs());
+      TreeNode rightNegated = negateTreeNode(node.getRhs());
+      return new TreeNode(leftNegated, newOperator, rightNegated);
+    }
+  }
+
+  private Operator invertOperator(Operator operator) {
+    switch (operator) {
+      case EQUALS:
+        return Operator.NOTEQUALS;
+      case NOTEQUALS:
+      case NOTEQUALS2:
+        return Operator.EQUALS;
+      case GREATERTHAN:
+        return Operator.LESSTHANOREQUALTO;
+      case LESSTHAN:
+        return Operator.GREATERTHANOREQUALTO;
+      case GREATERTHANOREQUALTO:
+        return Operator.LESSTHAN;
+      case LESSTHANOREQUALTO:
+        return Operator.GREATERTHAN;
+      case LIKE:
+        throw new UnsupportedOperationException("LIKE operator inversion is not supported.");
+      default:
+        throw new IllegalArgumentException("Unsupported operator for inversion: " + operator.getOp());
+    }
+  }
+
+  @Override
+  public TreeNode visitWrappedExpressionIsBoolean(PartitionFilterParser.WrappedExpressionIsBooleanContext ctx) {
+    // Visit the inner expression and check if "NOT" is used
+    TreeNode innerNode = (TreeNode) visit(ctx.orExpression());
+    boolean isNot = ctx.NOT() != null;
+
+    // Determine the expected boolean value (TRUE/FALSE) and negate if necessary
+    boolean expectedValue = ctx.booleanLiteral().getText().equalsIgnoreCase("TRUE");
+    if (isNot) expectedValue = !expectedValue;
+
+    // Return the node based on the expected boolean value (negated or not)
+    return expectedValue ? innerNode : negateTree(innerNode);
+  }
+
+  private TreeNode negateTree(TreeNode node) {

Review Comment:
   `negateTree` seems to be a duplicate of `negateTreeNode`, except it doesn't modify the existing tree, but a negated copy.  



-- 
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-unsubscribe@hive.apache.org

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


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