andygrove opened a new issue, #6636:
URL: https://github.com/apache/datafusion-comet/issues/6636
Part of #6615. Related to #6385.
## What / Why
`CometFloatSemanticsSuite` (added in #6521 for #6385) sweeps Spark's `-0.0`
and NaN rules across every operator context and float expression. Its single
`test(c.name)` call expands to 412 runtime tests over DOUBLE and FLOAT.
Static analysis on `main` at 3bc2faa934 split the cases into five groups:
| Group | Runtime tests | Convert | Known gap today |
| --- | --- | --- | --- |
| comparison | 224 | 88: Project, Filter and both window contexts for every
op; `=` and `<=>` for the 4 join conditions; `=` and `!=` for the 4
array/struct operand contexts | 136: #6385 (96), #6157 (40) |
| literal comparison | 42 | 28: the Project and Filter instances | 14: the
scan-filter instances; they need ConstantFolding, and 12 are also known gaps |
| key | 44 | all 44 | none |
| expression | 76 | 72 | 4: `greatest` / `least` (#6385) |
| aggregate | 26 | 16, plus `collect_set` in a `MinSparkVersion: 4.2` file |
10: max/min/global/window (8, #6385) and `collect_set` before 4.2 (2, #5312) |
Before Spark 4.2, 250 cases must match Spark and can move to SQL tests
today. The other 162 (160 on 4.2+) are tripwires that **expect** a mismatch.
They are tied to open issues and are meant to start failing when those issues
are fixed.
| Suite | Covered | Convert | Blocked | Split | Keep | Total |
|---|---|---|---|---|---|---|
| `CometFloatSemanticsSuite` | 0 | 1 | 0 | 4 | 0 | 5 |
## Work
1. **Convert the 250 matching cases** to `sql-tests/float_semantics/*.sql`,
one file per group: `comparison.sql`, `literal_comparison.sql`, `keys.sql`,
`expressions.sql`, `aggregates.sql`, and `aggregates_collect_set_4_2.sql`.
- Each file needs the 9 edge rows: `0.0`, `-0.0`, canonical NaN, sign-bit
NaN, `1.0`, `-1.0`, ±Infinity and NULL.
- Add the CTAS pair table, and replace the suite's temp views with CTEs.
- Spark's Parquet writer canonicalizes NaN, so make the sign-bit NaN at
query time by negating a stored NaN, as the suite does. The SQL comparator
distinguishes the zero signs and canonicalizes NaN payloads, which matches what
the suite asserts.
- Some of these overlap fixtures that already exist. The nested ORDER BY
and window order keys are in `windows/nested_float_order_keys.sql`, and the
hash cases are largely in `hash.sql:41-55`. Don't duplicate them.
2. **Decide what to do with the 162 known-gap tripwires.** The SQL framework
has no expected-mismatch mode. `query ignore(<issue>)` would drop the tripwire:
nothing would notice when a gap closes, and #6385 relies on that signal. There
are two options:
- Keep the known-gap cases in a much smaller Scala suite until #6385,
#6157 and #5312 land. This is simplest.
- Add a query mode that asserts the Comet answer *differs* from Spark and
fails when they match. Use it only for tripwires that carry an issue link.
3. **The 14 scan-filter literal cases need ConstantFolding (#6618).** The
Scala suite folds `-CAST('NaN' AS T)` into a sign-bit Literal, while a fixture
keeps `UnaryMinus(Literal)`. #6385's scan-filter gap lives in exactly that
difference.
This issue is lower priority than the others in #6615. The suite is new, it
is self-contained, and it is actively used to track #6385, so coordinate with
that work before moving cases.
## Done when
- Every case group is either in a fixture or deliberately kept as a
tripwire, with the reason recorded in the suite or the fixture.
- The fixtures pass on every Spark profile (apply `run-all-spark-profiles`).
<details><summary>Rows in scope</summary>
| Suite | Line | Test | Verdict | Target |
|---|---|---|---|---|
| `CometFloatSemanticsSuite` | 72 | comparison: {context}, {op}
[{double\|float}] (224 instances: 16 contexts x 7 ops x 2 types) | split (rest:
other) | ../float_semantics/comparison.sql (new; split joins/windows into
sibling files if preferred) |
| `CometFloatSemanticsSuite` | 72 | literal comparison:
{Project\|Filter\|scan filter}, {op} [{double\|float}] (42 instances) | split
(rest: #6618) | ../float_semantics/literal_comparison.sql (new) |
| `CometFloatSemanticsSuite` | 72 | key: {context} [{double\|float}] (44
instances: 22 contexts x 2 types) | convert | ../float_semantics/keys.sql (new)
|
| `CometFloatSemanticsSuite` | 72 | expression: {name} [{double\|float}] (76
instances: 38 expressions x 2 types) | split (rest: other) |
../float_semantics/expressions.sql (new) |
| `CometFloatSemanticsSuite` | 72 | aggregate: {name} [{double\|float}] (26
instances: 13 aggregates x 2 types) | split (rest: other) |
../float_semantics/aggregates.sql (new) + aggregates_collect_set_4_2.sql (new) |
</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]
