fix: zero-pad years below 1000 in SQL date and datetime literals#2797
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes cross-platform year formatting for SQL DATE/TIMESTAMP literals and Soda Cloud date serialization when the year is below 1000 (e.g., 0200), avoiding invalid literals on glibc-based environments (notably Spark/Databricks and Trino).
Changes:
- Replace
strftime("%Y-...")usage withdate.isoformat()/datetime.isoformat(...)for SQL literals and JSON serialization to guarantee 4-digit years on all platforms. - Update Trino timestamp literal formatting to use
isoformat(..., timespec="microseconds"), including coloned timezone offsets for tz-aware values. - Add unit tests covering the <1000-year padding behavior across core, Databricks, SQL Server, and Trino dialects.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| soda-core/src/soda_core/common/sql_dialect.py | Switches base literal_date to date.isoformat() to ensure 4-digit years in SQL DATE literals. |
| soda-core/src/soda_core/common/soda_cloud.py | Switches to_jsonnable date serialization to date.isoformat() for consistent 4-digit years in wire payloads. |
| soda-sqlserver/src/soda_sqlserver/common/data_sources/sqlserver_data_source.py | Fixes SQL Server literal_date override to use date.isoformat() for 4-digit years. |
| soda-trino/src/soda_trino/common/data_sources/trino_data_source.py | Uses datetime.isoformat(sep=" ", timespec="microseconds") for Trino timestamp literals (naive and tz-aware). |
| soda-tests/tests/unit/test_sql_generation.py | Adds a core SQL AST test asserting padded DATE '0200-…' literals and INSERT generation. |
| soda-databricks/tests/unit/test_databricks_dialect.py | Adds a Databricks dialect test asserting padded DATE '0200-…' literals. |
| soda-sqlserver/tests/unit/test_sqlserver_dialect.py | Adds a SQL Server dialect test asserting padded CAST('0200-…' AS DATE) output. |
| soda-trino/tests/unit/test_trino_dialect.py | Adds Trino dialect tests asserting padded timestamp years and tz-aware offset rendering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Niels-b
left a comment
Niels-b
left a comment
There was a problem hiding this comment.
Pre-approving, assuming all CI passes in extensions
C strftime("%Y") does not zero-pad years < 1000 on glibc, so a date like
0200-12-17 rendered as DATE '200-12-17' — an INVALID_TYPED_LITERAL for
Spark/Databricks that broke failed-row-keys inserts into the diagnostics
warehouse. Switch to isoformat(), which always emits a 4-digit year, in
the base SqlDialect.literal_date, the SQLServer literal_date override,
the Trino literal_datetime overrides, and Soda Cloud payload date
serialization.
SCS-1320
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3f45be7 to
9a0ab49
Compare
|
|
@Niels-b double checking before I merge (I cannot meaningfully verify that the changed code is right) - the extensions CI passed with replay off, so good to merge? |
|
@m1n0, if all of this runs with replay=off, then it should be okay to merge from my pov. The tests there should cover this change AFAIK. |
Description
What problem are you solving?
Failed-row-keys inserts into the Databricks diagnostics warehouse fail with
[INVALID_TYPED_LITERAL] The value of the typed literal "DATE" is invalid: '200-12-17'when a source date has a year below 1000 (customer report: SCS-1320).Root cause:
SqlDialect.literal_dateformatted dates withstrftime("%Y-%m-%d"), and Cstrftime("%Y")does not zero-pad years < 1000 on glibc (where scan pods run) — it does pad on macOS, so the bug never reproduces on a dev Mac.date.isoformat()always emits a 4-digit year on every platform.Fixed the same pattern at every
%Ysite that feeds SQL literals or wire payloads:SqlDialect.literal_date(inherited by Databricks, Spark, Postgres, Snowflake, …)SqlServerSqlDialect.literal_date(same bug in its override)literal_datetime/literal_datetime_with_tz(same bug for timestamps)to_jsonnabledate serialization insoda_cloud.pyAny expected impact on downstream packages/services?
For years ≥ 1000 all outputs are byte-identical except Trino's tz-aware timestamp offset, which now renders
+02:00(the format Trino documents) instead of+0200. Grepped soda-extensions: nothing pins the old formats.Reference issue: SCS-1320
Checklist
🤖 Generated with Claude Code