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:45:37 UTC
[I] test: move datetime expression coverage from Scala suites to Comet SQL tests [datafusion-comet]
andygrove opened a new issue, #6625:
URL: https://github.com/apache/datafusion-comet/issues/6625
Part of #6615.
## What / Why
This issue moves datetime expression coverage to Comet SQL tests under `sql-tests/expressions/datetime/`. The sources are `CometTemporalExpressionSuite` and the datetime tests in `CometExpressionSuite` (`date_add`/`date_sub`, `hour`/`minute`/`second`, `trunc`/`date_trunc`, INT96 timestamps, `from_unixtime`, day-of-week, date partition columns). String-to-timestamp/date casts, including all of `CometDateTimeUtilsSuite`, are in the string-cast issue.
Static analysis on `main` at 3bc2faa934 found the following:
| Suite | Covered | Convert | Blocked | Split | Keep | Total |
|---|---|---|---|---|---|---|
| `CometTemporalExpressionSuite` | 5 | 24 | 1 | 0 | 7 | 37 |
| `CometExpressionSuite` | 1 | 18 | 0 | 0 | 1 | 20 |
| **Total** | **6** | **42** | **1** | **0** | **8** | **57** |
The Scala tests pin far more than the fixtures do, which is why few are covered:
- `date_trunc`: 15 formats, pinned in UTC and in non-UTC zones on the native, dispatch and fallback routes. Fixtures have at most 6 formats, mostly under allowIncompatible.
- `date_format`: 23 native formats and the non-UTC routes. Fixtures have 3 native formats.
- `unix_timestamp` is tested across zones and on NTZ and date input; the fixtures are UTC-only.
- `hour`/`minute`/`second` on NTZ are tested across four zones.
Closed PR #4765 migrated four of these tests and concluded that the fuzz-based tests, `Days`/`Hours` and the operator-only allowIncompatible test could not move. Only the `Days`/`Hours` part holds up.
- **Fuzz tests.** They reduce to curated values. `FuzzDataGenerator`'s default base date resolves to about 3333-01-01, so fuzz timestamps are winter dates around the 3332/3333 boundary, written as INT96 and never NULL. Use far-future winter values to mirror them; summer far-future values in DST zones would test behaviour the Scala tests never covered.
- **The operator-only test (L737).** It converts with rows at 12:00 UTC, so `expect_native(date_format)` can compare answers too.
## Work
Line numbers refer to `CometTemporalExpressionSuite` unless marked `CometExpressionSuite`.
**New fixtures**
- `trunc_timestamp_formats.sql` (L145, L170)
- UTC.
- Data: far-future INT96 timestamps with millisecond fractions, a pre-epoch value and NULL.
- Add a format column with valid, mixed-case, invalid and NULL values.
- `expect_native(date_trunc)` for each of the 15 formats, and `expect_dispatch(date_trunc)` for `'invalid'` and for the format column.
- `trunc_timestamp_dispatch.sql` (L200): matrix over Los_Angeles, London and Tokyo, with 15 `expect_dispatch(date_trunc)` queries.
- `trunc_timestamp_non_utc_fallback.sql` (L222)
- New_York and London with the dispatcher off.
- Assert `expect_fallback(date_trunc: spark.comet.exec.scalaUDF.codegen.enabled=false)`.
- `trunc_timestamp_non_utc_native.sql` (L239)
- Set `TruncTimestamp.allowIncompatible=true` and turn the dispatcher off.
- Matrix over New_York, London and Tokyo, with data from 2024-05-21 to 2024-07-10.
- Assert `expect_native(date_trunc)`.
- `trunc_timestamp_ntz.sql` (L317, `CometExpressionSuite` L1150)
- Matrix over four zones, with NTZ values in summer 2024 and all 15 formats.
- No fixture runs `date_trunc` on NTZ today.
- `unix_timestamp_timezones.sql` (L264, L273, L285)
- Timestamp, date and NTZ columns with far-future, DST-date, pre-epoch and NULL values.
- Matrix over four zones, with `expect_native(unix_timestamp)`.
- `unix_timestamp_collation.sql` (L457, L481, `MinSparkVersion: 4.0`)
- Use an allowIncompatible matrix.
- Assert `expect_dispatch(unix_timestamp)` for string input with a collated format or subject.
- Assert `expect_native(unix_timestamp)` for date, timestamp and NTZ input.
- `hour_minute_second_ntz.sql` (L301): matrix over four zones, with `expect_native(hour, minute, second)` on NTZ.
- `date_format_native_formats.sql` (L603, L620, L650)
- UTC.
- Every key in `CometDateFormat.supportedFormats` (23) as `expect_native(date_format)`, over far-future, AM/PM-edge, pre-epoch and NULL values.
- Add five literal `TIMESTAMP '...'` queries.
- `date_format_non_utc_fallback.sql` (L721).
- `date_format_non_utc_native.sql` (L737)
- Set `DateFormatClass.allowIncompatible=true` and turn the dispatcher off.
- Matrix over New_York, London and Tokyo, with rows at 12:00 UTC.
- `date_format_ntz.sql` (L349): matrix over four zones and 22 formats.
- `timestamp_ntz_cross_timezone.sql` (L371)
- Use a read-zone matrix: London, New_York, UTC and Auckland.
- NTZ writes don't depend on the zone, so the Scala test's separate write zone adds nothing.
- `date_add_overflow.sql` (`CometExpressionSuite` L722): `query ignore(https://github.com/apache/datafusion-comet/issues/2539)` over `d + 2147483647`.
- `date_sub_overflow.sql` (`MaxSparkVersion: 3.5`) and `date_sub_overflow_spark4.sql` (`MinSparkVersion: 4.0`) (L769)
- `d - 2147483647`.
- Assert `expect_error(integer overflow)` on Spark 3.x and `expect_error(EXPRESSION_DECODING_FAILED)` on Spark 4.
- `int96_timestamp_conversion.sql` (L983, L1213)
- Set `spark.sql.parquet.int96TimestampConversion=true`.
- That conf turns off the whole Comet extension, so this leg compares Spark with Spark. Use `spark_answer_only`, or drop this leg.
- The conversion=false legs are covered once the trunc fixtures have all 15 formats, because INT96 is Spark's default timestamp output type.
- `date_timestamp_literals.sql` (L693)
- `WHERE d > DATE'2020-01-01' AND ts < TIMESTAMP'2020-01-01 00:00:00'`, over data on both sides of the boundary.
- The Scala data never passes this filter.
- `dayofweek_weekday.sql` (L2308)
- Set `spark.sql.parquet.datetimeRebaseModeInWrite=CORRECTED`.
- A DATE column covering the epoch week, leap days, 1900-01-01, 0001-01-01 and 9999-12-31.
- `date_partition_column.sql` (L2279): a `PARTITIONED BY (cal_dt)` table, `CAST(cal_dt AS STRING)`, and `SHOW PARTITIONS` as `spark_answer_only`.
**Existing fixtures to extend**
- `trunc_date.sql` (L82, `CometExpressionSuite` L1080)
- Add the `yyyy`, `yy`, `mon`, `mm` and `week` formats, plus the upper-case aliases `YEAR`, `YYYY`, `YY`, `QUARTER`, `MON`, `MONTH`, `MM` and `WEEK`, as `expect_native(trunc)`.
- Add dates across the 3332/3333 boundary.
- Add `expect_dispatch(trunc)` for `'invalid'` and for a format column.
- `trunc_date_enabled.sql` (L1094): the same aliases as values in the format column.
- `trunc_timestamp.sql` (L1113)
- Add the 11 missing formats, over pre-epoch fractional values and one microsecond-precision value.
- Pin the session time zone. Today the file sets `TruncTimestamp.allowIncompatible=true` without one, so on a non-UTC machine it runs the Incompatible native path in that machine's zone.
- `trunc_timestamp_enabled.sql` (L1187): all 15 formats as format-column values, plus an NTZ column and pre-epoch values.
- `unix_timestamp_strings.sql` (L423): add `ConfigMatrix: spark.comet.expression.UnixTimestamp.allowIncompatible=false,true`.
- `date_format.sql` (L703)
- Replace the Los_Angeles `Config` with a matrix over New_York, Los_Angeles, London and Tokyo.
- Switch the queries to `expect_dispatch(date_format)`.
- `unix_date.sql` (L758)
- Change `spark_answer_only` to `query`, since `CometUnixDate` is a native cast.
- Add 1960-01-01, 1970-01-02 and dates near 3333.
- `date_add.sql` / `date_sub.sql` (L707, L741, L755, L789): add tinyint and smallint day counts, using `2Y`/`2S` literals and tinyint/smallint columns.
- `hour.sql`, `minute.sql`, `second.sql` (L934)
- Add pre-epoch timestamps with fractional seconds.
- Fix the stale comments saying TimestampNTZ is Incompatible and dispatched (#3180). `CometHour`, `CometMinute` and `CometSecond` have no NTZ check, and the native kernel extracts NTZ fields directly.
- `from_unix_time_enabled.sql` (L2334)
- Change the default-format queries from `spark_answer_only` to `query`.
- Add a string-input column and seconds in -128..127.
- Add an Asia/Kathmandu variant.
**Blocked**
- **#6617: L51** (`next_day` / `make_date` ANSI exception parity).
- The test checks exception class, error class, SQLSTATE, message parameters and exact message, and that the error is not a `CometNativeException`.
- Interim: `make_date(1000000000, 1, 1)` and `make_date(1000000000, 13, 0)` as `expect_error`.
## Stays in Scala
- **L795, L805, L819, L885, L899.** `Days` / `Hours` are `Unevaluable` partition transforms built with `getColumnFromExpression`. SQL cannot call them.
- **L111** unit-tests `CometTimeZone.nativeId`. Eight of its IDs are covered end to end by `session_timezone_ids.sql`; EST, GMT and +00:00 could be added there.
- **L138** checks the native tzdata version and a warning-text helper.
- **`CometExpressionSuite` L960** asserts that Spark constant-folds `hour`/`minute`/`second`, which is optimizer plan shape.
## Fixture problems to fix along the way
- **NULL literal arguments are folded by `NullPropagation`** and never reach Comet: `last_day.sql:29`, `date_diff.sql:37`, `unix_date.sql:29`, `date_format_time_parser_policy.sql:48-49`, `next_day.sql:73-74` (the whole query) and `make_date.sql:57-64`. Read NULLs from a column instead.
- **`date_format_time_parser_policy.sql` uses `spark_answer_only`** although the plan is native (UTC with whitelisted formats).
- **The leap-year `datediff` literals** look like calendar tests, but `CometCast` parses the `DATE('...')` literals on the JVM. Native code only subtracts two integers. Use date columns.
## 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.
<details><summary>Every Scala test in scope, with its verdict and target fixture</summary>
| Suite | Line | Test | Verdict | Target |
|---|---|---|---|---|
| `CometExpressionSuite` | 693 | date and timestamp type literals | convert | datetime/date_timestamp_literals.sql (new) |
| `CometExpressionSuite` | 707 | date_add with int scalars | convert | datetime/date_add.sql |
| `CometExpressionSuite` | 722 | date_add with scalar overflow (ignored, TODO #2539) | convert | datetime/date_add_overflow.sql (new) |
| `CometExpressionSuite` | 741 | date_add with int arrays | convert | datetime/date_add.sql |
| `CometExpressionSuite` | 755 | date_sub with int scalars | convert | datetime/date_sub.sql |
| `CometExpressionSuite` | 769 | date_sub with scalar overflow | convert | datetime/date_sub_overflow.sql (new, MaxSparkVersion: 3.5) + datetime/date_sub_overflow_spark4.sql (new, MinSparkVersion: 4.0) |
| `CometExpressionSuite` | 789 | date_sub with int arrays | convert | datetime/date_sub.sql |
| `CometExpressionSuite` | 934 | hour, minute, second | convert | datetime/hour.sql + datetime/minute.sql + datetime/second.sql |
| `CometExpressionSuite` | 960 | time expressions folded on jvm | keep | |
| `CometExpressionSuite` | 983 | hour on int96 timestamp column | convert | datetime/int96_timestamp_conversion.sql (new) |
| `CometExpressionSuite` | 1080 | trunc | convert | datetime/trunc_date.sql |
| `CometExpressionSuite` | 1094 | trunc with format array | convert | datetime/trunc_date_enabled.sql |
| `CometExpressionSuite` | 1113 | date_trunc | convert | datetime/trunc_timestamp.sql |
| `CometExpressionSuite` | 1150 | date_trunc with timestamp_ntz | convert | datetime/trunc_timestamp.sql |
| `CometExpressionSuite` | 1187 | date_trunc with format array | convert | datetime/trunc_timestamp_enabled.sql |
| `CometExpressionSuite` | 1213 | date_trunc on int96 timestamp column | convert | datetime/int96_timestamp_conversion.sql (new) + datetime/trunc_timestamp.sql |
| `CometExpressionSuite` | 2279 | date partition column does not forget date type | convert | datetime/date_partition_column.sql (new) |
| `CometExpressionSuite` | 2293 | DatePart functions: Year/Month/DayOfMonth/DayOfWeek/DayOfYear/WeekOfYear/Quarter | covered | datetime/datetime.sql |
| `CometExpressionSuite` | 2308 | dayofweek/weekday over a date column, including ancient dates | convert | datetime/dayofweek_weekday.sql (new) |
| `CometExpressionSuite` | 2334 | from_unixtime | convert | datetime/from_unix_time_enabled.sql + a new Asia/Kathmandu variant |
| `CometTemporalExpressionSuite` | 51 | next_day and make_date ANSI errors match Spark exceptions | blocked: #6617 | datetime/next_day_ansi.sql; datetime/make_date_ansi.sql |
| `CometTemporalExpressionSuite` | 82 | trunc (TruncDate) | convert | datetime/trunc_date.sql |
| `CometTemporalExpressionSuite` | 111 | session timezone IDs are normalized for native code | keep | |
| `CometTemporalExpressionSuite` | 138 | native tzdata version is reported | keep | |
| `CometTemporalExpressionSuite` | 145 | date_trunc (TruncTimestamp) - reading from DataFrame | convert | datetime/trunc_timestamp_formats.sql (new) |
| `CometTemporalExpressionSuite` | 170 | date_trunc (TruncTimestamp) - reading from Parquet | convert | datetime/trunc_timestamp_formats.sql (new) |
| `CometTemporalExpressionSuite` | 200 | date_trunc - non-UTC timezone routes via codegen dispatcher | convert | datetime/trunc_timestamp_dispatch.sql (new) |
| `CometTemporalExpressionSuite` | 222 | date_trunc - non-UTC timezone falls back when codegen dispatcher disabled | convert | datetime/trunc_timestamp_non_utc_fallback.sql (new) |
| `CometTemporalExpressionSuite` | 239 | date_trunc - non-UTC timezone takes native path when allowIncompatible is enabled | convert | datetime/trunc_timestamp_non_utc_native.sql (new) |
| `CometTemporalExpressionSuite` | 264 | unix_timestamp - timestamp input | convert | datetime/unix_timestamp_timezones.sql (new) |
| `CometTemporalExpressionSuite` | 273 | unix_timestamp - date input | convert | datetime/unix_timestamp_timezones.sql (new) |
| `CometTemporalExpressionSuite` | 285 | unix_timestamp - timestamp_ntz input | convert | datetime/unix_timestamp_timezones.sql (new) |
| `CometTemporalExpressionSuite` | 301 | hour/minute/second - timestamp_ntz input | convert | datetime/hour_minute_second_ntz.sql (new) |
| `CometTemporalExpressionSuite` | 317 | date_trunc - timestamp_ntz input | convert | datetime/trunc_timestamp_ntz.sql (new) |
| `CometTemporalExpressionSuite` | 349 | date_format - timestamp_ntz input | convert | datetime/date_format_ntz.sql (new) |
| `CometTemporalExpressionSuite` | 371 | timestamp_ntz - cross-timezone Parquet round-trip | convert | datetime/timestamp_ntz_cross_timezone.sql (new) |
| `CometTemporalExpressionSuite` | 423 | unix_timestamp - string input uses codegen dispatch | convert | datetime/unix_timestamp_strings.sql |
| `CometTemporalExpressionSuite` | 457 | unix_timestamp - collated strings use codegen even when native is opted into | convert | datetime/unix_timestamp_collation.sql (new) |
| `CometTemporalExpressionSuite` | 481 | unix_timestamp - date and timestamp inputs ignore collated formats and stay native | convert | datetime/unix_timestamp_collation.sql (new) |
| `CometTemporalExpressionSuite` | 528 | last_day | covered | datetime/last_day.sql |
| `CometTemporalExpressionSuite` | 554 | datediff | covered | datetime/date_diff.sql |
| `CometTemporalExpressionSuite` | 603 | date_format with timestamp column | convert | datetime/date_format_native_formats.sql (new) |
| `CometTemporalExpressionSuite` | 620 | date_format with specific format strings | convert | datetime/date_format_native_formats.sql (new) |
| `CometTemporalExpressionSuite` | 650 | date_format with literal timestamp | convert | datetime/date_format_native_formats.sql (new) |
| `CometTemporalExpressionSuite` | 669 | date_format with null | covered | datetime/date_format_enabled.sql |
| `CometTemporalExpressionSuite` | 678 | date_format unsupported format routes via codegen dispatcher | covered | datetime/routing_temporal_enabled.sql |
| `CometTemporalExpressionSuite` | 691 | date_format unsupported format falls back when codegen dispatcher disabled | covered | datetime/routing_temporal_disabled.sql |
| `CometTemporalExpressionSuite` | 703 | date_format with non-UTC timezone routes via codegen dispatcher | convert | datetime/date_format.sql |
| `CometTemporalExpressionSuite` | 721 | date_format with non-UTC timezone falls back when codegen dispatcher disabled | convert | datetime/date_format_non_utc_fallback.sql (new) |
| `CometTemporalExpressionSuite` | 737 | date_format with non-UTC timezone takes native path when allowIncompatible is enabled | convert | datetime/date_format_non_utc_native.sql (new) |
| `CometTemporalExpressionSuite` | 758 | unix_date | convert | datetime/unix_date.sql |
| `CometTemporalExpressionSuite` | 795 | days - date input | keep | |
| `CometTemporalExpressionSuite` | 805 | days - timestamp input | keep | |
| `CometTemporalExpressionSuite` | 819 | days - literal edge cases | keep | |
| `CometTemporalExpressionSuite` | 885 | hours - timestamp input | keep | |
| `CometTemporalExpressionSuite` | 899 | hours - timestamp_ntz input | keep | |
| `CometTemporalExpressionSuite` | 924 | cast TimestampNTZ to Timestamp - DST edge cases | convert | cast/cast_timestamp_ntz.sql |
</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