zhuxiangyi opened a new pull request, #10203:
URL: https://github.com/apache/paimon/pull/10203
### Purpose
A query like `WHERE d = 0.0` silently loses rows whose value is `-0.0`. It
happens whenever the filter is pushed down to Paimon, for every file format, on
append and primary-key tables, and for both DOUBLE and FLOAT.
Spark and Flink compare floating-point values numerically, so `-0.0 = 0.0`
is true. Paimon's predicates compare them with `Double/Float.compareTo`, which
orders `-0.0` before `0.0`. A data file whose values are all `-0.0` has min =
max = `-0.0`, so file-stats pruning decides that `= 0.0` can't match and skips
the file before the engine sees the row. `IN`, `>=` and `BETWEEN` lose rows the
same way. The mirror case, a file holding only `+0.0` queried with `-0.0`,
loses under `=`, `IN`, `<=` and `BETWEEN`. The file doesn't need to be large or
special: a three-row Spark `INSERT` was split into two files, and the one
holding `-0.0` was skipped.
Reproduced on master (`e42cd1302`), no Paimon options needed:
```sql
-- Spark
CREATE TABLE t (id INT, d DOUBLE) USING paimon;
INSERT INTO t VALUES (1, CAST('-0.0' AS DOUBLE));
SELECT id FROM t WHERE d = 0.0; -- returns nothing, expected 1
```
- **Spark 3.5:** `=`, `IN`, `>=`, `BETWEEN` on DOUBLE and FLOAT, parquet /
orc / avro, append and primary-key tables: all lose the row.
- **Flink 1.20:** `=` loses the row on all three formats. It only happens
when `-0.0` is produced at runtime (e.g. `SELECT -x`), because Flink folds
constant `-0.0` to `0.0`. Flink itself evaluates `-0.0 >= 0.0` as false, so
`>=`/`IN`/`BETWEEN` don't show a difference there.
There are two places to fix:
1. **`CompareUtils.compareLiteral`** (paimon-common) compares DOUBLE/FLOAT
as `compare(a + 0.0, b + 0.0)`. Adding a positive zero turns `-0.0` into `0.0`
and leaves every other value, NaN included, unchanged. Every leaf function uses
this method, so it fixes both file-stats pruning and Paimon's own row filtering
(`executeFilter`) for all formats.
2. **`OrcPredicateFunctionVisitor`** (paimon-format). ORC evaluates the
search argument against stripe/row-group statistics with `compareTo` as well,
so even with Paimon's stats disabled ORC skipped these files; parquet and avro
did not. For a zero literal the visitor now:
- uses `-0.0` for `<` and `>=`, which are built on ORC's less-than, and
`+0.0` for `<=` and `>`, which are built on less-than-or-equal;
- turns `=` into `>= -0.0 AND <= +0.0` and `!=` into its negation;
- expands a zero in `IN` to both zeros.
Non-zero literals are converted exactly as before.
**Behaviour change to note:** `compareLiteral` is also used when MIN/MAX
aggregates are answered from statistics (`DataSplit`, `LocalAggregator`,
`AggFuncEvaluator`) and by the TopN split evaluator. When both `-0.0` and `0.0`
are present they now tie, so MIN/MAX may return either zero, which is what the
engines do too. Ordering of all other values, NaN included, is unchanged.
Related:
- #10144: while pushing down predicates on nested fields, the review
(https://github.com/apache/paimon/pull/10144#pullrequestreview-5300205177)
pointed out signed-zero false negatives for nested FLOAT/DOUBLE. #10144 left
those predicates to Flink with a nested-only guard
(`rejectNestedFloatingPoint`) and noted that top-level columns have the same
problem on master and would be handled separately. This PR is that fix. Once
it's in, the nested guard is no longer needed for correctness and can be
relaxed in a follow-up.
- #9427 added negated predicates to Flink's `PredicateConverter` and keeps
negated FLOAT/DOUBLE comparisons unconverted because `compareTo` tells the
zeros apart
(`PredicateConverterTest#testNegatedFloatingPointSignedZerosRemainUnsupported`).
That rejection is left as is here. The last assertion of that test pinned the
old behaviour (`Equal(d, 0.0)` rejects a `-0.0` row) and is updated.
### Tests
Each new test fails on master and passes with this change. I checked this by
reverting only the two main-code files.
- `PredicateTest#testSignedZeroDouble` / `#testSignedZeroFloat`: row and
statistics evaluation for `=`, small and large `IN` (a single `In` leaf), `>=`,
`<=`, `BETWEEN`, plus `!=`, `<`, `>`, `NOT IN`, `NOT BETWEEN`, in both sign
directions. Also checks that NaN still matches NaN.
- `OrcFormatReadWriteTest#testSignedZeroPredicatesKeepTheFile`: ORC files
holding only `-0.0` or only `+0.0` are read under each predicate on the other
zero. A control predicate (`= 1.0`) confirms the file is still skipped when it
should be.
- `SignedZeroFilterTest` (new, paimon-core): parquet / orc / avro × append /
primary-key × DOUBLE / FLOAT, through scan planning and reading, with and
without `executeFilter`, plus the same control.
- `FilterPushDownITCase#testNegativeZeroMatchesEqualityOnZero`: Flink SQL
end to end, for all three formats.
- `PaimonPushDownTestBase` "-0.0 matches predicates on 0.0": Spark SQL end
to end, for all three formats × append / primary-key, and 5 predicates on
DOUBLE and FLOAT. It asserts the raw bits so the test can't pass on a stored
`+0.0`.
Also ran the existing suites around the changed code: paimon-common
predicate tests (538), ORC / parquet / avro format tests (163), core scan and
filter tests including `TableScanTest`, `*FileMetaFilterTest` and
`DataSplitCompatibleTest` (309), Flink `FilterPushDownITCase`,
`BatchFileStoreITCase`, `PredicateConverterTest`,
`FilterPushdownWithSchemaChangeITCase`, the nested push-down tests and
`FlinkTableSourceTest` (207), and Spark 3.5 `PaimonPushDownTest` +
`PushDownAggregatesTest` (33). All pass. Not run locally: Spark 4 and Flink 2.x.
--
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]
