JingsongLi commented on PR #10203: URL: https://github.com/apache/paimon/pull/10203#issuecomment-5950500180
[P1] Keep Flink's ordering semantics when normalizing signed zero (`CompareUtils.java:34–37`) The equality fix has a real query-correctness benefit, but changing the shared comparator for every operator introduces incorrect Flink SQL results. Flink's runtime ordering distinguishes the zeros: for a stored `-0.0`, `< 0.0` is true and `>= 0.0` is false. The new comparator makes them equal. I reproduced this with real Paimon writes and reads on Flink 1.20.1 and 2.2.0, for both DOUBLE and FLOAT, with parquet and avro: ```sql CREATE TABLE src (d DOUBLE, f FLOAT); INSERT INTO src VALUES (CAST(0 AS DOUBLE), CAST(0 AS FLOAT)); CREATE TABLE t (id INT, d DOUBLE, f FLOAT); INSERT INTO t SELECT 1, -d, -f FROM src; SELECT CAST(d AS STRING), d < 0.0 FROM t; -- (-0.0, true) SELECT id FROM t WHERE d < 0.0; -- head: empty; before change: 1 ``` `LessThan.test` on file statistics now considers the two zeros equal and prunes the matching file before Flink can evaluate its residual filter. There is also a false-positive case for fully consumed partition predicates, reproduced on Flink 1.20.1: ```sql CREATE TABLE p (id INT, d DOUBLE) PARTITIONED BY (d); INSERT INTO p SELECT 1, -d FROM src; SELECT d >= 0.0 FROM p; -- false SELECT id FROM p WHERE d >= 0.0; -- head: 1; before change: empty ``` FLOAT/DOUBLE partitions are accepted by schema validation, and bounded `FlinkTableSource.applyFilters` consumes partition-only predicates, so the engine cannot remove the extra row. For ordinary columns the residual does remove the `>=` false positive, but it cannot recover the `<` false negative. Reverting only `CompareUtils` to the parent version makes all 17 ordering/partition oracle checks pass on Flink 1.20.1; the existing equality bug is a separate reason to keep this fix's intent. Please make the pushdown conservative for each engine's comparison semantics (or leave incompatible Flink floating predicates unconverted), and cover both signed-zero directions with the engine expression as the oracle, including partition columns. Validation on the PR head: 64 predicate/core/file-format tests and 63 Flink predicate-converter tests pass with normal Maven checks. The new SQL oracle exposes cases absent from the added equality-only Flink test. Spark 3.5: all 23 PaimonPushDownTest cases (including the new actual SQL signed-zero checks) and all 10 PushDownAggregatesTest cases also pass with normal Maven checks. -- 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]
