jayzhan211 commented on issue #26138:
URL: https://github.com/apache/datafusion/issues/26138#issuecomment-6497193637
@mkleen slt is right for the cases we already know: each fix PR should pin
its rows from the table in the description in slt. The differential test is for
the cases nobody has found yet, so it can't rely on hand-written expected
output.
The rule: changing a `-0.0` to `0.0` must not change any result, apart from
how `-0.0` prints. So the expected result comes from DataFusion itself, run on
a copy where every `-0.0` is `0.0`.
1. Generate tables whose float columns (flat, list, struct) draw from a
small pool (`0.0`, `1.0`, `-1.0`, `inf`, one NaN), so keys tie often. Add a
unique `id`.
2. Make a second copy by flipping the sign of some zeros, both in the data
and in the zero literals of the query, since constant folding is a separate
path (today `0.0 IN (-0.0)` is false while `-0.0 = 0.0` is true).
3. Run the query on both copies under the same settings, print `-0.0` as
`0.0`, and compare. Repeat under several settings: hash vs sort-merge join,
batch size 1, 1 vs 4 partitions, a memory limit that forces spilling, and both
copies read from Parquet with small row groups.
This also covers code with only one implementation (IN, CASE, `nullif`,
`array_*`), where comparing hash join with sort-merge join can't help. It would
replace the "operators that must agree" test in the description; I'll update
the description if you agree.
I tried a quick SQL version on main: 25 queries over 4 rows, queries below.
It flags every case in the table, plus some new ones, which I've added to the
table:
- `count(DISTINCT a)` counts `-0.0` and `0.0` separately when there is a
second DISTINCT aggregate, or when it is a window function. On its own it
passes, because it is planned as a GROUP BY.
- `array_remove`, and `array_has(k, 0.0)` on a list column. With a column
`a`, `array_has(make_array(a), 0.0)` passes, but only because it is rewritten
to `a = 0.0` and never reaches the `array_has` kernel.
- Float64 `z IN (0.0, 1.0)` now returns false too, not only Float32.
- With `prefer_hash_join = false`, a join on a `List(Float64)` key returns 3
of 6 pairs, and two of the dropped pairs contain no `-0.0` (#26115).
GROUP BY and the flat equi-join pass.
For the real test I'd suggest Rust in `datafusion/core/tests/fuzz_cases/`.
`aggregation_fuzzer` shows how to run a query under random settings, but its
check sorts the rows, which would hide the ORDER BY bugs here. Some rules:
- A fixed list of query templates, not random SQL: ORDER BY (several
columns, list keys, with LIMIT), window PARTITION BY / rank / RANGE frames,
GROUP BY / DISTINCT / DISTINCT ON, `count(DISTINCT)`, equi and range joins, set
operations, IN, CASE, `nullif`, `array_*`, `min`/`max` on lists.
- Each query must have one correct answer. An ORDER BY that picks or numbers
rows (LIMIT, `first_value`, DISTINCT ON, `row_number`, `lag`, ROWS frames) ends
with `id`. `rank` and RANGE frames don't, because their ties are what we test.
Compare ORDER BY output in order and everything else as a multiset.
- No arithmetic on the float columns (`-x`, `x * y`), since it can create
`-0.0` in the zero-only copy. No functions that can see the sign (`1 / x`,
`atan2`, `cot`, casts to string).
- A list of (query, setting) pairs that are still wrong, each with its
issue. Random data can only check that unlisted pairs pass. A fixed dataset
where each listed pair is known to fail also checks that listed pairs still
fail, so each fix PR has to remove its entries.
- Once #26091 is decided, add NaN payloads to the pool and to step 2.
Does this match what you had in mind? If so, this test could be the next PR,
before the sort-order change, and I'm happy to review it.
<details><summary>Queries (each runs on <code>raw</code> and on
<code>canon</code> in place of <code>{t}</code>)</summary>
```sql
CREATE TABLE raw AS SELECT
column1 AS id, column2 AS a, arrow_cast(column2, 'Float32') AS a32,
column3 AS b, 1 AS g, column4 AS k
FROM (VALUES
(1, arrow_cast(-0.0, 'Float64'), 1, make_array(arrow_cast(-0.0,
'Float64'), 1.0)),
(2, arrow_cast(0.0, 'Float64'), 0, make_array(0.0, 0.0)),
(3, arrow_cast(1.0, 'Float64'), 2, make_array(1.0)),
(4, arrow_cast(-0.0, 'Float64'), 3, make_array(0.0, 1.0))
);
CREATE TABLE canon AS SELECT
column1 AS id, column2 AS a, arrow_cast(column2, 'Float32') AS a32,
column3 AS b, 1 AS g, column4 AS k
FROM (VALUES
(1, arrow_cast(0.0, 'Float64'), 1, make_array(0.0, 1.0)),
(2, arrow_cast(0.0, 'Float64'), 0, make_array(0.0, 0.0)),
(3, arrow_cast(1.0, 'Float64'), 2, make_array(1.0)),
(4, arrow_cast(0.0, 'Float64'), 3, make_array(0.0, 1.0))
);
SELECT a, b FROM {t} WHERE id <= 2 ORDER BY a, b;
SELECT id, rank() OVER (ORDER BY a) FROM {t} ORDER BY id;
SELECT id, sum(b) OVER (ORDER BY a RANGE BETWEEN CURRENT ROW AND CURRENT
ROW) FROM {t} ORDER BY id;
SELECT max(rn) FROM (SELECT row_number() OVER (PARTITION BY a) rn FROM {t}
WHERE id <= 2);
SELECT first_value(b ORDER BY a, b) FROM {t} WHERE id <= 2;
SELECT DISTINCT ON (g) g, b FROM {t} ORDER BY g, a, b;
SELECT id FROM {t} WHERE id <= 2 ORDER BY k;
SELECT min(k), max(k) FROM {t} WHERE id <= 2;
SELECT id, a32 IN (0.0, 1.0) FROM {t} ORDER BY id;
SELECT id, a IN
(0.0,1.0,2.0,3.0,4.0,5.0,6.0,7.0,8.0,9.0,10.0,11.0,12.0,13.0,14.0,15.0,16.0,17.0,18.0,19.0,20.0,21.0,22.0,23.0,24.0,25.0)
FROM {t} ORDER BY id;
SELECT id, array_has(make_array(a), 0.0), array_has_any(make_array(a),
make_array(0.0)), array_has_all(make_array(a), make_array(0.0)) FROM {t} ORDER
BY id;
SELECT id, array_position(k, 0.0) FROM {t} ORDER BY id;
SELECT id, array_replace(k, 0.0, 9.0) FROM {t} ORDER BY id;
SELECT id, nullif(a, 0.0) FROM {t} ORDER BY id;
SELECT id, CASE a WHEN 0.0 THEN 'eq' ELSE 'ne' END FROM {t} ORDER BY id;
SELECT count(*) c FROM {t} GROUP BY a ORDER BY c;
SELECT count(*) FROM {t} x JOIN {t} y ON x.a = y.a;
SELECT count(DISTINCT a) FROM {t};
SELECT count(*) FROM {t} x JOIN {t} y ON x.k = y.k;
SELECT a, b FROM {t} WHERE id <= 2 ORDER BY a, b LIMIT 1;
SELECT id, array_has(k, 0.0) FROM {t} ORDER BY id;
SELECT id, a IN (0.0, 1.0) FROM {t} ORDER BY id;
SELECT count(DISTINCT a), count(DISTINCT b) FROM {t};
SELECT DISTINCT count(DISTINCT a) OVER () FROM {t};
SELECT id, array_remove(k, 0.0) FROM {t} ORDER BY id;
```
</details>
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
