JingsongLi commented on code in PR #9427:
URL: https://github.com/apache/paimon/pull/9427#discussion_r3878873279
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/PredicateConverter.java:
##########
@@ -74,74 +74,93 @@ public PredicateConverter(PredicateBuilder builder) {
@Override
public Predicate visit(CallExpression call) {
+ return visit(call, false);
+ }
+
+ private Predicate visit(CallExpression call, boolean negated) {
FunctionDefinition func = call.getFunctionDefinition();
List<Expression> children = call.getChildren();
if (func == BuiltInFunctionDefinitions.AND) {
- return PredicateBuilder.and(flattenAndConvert(children, func));
+ requireAtLeastArity(children, 2);
+ List<Predicate> predicates = flattenAndConvert(children, func,
negated);
+ return negated ? PredicateBuilder.or(predicates) :
PredicateBuilder.and(predicates);
} else if (func == BuiltInFunctionDefinitions.OR) {
- return PredicateBuilder.or(flattenAndConvert(children, func));
+ requireAtLeastArity(children, 2);
+ List<Predicate> predicates = flattenAndConvert(children, func,
negated);
+ return negated ? PredicateBuilder.and(predicates) :
PredicateBuilder.or(predicates);
+ } else if (func == BuiltInFunctionDefinitions.NOT) {
+ requireArity(children, 1);
+ return visit(children.get(0), !negated);
} else if (func == BuiltInFunctionDefinitions.EQUALS) {
- return visitBiFunction(children, builder::equal, builder::equal);
+ return negated
+ ? visitBiFunction(children, builder::notEqual,
builder::notEqual)
+ : visitBiFunction(children, builder::equal,
builder::equal);
} else if (func == BuiltInFunctionDefinitions.NOT_EQUALS) {
- return visitBiFunction(children, builder::notEqual,
builder::notEqual);
+ return negated
+ ? visitBiFunction(children, builder::equal, builder::equal)
+ : visitBiFunction(children, builder::notEqual,
builder::notEqual);
} else if (func == BuiltInFunctionDefinitions.GREATER_THAN) {
- return visitBiFunction(children, builder::greaterThan,
builder::lessThan);
+ return negated
+ ? visitBiFunction(children, builder::lessOrEqual,
builder::greaterOrEqual)
+ : visitBiFunction(children, builder::greaterThan,
builder::lessThan);
} else if (func == BuiltInFunctionDefinitions.GREATER_THAN_OR_EQUAL) {
- return visitBiFunction(children, builder::greaterOrEqual,
builder::lessOrEqual);
+ return negated
+ ? visitBiFunction(children, builder::lessThan,
builder::greaterThan)
+ : visitBiFunction(children, builder::greaterOrEqual,
builder::lessOrEqual);
} else if (func == BuiltInFunctionDefinitions.LESS_THAN) {
- return visitBiFunction(children, builder::lessThan,
builder::greaterThan);
+ return negated
+ ? visitBiFunction(children, builder::greaterOrEqual,
builder::lessOrEqual)
+ : visitBiFunction(children, builder::lessThan,
builder::greaterThan);
} else if (func == BuiltInFunctionDefinitions.LESS_THAN_OR_EQUAL) {
- return visitBiFunction(children, builder::lessOrEqual,
builder::greaterOrEqual);
+ return negated
+ ? visitBiFunction(children, builder::greaterThan,
builder::lessThan)
+ : visitBiFunction(children, builder::lessOrEqual,
builder::greaterOrEqual);
} else if (func == BuiltInFunctionDefinitions.IN) {
- FieldReferenceExpression fieldRefExpr =
-
extractFieldReference(children.get(0)).orElseThrow(UnsupportedExpression::new);
+ requireAtLeastArity(children, 2);
+ ResolvedField field = resolveField(children.get(0));
List<Object> literals = new ArrayList<>();
for (int i = 1; i < children.size(); i++) {
- literals.add(extractLiteral(fieldRefExpr.getOutputDataType(),
children.get(i)));
+
literals.add(extractLiteral(field.expression.getOutputDataType(),
children.get(i)));
}
- return builder.in(builder.indexOf(fieldRefExpr.getName()),
literals);
+ return negated
+ ? builder.notIn(field.index, literals)
Review Comment:
[P2] Short-circuit `NOT IN` lists containing NULL
Under SQL WHERE semantics, `v NOT IN (1, NULL, 3)` can never be true.
Passing the NULL literal to `builder.notIn` is also unsafe for file-index
evaluation: the BSI reader can unbox a null mapped value, and the range-bitmap
reader can pass null to its comparator, causing the query to fail when either
index is enabled. Before this change, the unsupported expression stayed as a
Flink residual filter.
Please return an always-false predicate when a negated IN list contains
NULL, or reject the conversion so it remains residual. An index-enabled
regression test would cover both paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
