You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/10/04 18:46:06 UTC

[I] test: move the Spark 4 collation suites to Comet SQL tests [datafusion-comet]

andygrove opened a new issue, #6633:
URL: https://github.com/apache/datafusion-comet/issues/6633

   Part of #6615.
   
   ## What / Why
   
   This issue moves the Spark 4 collation suites to Comet SQL tests:
   
   - `spark-4.0/CometCollationSuite`
   - `spark-4.1/CometCollationSuite`
   - `spark-4.x/CometSortCollationSuite`
   - `spark-4.x/CometCastCollatedStringSuite`
   
   Static analysis on `main` at 3bc2faa934 found the following:
   
   | Suite | Covered | Convert | Blocked | Split | Keep | Total |
   |---|---|---|---|---|---|---|
   | `CometCastCollatedStringSuite` | 0 | 14 | 0 | 4 | 3 | 21 |
   | `spark-4.0/CometCollationSuite` | 3 | 15 | 1 | 0 | 6 | 25 |
   | `spark-4.1/CometCollationSuite` | 3 | 15 | 1 | 0 | 1 | 20 |
   | `spark-4.x/CometSortCollationSuite` | 0 | 5 | 0 | 0 | 2 | 7 |
   | **Total** | **6** | **49** | **2** | **4** | **12** | **73** |
   
   The 4.1 collation suite is the 4.0 suite minus its five join-converter tests, so the other 20 tests are identical in both. Converted into fixtures gated `-- MinSparkVersion: 4.0`, they run on 4.0, 4.1 and 4.2, which takes most of the duplication that #4770 is about off the table.
   
   It also adds coverage. The shuffle and sort fallback reasons have never been checked on Spark 4.2, because the 4.0 and 4.1 suites don't run there. If 4.2 reports different reasons, split the fixture into a `MaxSparkVersion: 4.1` / `MinSparkVersion: 4.2` pair.
   
   Collated tests in other suites (`rlike`, `unix_timestamp`, string concat/reverse) are in the regex, datetime and string issues.
   
   ## Facts that affect the conversion
   
   1. **Collated literals.** `'x' COLLATE c` parses to `Collate(Literal, ...)`. With ConstantFolding excluded it stays a `Collate` node, which has no Comet serde. `CAST('MON' AS STRING COLLATE UTF8_LCASE)` works instead, because CometCast folds a cast of a literal into a collated `Literal`.
      - That is enough for `next_day`'s native path (4.0 L386 / 4.1 L207).
      - It is not enough for `date_format`, because `CometDateFormat.nativeApplicable` needs a Spark-level `Literal` format. So 4.0 L403 / 4.1 L224 wait on #6618.
   2. **Collated datetime fallbacks record two reasons.** When a `CodegenDispatchFallback` serde reports `Incompatible` for a collation and the dispatcher is disabled, Comet records both `<fn>: spark.comet.exec.scalaUDF.codegen.enabled=false` and `... does not support non-UTF8_BINARY collations`. `routing_datetime_collation_disabled.sql` asserts the first and the Scala tests assert the second, so each expression needs a second `expect_fallback` query.
   3. **Cast eval modes.** For `CometCastCollatedStringSuite`, the cast values never raise. So `-- ConfigMatrix: spark.sql.ansi.enabled=false,true` plus `try_cast` covers all three eval modes that `isSupported` checks.
   
   ## Work
   
   Collation-suite line numbers are given as "4.0 Lx / 4.1 Ly".
   
   **Datetime collation routing (existing fixtures)**
   
   - `datetime/routing_datetime_collation_disabled.sql`
     - Add a second query per expression asserting the collation-specific reason: `next_day` (L292 / L113), `convert_timezone` (L359 / L180), `trunc` (L366 / L187), `date_trunc` (L372 / L193), and `date_format` with a collated *column* format (L379 / L200).
     - Add `from_unixtime` with `expect_fallback(Only the default datetime format pattern)` (L315 / L136).
     - Add `expect_native(unix_timestamp)` for `unix_timestamp(ts, fmt_ts COLLATE UTF8_LCASE)` (L299 / L120), using a new `fmt_ts` column.
   - `datetime/routing_datetime_collation_enabled.sql`
     - The same `unix_timestamp` query with codegen on.
     - `expect_dispatch(date_format)` with a collated column format (L426 / L247). The existing query uses a foldable `Collate(Literal)` format, which takes `DateFormatClass`'s precomputed-formatter branch instead of the per-row one.
   - `datetime/unix_timestamp.sql` (L440 / L261): add the two-argument `unix_timestamp(ts, fmt_column)` with `expect_native`.
   
   **New fixtures (all `MinSparkVersion: 4.0`)**
   
   - `datetime/next_day_collation_allow_incompatible.sql` (L386 / L207)
     - Codegen off and `NextDay.allowIncompatible=true`.
     - `expect_native(next_day)` over `next_day(d, CAST('MON' AS STRING COLLATE UTF8_LCASE))`.
   - `string/collation_shuffle_fallback.sql` (L47/L35, L55/L43, L62/L50, L70/L58, L77/L65)
     - listagg DISTINCT, DISTINCT and GROUP BY on a collated string, each with `expect_fallback(unsupported hash partitioning data type for columnar shuffle)`.
     - ORDER BY on a collated string, with `expect_fallback(unsupported range partitioning data type for columnar shuffle)`.
     - A native control over Parquet.
   - `windows/window_sort_collation.sql` (sort L51, L118, and the window half of L182), with AQE off
     - `expect_fallback(Sort does not support non-default string collation)` for keys `c, _1`, `_1, c, _2` and `struct(array(c)), _2`.
     - Native window controls.
   - `string/collation_sort.sql` (sort L160 and the TopK half of L182): TopK `ORDER BY c, _1 LIMIT 2` over a `REPARTITION(1)` subquery.
   - `string/collation_sort_round_robin.sql` (sort L135): jvm shuffle, with `SELECT /*+ REPARTITION(2) */ ... SORT BY c, _1`.
   - `cast/routing_cast_collation_disabled.sql` and `cast/routing_cast_collation_enabled.sql` (`CometCastCollatedStringSuite`)
     - Table `(_1 string, _2 int)` with `('123', 1)` and `('456', 2)`, plus the ANSI ConfigMatrix.
     - Disabled file: codegen off, and `expect_fallback(Cast involving a non-default string collation is not supported)` for L98, L102, L109, L113, L117, L131, L140, L161, L170, L174, the convertible halves of L146, L151 and L156, and the end-to-end tests L224 and L243.
     - Enabled file: `expect_dispatch(cast)` for the same expressions, the end-to-end tests L234 and L261, and `expect_native(cast)` for the uncollated half of L183.
     - Things to check:
       - L109: Spark must type `AS STRING` as UTF8_BINARY.
       - L151 and L170: Spark 4 must accept a collated map key in `CreateMap`.
       - L174: the suite says no SQL builds `ArrayType(NullType) -> ARRAY<STRING COLLATE UTF8_LCASE>`, but with ConstantFolding excluded `CAST(array(NULL) AS ...)` should reach `isSupported`.
   
   **Blocked**
   
   - **4.0 L403 / 4.1 L224** (`date_format` with a collated literal format): needs ConstantFolding, #6618.
   
   ## Stays in Scala
   
   - **4.0 L119, L149, L175, L203, L228** call the join operator serde on hand-built plans; the test comment says end-to-end SQL cannot reach the join converter. The 4.1 copy dropped these, although they build plans directly and could run on 4.1+. Consider moving them to a 4.x shim while deleting the rest of these suites.
   - **Sort L65, L93** call `QueryPlanSerde.supportedSortType` on synthetic SortExec plans: map keys and values, which SQL cannot sort by, and the struct single-key versus multi-key rule.
   - **4.0 L328 / 4.1 L149** pin explain text (#6182). Their dispatch/fallback assertions duplicate `routing_datetime_collation_{enabled,disabled}.sql` L57-61.
   - **The identity casts in `CometCastCollatedStringSuite`** (L121, L135, L189, and the identity halves of L146, L151, L156 and L183). `SimplifyCasts` drops them before Comet sees them.
   
   ## 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.
   
   What stays in Scala afterwards (the explain-text tests and the join-converter serde tests) is small enough to move into one `spark-4.x` suite. That would retire both version-specific `CometCollationSuite` files and resolve #4770.
   
   <details><summary>Every Scala test in scope, with its verdict and target fixture</summary>
   
   | Suite | Line | Test | Verdict | Target |
   |---|---|---|---|---|
   | `CometCastCollatedStringSuite` | 98 | cast collated string to IntegerType has no native path | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 102 | cast IntegerType to collated string has no native path | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 109 | cast collated string to default-collation StringType has no native path | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 113 | cast default-collation StringType to collated string has no native path | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 117 | cast between two different collations has no native path | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 121 | cast collated string to the same collation has no native path | keep |  |
   | `CometCastCollatedStringSuite` | 131 | cast array of collated strings to another collation has no native path | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 135 | cast array of collated strings to the same collation has no native path | keep |  |
   | `CometCastCollatedStringSuite` | 140 | cast array of collated strings to StringType has no native path | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 146 | cast struct with a collated field has no native path | split | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 151 | cast map with a collated key has no native path | split | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 156 | cast map with a collated value has no native path | split | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 161 | cast struct whose collated field is unchanged while a sibling field is cast | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 170 | cast map whose collated key is unchanged while the value type is cast | convert | cast/routing_cast_collation_disabled.sql (new); cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 174 | cast array of nulls to array of collated strings has no native path | convert | cast/routing_cast_collation_disabled.sql (new) |
   | `CometCastCollatedStringSuite` | 183 | default-collation string casts are untouched by the collation guard | split | cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 189 | nested default-collation string casts are untouched by the collation guard | keep |  |
   | `CometCastCollatedStringSuite` | 224 | cast from a collated string falls back to Spark when codegen dispatch is off | convert | cast/routing_cast_collation_disabled.sql (new) |
   | `CometCastCollatedStringSuite` | 234 | cast from a collated string routes through the codegen dispatcher when it is on | convert | cast/routing_cast_collation_enabled.sql (new) |
   | `CometCastCollatedStringSuite` | 243 | cast of a struct carrying a collated field has no native path end to end | convert | cast/routing_cast_collation_disabled.sql (new) |
   | `CometCastCollatedStringSuite` | 261 | cast of a struct carrying a collated field routes through the codegen dispatcher | convert | cast/routing_cast_collation_enabled.sql (new) |
   | `spark-4.0/CometCollationSuite` | 47 | listagg DISTINCT with utf8_lcase collation (issue #1947) | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.0/CometCollationSuite` | 55 | DISTINCT on utf8_lcase collated string groups case-insensitively | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.0/CometCollationSuite` | 62 | GROUP BY utf8_lcase collated string groups case-insensitively | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.0/CometCollationSuite` | 70 | ORDER BY utf8_lcase collated string sorts case-insensitively | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.0/CometCollationSuite` | 77 | default UTF8_BINARY string still runs through Comet | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.0/CometCollationSuite` | 119 | CometBroadcastHashJoinExec rejects non-default collated join keys | keep |  |
   | `spark-4.0/CometCollationSuite` | 149 | CometHashJoinExec rejects non-default collated join keys | keep |  |
   | `spark-4.0/CometCollationSuite` | 175 | CometBroadcastHashJoinExec still accepts default UTF8_BINARY string keys | keep |  |
   | `spark-4.0/CometCollationSuite` | 203 | CometSortMergeJoinExec rejects non-default collated join keys | keep |  |
   | `spark-4.0/CometCollationSuite` | 228 | CometSortMergeJoinExec still accepts default UTF8_BINARY string keys | keep |  |
   | `spark-4.0/CometCollationSuite` | 292 | next_day rejects non-UTF8_BINARY collated dayOfWeek (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.0/CometCollationSuite` | 299 | unix_timestamp stays native with a collated format for timestamp input (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql + datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.0/CometCollationSuite` | 315 | from_unixtime rejects non-UTF8_BINARY collated format (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.0/CometCollationSuite` | 328 | $functionName with collated arguments has no native opt-in (codegen=$codegenEnabled, issue #6080) [loop: make_timestamp\|to_unix_timestamp x codegen false\|true = 4 instances] | keep |  |
   | `spark-4.0/CometCollationSuite` | 359 | convert_timezone rejects non-UTF8_BINARY collated timezone (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.0/CometCollationSuite` | 366 | trunc falls back with collated format when codegen is disabled (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.0/CometCollationSuite` | 372 | date_trunc falls back with collated format when codegen is disabled (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.0/CometCollationSuite` | 379 | date_format falls back with collated format when codegen is disabled (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.0/CometCollationSuite` | 386 | next_day uses native path with collated dayOfWeek when allowIncompatible is enabled | convert | datetime/next_day_collation_allow_incompatible.sql (new) |
   | `spark-4.0/CometCollationSuite` | 403 | date_format uses native path with collated format when allowIncompatible is enabled | blocked: #6618 | datetime/date_format_collation_allow_incompatible.sql (new) |
   | `spark-4.0/CometCollationSuite` | 415 | trunc routes collated format through codegen dispatcher (issue #4646) | covered | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.0/CometCollationSuite` | 420 | date_trunc routes collated format through codegen dispatcher (issue #4646) | covered | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.0/CometCollationSuite` | 426 | date_format routes collated format through codegen dispatcher (issue #4646) | convert | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.0/CometCollationSuite` | 432 | next_day routes collated dayOfWeek through codegen dispatcher (issue #5591) | covered | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.0/CometCollationSuite` | 440 | datetime expressions still run with default UTF8_BINARY collation (issue #4646) | convert | datetime/unix_timestamp.sql |
   | `spark-4.1/CometCollationSuite` | 35 | listagg DISTINCT with utf8_lcase collation (issue #1947) | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.1/CometCollationSuite` | 43 | DISTINCT on utf8_lcase collated string groups case-insensitively | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.1/CometCollationSuite` | 50 | GROUP BY utf8_lcase collated string groups case-insensitively | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.1/CometCollationSuite` | 58 | ORDER BY utf8_lcase collated string sorts case-insensitively | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.1/CometCollationSuite` | 65 | default UTF8_BINARY string still runs through Comet | convert | string/collation_shuffle_fallback.sql (new) |
   | `spark-4.1/CometCollationSuite` | 113 | next_day rejects non-UTF8_BINARY collated dayOfWeek (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.1/CometCollationSuite` | 120 | unix_timestamp stays native with a collated format for timestamp input (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql + datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.1/CometCollationSuite` | 136 | from_unixtime rejects non-UTF8_BINARY collated format (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.1/CometCollationSuite` | 149 | $functionName with collated arguments has no native opt-in (codegen=$codegenEnabled, issue #6080) [loop: make_timestamp\|to_unix_timestamp x codegen false\|true = 4 instances] | keep |  |
   | `spark-4.1/CometCollationSuite` | 180 | convert_timezone rejects non-UTF8_BINARY collated timezone (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.1/CometCollationSuite` | 187 | trunc falls back with collated format when codegen is disabled (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.1/CometCollationSuite` | 193 | date_trunc falls back with collated format when codegen is disabled (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.1/CometCollationSuite` | 200 | date_format falls back with collated format when codegen is disabled (issue #4646) | convert | datetime/routing_datetime_collation_disabled.sql |
   | `spark-4.1/CometCollationSuite` | 207 | next_day uses native path with collated dayOfWeek when allowIncompatible is enabled | convert | datetime/next_day_collation_allow_incompatible.sql (new) |
   | `spark-4.1/CometCollationSuite` | 224 | date_format uses native path with collated format when allowIncompatible is enabled | blocked: #6618 | datetime/date_format_collation_allow_incompatible.sql (new) |
   | `spark-4.1/CometCollationSuite` | 236 | trunc routes collated format through codegen dispatcher (issue #4646) | covered | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.1/CometCollationSuite` | 241 | date_trunc routes collated format through codegen dispatcher (issue #4646) | covered | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.1/CometCollationSuite` | 247 | date_format routes collated format through codegen dispatcher (issue #4646) | convert | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.1/CometCollationSuite` | 253 | next_day routes collated dayOfWeek through codegen dispatcher (issue #5591) | covered | datetime/routing_datetime_collation_enabled.sql |
   | `spark-4.1/CometCollationSuite` | 261 | datetime expressions still run with default UTF8_BINARY collation (issue #4646) | convert | datetime/unix_timestamp.sql |
   | `spark-4.x/CometSortCollationSuite` | 51 | window sort with UTF8_LCASE and multiple keys falls back to Spark (issue #6158) | convert | ../windows/window_sort_collation.sql (new) |
   | `spark-4.x/CometSortCollationSuite` | 65 | sort type check rejects non-default collation in every key and nested type | keep |  |
   | `spark-4.x/CometSortCollationSuite` | 93 | sort type check preserves default collation and existing single-key restrictions | keep |  |
   | `spark-4.x/CometSortCollationSuite` | 118 | window sort rejects collated keys after another key and inside nested types | convert | ../windows/window_sort_collation.sql (new) |
   | `spark-4.x/CometSortCollationSuite` | 135 | local sort after round-robin shuffle rejects non-default collation | convert | string/collation_sort_round_robin.sql (new) |
   | `spark-4.x/CometSortCollationSuite` | 160 | top-k with multiple keys rejects non-default collation | convert | string/collation_sort.sql (new) |
   | `spark-4.x/CometSortCollationSuite` | 182 | default UTF8_BINARY string and multi-key struct sorting remain native | convert | ../windows/window_sort_collation.sql (new) + string/collation_sort.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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org