Skip to content

test: move CometFloatSemanticsSuite's matching cases to Comet SQL tests #6636

Open

Description

@andygrove

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 3bc2faa 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 [EPIC] Match Spark's -0.0 and NaN semantics systematically instead of per expression #6385 relies on that signal. There are two options:
  3. The 14 scan-filter literal cases need ConstantFolding (test: let Comet SQL tests keep ConstantFolding, exclude optimizer rules and change configs mid-file #6618). The Scala suite folds -CAST('NaN' AS T) into a sign-bit Literal, while a fixture keeps UnaryMinus(Literal). [EPIC] Match Spark's -0.0 and NaN semantics systematically instead of per expression #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).
Rows in scope
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions