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-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org