Skip to content

test: move window function coverage from CometWindowExecSuite to Comet SQL tests #6632

Open

Description

@andygrove

Part of #6615.

What / Why

This issue moves window function coverage from exec/CometWindowExecSuite to Comet SQL tests under sql-tests/windows/ and sql-tests/expressions/window/. It also covers the floating-point sort-key tests in CometExpressionSuite, which target the same order-key fixtures. Static analysis on main at 3bc2faa found the following:

Suite Covered Convert Blocked Split Keep Total
CometExpressionSuite 0 5 0 0 2 7
CometWindowExecSuite 25 30 0 2 0 57
Total 25 35 0 2 2 64

Unlike most areas, 25 of the window tests are already duplicated by windows/window_functions.sql and expressions/window/lag_lead.sql. They can be deleted as soon as this issue starts; the table cites the covering fixture section for each. The rest convert, with two framework facts to know first.

  1. Write N PRECEDING as -N FOLLOWING in ROWS frames.
    • Spark parses N PRECEDING as UnaryMinus(Literal(N)), and WindowEvaluatorFactoryBase.createBoundOrdering accepts only an IntegerLiteral as a ROWS bound. The Scala tests get Literal(-N) only because ConstantFolding runs there. In a fixture, the Spark side of the comparison throws "Unhandled bound in windows expressions".
    • -N FOLLOWING parses straight to Literal(-N), the same bound the Scala run ends up with. CometWindowExec reads ROWS bounds with e.eval(), so the native frame is identical.
    • RANGE frames are not affected.
    • This is a static conclusion. Confirm it with one fixture first. If it fails, L590, L904, L924 and L1497 wait on test: let Comet SQL tests keep ConstantFolding, exclude optimizer rules and change configs mid-file #6618.
  2. PERCENT_RANK and CUME_DIST are compared only under tolerance today, in window_functions.sql 1.6, 5.2 and 5.3. Those tolerance queries have no ORDER BY, so they depend on both engines collecting rows in the same order (test: query tolerance= in Comet SQL tests passes when either side is NaN #6616). The Scala tests compare exactly and pass, so use an exact query.

Work

Line numbers refer to CometWindowExecSuite unless marked.

windows/window_functions.sql (existing): 18 tests

  • New small tables:
    • L389: (value int) = 1,2,4,3,2,1 with SUM(value) OVER (ORDER BY value DESC RANGE BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING).
    • L408: the same table, with RANGE BETWEEN 1 FOLLOWING AND UNBOUNDED FOLLOWING as expect_fallback(Unsupported RANGE frame lower offset).
    • L443: (k int, v boolean) with every(v) OVER (PARTITION BY k ORDER BY v).
    • L459: the nine testData rows including NULL keys, with a count(val) over a ROWS CURRENT ROW frame.
    • L482: count(key) over RANGE BETWEEN CURRENT ROW AND 2147483648 FOLLOWING and over RANGE BETWEEN 2147483649 PRECEDING AND CURRENT ROW.
  • New queries on emp:
    • L639: SUM(salary) OVER (PARTITION BY dept). Today no native-mode OVER (PARTITION BY x) without ORDER BY is tested; sections 6.3-6.6 all expect fallback.
    • L852, L944, L964: COUNT(*) and MAX over ROWS frames.
    • L1208: FIRST_VALUE with the default frame. Order by id: every partition has tied b values with different c, so the Scala query is nondeterministic.
    • L1229: LAST_VALUE with RESPECT NULLS.
    • L1250: NTH_VALUE(salary, 2).
    • L1044, L1270, L1517: one exact query with ROW_NUMBER, RANK, DENSE_RANK, PERCENT_RANK, CUME_DIST and NTILE(3) in one spec.
  • PRECEDING frames: L904, L924, L1497, written with -N FOLLOWING. Also correct the section 1.9 comment (L197-205), which says these frames are covered in Scala "via the DataFrame API".

New fixtures

  • windows/window_decimal_aggregates.sql + _ansi.sql (L499, L672, L704, L742, L778)
    • Covers decimal SUM/AVG over windows: OVER () on decimal(2,1), and running AVG/SUM on decimal(10,2) and (8,2).
    • The curated running AVG at (9,1), (9,4), (10,2), (11,3) and (11,6) includes half-up ties at scale s+4.
    • Add decimal(38,38) 0.6, 0.6, -0.6, where SUM and try_sum give 0.6, NULL, 0.6. In the ANSI file, try_sum is the native sentinel and SUM gets expect_error(NUMERIC_VALUE_OUT_OF_RANGE), which matches the error classes on both sides of 4.0.
    • Today these shapes are only in window_filter.sql, which is gated to 4.2+.
  • windows/window_basic_frames.sql (L590)
    • -- ConfigMatrix: spark.sql.adaptive.enabled=true,false.
    • t1(_1, _2) = (i, 10-i) written as one file, because a ROWS frame without ORDER BY depends on input order.
    • 35 queries: COUNT(_1), COUNT(*), MAX, MIN and SUM, each over 7 frame shapes.
  • windows/window_aggregate_types.sql (L528)
    • A curated replacement for the random makeParquetFile columns: tinyint through bigint, float, double, decimal(5,2)/(18,10)/(38,37), timestamp and date.
    • Include whole-row NULLs, plus low- and high-cardinality variants.
    • Add spark.comet.batchSize and outputTimestampType matrices.
    • Use exact query except float/double SUM, which needs tolerance with a total ORDER BY.
  • windows/window_literal_partition_order.sql (L425)
    • sum(product + 1) OVER (PARTITION BY 1 ORDER BY 2) over Parquet, with a shuffle-mode matrix.
    • This is stronger than the Scala LocalTableScan source, where ConvertToLocalRelation folds product + 1.
  • windows/window_repeated_shuffle.sql (L508)
    • spark.sql.requireAllClusterKeysForDistribution=true, native shuffle, and an AQE matrix.
    • lead(...) over a REPARTITION(key1) subquery.
  • window/lag_lead_offset_beyond_rows.sql (L368): jvm shuffle and localTableScan on, with both of the test's queries verbatim.
  • Floating-point keys:
    • windows/window_group_limit_float_peers.sql (L126, MinSparkVersion: 3.5): signed-zero and sign-bit-NaN peers at the group-limit cutoff, written with IF(neg, -x, x), with a strictFloatingPoint matrix.
    • windows/nested_float_order_keys.sql / _strict.sql (existing; L213, and CometExpressionSuite L282 and L290)
      • RANK/DENSE_RANK over array(f), array(d) and named_struct(...) keys, with signed zeros at the rnk <= 1 cutoff.
      • Add array and struct float columns read from Parquet, including multi-element, empty and NULL arrays.
    • windows/scalar_float_order_keys_strict.sql (L276, and CometExpressionSuite L96, L158 (16 instances) and L177)
      • strictFloatingPoint=true with the SortOrder opt-in off.
      • Curated ±0.0, NaN, ±Inf, NULL, Min and Max values with a unique id.
      • Cover ORDER BY, TopK, and NULLS FIRST/LAST.
    • windows/window_float_partition_keys.sql + window_group_limit_float_partition_keys.sql (L315, the latter gated 3.5): keys 1.0, 2.0, NULL, 0.0, -0.0, NaN and a sign-bit NaN.

Stays in Scala

Notes

  • No-op config. The suite-wide spark.comet.operator.WindowExec.allowIncompatible=true (L51) and the same key in window/lag_lead.sql do nothing. CometWindowExec does not override getSupportLevel, so the operator is always Compatible.
  • Misnamed tests.
    • L443 ("fall back to Spark when ...") actually asserts a fully native plan. It also creates a view named test_agg inside withTempView("test"), so the view is never dropped.
    • L499 ("Unsupported window expression should fall back to Spark") never compares with Spark or checks for a fallback.
    • L1208 ("FIRST_VALUE with default ignore nulls") runs with the default, which is RESPECT NULLS.
  • Redundant loops. L528 makes twelve runs that cover four distinct cases.
  • Internal duplicates. L1517 subsumes L984, L1004, L1024, L1044 and L1270.
  • Stale fixture header. window_group_limit_rank_dense_rank.sql says its rk = 0 query uses spark_answer_only, but it is a plain query.

Done when

  • Every test in the table below is accounted for. Either a fixture covers it (the same inputs, configs and assertion strength, or stronger), the fixture that already covers it is cited, or it stays in Scala for the reason given above.
  • Each converted Scala test is deleted in the PR that adds its coverage. A suite left empty is deleted and removed from the suite lists in .github/workflows/pr_build_linux.yml and pr_build_macos.yml.
  • The new and changed fixtures pass on every Spark profile. Apply run-all-spark-profiles, because pull requests run only the default profile.
Every Scala test in scope, with its verdict and target fixture
Suite Line Test Verdict Target
CometExpressionSuite 96 sort floating point with negative zero convert ../windows/scalar_float_order_keys_strict.sql (new)
CometExpressionSuite 158 strict floating point: scalar sort on $label (loop: col f|d x ASC|DESC x NULLS FIRST|NULLS LAST x compound false|true = 16 instances) convert ../windows/scalar_float_order_keys_strict.sql (new)
CometExpressionSuite 177 strict floating point: scalar TopK sort key convert ../windows/scalar_float_order_keys_strict.sql (new)
CometExpressionSuite 198 strict floating point: scalar sort keeps NaN payloads and zero signs unchanged keep
CometExpressionSuite 282 sort array of floating point with negative zero convert ../windows/nested_float_order_keys_strict.sql + ../windows/nested_float_order_keys.sql
CometExpressionSuite 290 sort struct containing floating point with negative zero convert ../windows/nested_float_order_keys_strict.sql + ../windows/nested_float_order_keys.sql
CometExpressionSuite 301 strict floating point: nested sort keeps NaN payloads and zero signs unchanged keep
CometWindowExecSuite 126 window group limit: $orderColumn NaN and signed zero peers at the cutoff (strictFloatingPoint=$strictFloatingPoint) [loop: orderColumn f|d x strictFloatingPoint false|true = 4 instances] split (rest: #6618) ../windows/window_group_limit_float_peers.sql (new)
CometWindowExecSuite 213 window group limit: floating-point values nested in the order key convert ../windows/nested_float_order_keys.sql + ../windows/nested_float_order_keys_strict.sql
CometWindowExecSuite 276 window: scalar floating-point order key under strict floating point ($orderColumn) [loop: f|d = 2 instances] convert ../windows/scalar_float_order_keys_strict.sql (new)
CometWindowExecSuite 315 window: floating partition keys ($partitionColumn, $function, group limit=$groupLimit) [loop: f|d x {RANK() false, PERCENT_RANK() false, NTILE(2) false, RANK() true} = 8 instances] convert ../windows/window_float_partition_keys.sql (new) + ../windows/window_group_limit_float_partition_keys.sql (new)
CometWindowExecSuite 368 lead/lag should return the default value if the offset row does not exist convert window/lag_lead_offset_beyond_rows.sql (new)
CometWindowExecSuite 389 window query with rangeBetween convert ../windows/window_functions.sql
CometWindowExecSuite 408 window query with rangeBetween FOLLOWING lower bound falls back to Spark convert ../windows/window_functions.sql
CometWindowExecSuite 425 window function: partition and order expressions convert ../windows/window_literal_partition_order.sql (new)
CometWindowExecSuite 443 fall back to Spark when the partition spec and order spec are not the same for window function convert ../windows/window_functions.sql
CometWindowExecSuite 459 Native window operator should be CometUnaryExec convert ../windows/window_functions.sql
CometWindowExecSuite 482 Window range frame with long boundary should not fail convert ../windows/window_functions.sql
CometWindowExecSuite 499 Unsupported window expression should fall back to Spark convert ../windows/window_decimal_aggregates.sql (new)
CometWindowExecSuite 508 Repeated shuffle exchange don't fail convert ../windows/window_repeated_shuffle.sql (new)
CometWindowExecSuite 528 aggregate window function for all types convert ../windows/window_aggregate_types.sql (new)
CometWindowExecSuite 590 Windows support convert ../windows/window_basic_frames.sql (new)
CometWindowExecSuite 623 window: simple COUNT(*) without frame covered ../windows/window_functions.sql
CometWindowExecSuite 639 window: simple SUM with PARTITION BY convert ../windows/window_functions.sql
CometWindowExecSuite 655 window: AVG with PARTITION BY and ORDER BY covered ../windows/window_functions.sql
CometWindowExecSuite 672 window: decimal AVG with PARTITION BY and ORDER BY convert ../windows/window_decimal_aggregates.sql (new)
CometWindowExecSuite 704 window: decimal AVG fuzz with PARTITION BY and ORDER BY [loop over (precision, scale) in (9,1),(9,4),(10,2),(11,3),(11,6) inside one test] convert ../windows/window_decimal_aggregates.sql (new)
CometWindowExecSuite 742 window: decimal SUM with PARTITION BY and ORDER BY convert ../windows/window_decimal_aggregates.sql (new)
CometWindowExecSuite 778 window: decimal SUM recovers from an intermediate overflow in an expanding frame [loop over ANSI false/true inside one test] split (rest: #6617) ../windows/window_decimal_aggregates.sql (new) + ../windows/window_decimal_aggregates_ansi.sql (new)
CometWindowExecSuite 831 window: MIN and MAX with ORDER BY covered ../windows/window_functions.sql
CometWindowExecSuite 852 window: COUNT with ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW convert ../windows/window_functions.sql
CometWindowExecSuite 884 window: SUM with ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING covered ../windows/window_functions.sql
CometWindowExecSuite 904 window: AVG with ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING convert ../windows/window_functions.sql
CometWindowExecSuite 924 window: SUM with ROWS BETWEEN 2 PRECEDING AND CURRENT ROW convert ../windows/window_functions.sql
CometWindowExecSuite 944 window: COUNT with ROWS BETWEEN CURRENT ROW AND 2 FOLLOWING convert ../windows/window_functions.sql
CometWindowExecSuite 964 window: MAX with ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING convert ../windows/window_functions.sql
CometWindowExecSuite 984 window: ROW_NUMBER with PARTITION BY and ORDER BY covered ../windows/window_functions.sql
CometWindowExecSuite 1004 window: RANK with PARTITION BY and ORDER BY covered ../windows/window_functions.sql
CometWindowExecSuite 1024 window: DENSE_RANK with PARTITION BY and ORDER BY covered ../windows/window_functions.sql
CometWindowExecSuite 1044 window: PERCENT_RANK with PARTITION BY and ORDER BY convert ../windows/window_functions.sql
CometWindowExecSuite 1064 window: NTILE with PARTITION BY and ORDER BY covered ../windows/window_functions.sql
CometWindowExecSuite 1091 window: LAG with default offset covered ../windows/window_functions.sql + window/lag_lead.sql
CometWindowExecSuite 1111 window: LAG with offset 2 and default value covered ../windows/window_functions.sql + window/lag_lead.sql
CometWindowExecSuite 1131 window: LAG with IGNORE NULLS covered ../windows/window_functions.sql + window/lag_lead.sql
CometWindowExecSuite 1149 window: LEAD with default offset covered ../windows/window_functions.sql + window/lag_lead.sql
CometWindowExecSuite 1169 window: LEAD with offset 2 and default value covered ../windows/window_functions.sql + window/lag_lead.sql
CometWindowExecSuite 1189 window: LEAD with IGNORE NULLS covered ../windows/window_functions.sql + window/lag_lead.sql
CometWindowExecSuite 1208 window: FIRST_VALUE with default ignore nulls convert ../windows/window_functions.sql
CometWindowExecSuite 1229 window: LAST_VALUE with ROWS frame convert ../windows/window_functions.sql
CometWindowExecSuite 1250 window: NTH_VALUE with position 2 convert ../windows/window_functions.sql
CometWindowExecSuite 1270 window: CUME_DIST with PARTITION BY and ORDER BY convert ../windows/window_functions.sql
CometWindowExecSuite 1290 window: multiple window functions in single query covered ../windows/window_functions.sql
CometWindowExecSuite 1313 window: different window specifications in single query covered ../windows/window_functions.sql
CometWindowExecSuite 1335 window: ORDER BY DESC with aggregation covered ../windows/window_functions.sql
CometWindowExecSuite 1355 window: multiple PARTITION BY columns covered ../windows/window_functions.sql
CometWindowExecSuite 1375 window: multiple ORDER BY columns covered ../windows/window_functions.sql
CometWindowExecSuite 1395 window: RANGE BETWEEN with numeric ORDER BY covered ../windows/window_functions.sql
CometWindowExecSuite 1415 window: RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW covered ../windows/window_functions.sql
CometWindowExecSuite 1435 window: complex expression in window function covered ../windows/window_functions.sql
CometWindowExecSuite 1455 window: window function with WHERE clause covered ../windows/window_functions.sql
CometWindowExecSuite 1476 window: window function with GROUP BY covered ../windows/window_functions.sql
CometWindowExecSuite 1497 window: ROWS BETWEEN with negative offset convert ../windows/window_functions.sql
CometWindowExecSuite 1517 window: all ranking functions together convert ../windows/window_functions.sql
CometWindowExecSuite 1542 window: FIRST_VALUE/LAST_VALUE/NTH_VALUE IGNORE NULLS with leading nulls covered ../windows/window_functions.sql
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions