JingsongLi commented on code in PR #9427:
URL: https://github.com/apache/paimon/pull/9427#discussion_r3912060132
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/PredicateConverter.java:
##########
@@ -74,74 +74,116 @@ 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
Review Comment:
[P1] Keep every negated floating predicate with Java-incompatible ordering
residual
The guard in `visitComparison()` covers only negated inequalities, but these
new negated equality, IN, and BETWEEN branches still use Paimon's
`Float/Double.compareTo` total order even though this PR's test at lines
478-484 explicitly confirms it differs from Flink's Java operators. This yields
false negatives: `NOT (d = NaN)` and `d NOT IN (NaN)` should keep a NaN row but
Paimon considers NaN equal to NaN; `d NOT BETWEEN 1.0 AND NaN` with `d=2.0`
should be true but Paimon `NotBetween` is false; `NOT (d <> 0.0)` should keep
`-0.0`, while `Double.compare` distinguishes signed zero. Once pushdown drops
these rows, a Flink residual cannot recover them. Please apply the
floating-point soundness fallback to every newly negated comparison family and
add source/IT coverage for NaN and both signed zeros.
--
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]
