Tyagiquamar opened a new pull request, #25949:
URL: https://github.com/apache/datafusion/pull/25949
Fixes #25945
## Root cause
In `Unparser::handle_timestamp` (datafusion/sql/src/unparser/expr.rs), a
timestamp `ScalarValue` that carries a time zone is rendered as a tz-aware
string via `Dialect::timestamp_with_tz_to_string`, but the wrapping `CAST` was
always built with `self.dialect.timestamp_cast_dtype(&time_unit, &None)`: the
tz was hard-coded away. The generated SQL was therefore e.g.
```
CAST('2024-01-01T11:00:00+08:00' AS TIMESTAMP)
```
When PostgreSQL or DuckDB evaluates that SQL with a UTC session zone, the
offset in the string is discarded and the value silently shifts by the offset
(8 hours here). Unparsing an `Expr::Cast` to the same `Timestamp(_, Some(tz))`
type already routed the tz through `timestamp_cast_dtype` and produced
`TIMESTAMP WITH TIME ZONE`, so literals and casts disagreed.
## Fix
Pass the literal's tz through to `Dialect::timestamp_cast_dtype` instead of
`&None`, so each dialect renders its own tz-aware timestamp type for literals
exactly as it does for casts.
Per-dialect behavior after the fix:
| Dialect | `Timestamp(_, Some(tz))` literal | Unparsed |
|---|---|---|
| Default, PostgreSQL, DuckDB, Snowflake, Custom (default builder) |
`TIMESTAMP WITH TIME ZONE` (matches what the same dialect emits for a `CAST`
with tz) |
| MySQL (`DATETIME`), SQLite (`TEXT`), BigQuery (`TIMESTAMP`) | unchanged:
these dialects intentionally map tz to their naive/own type via
`timestamp_cast_dtype` |
| Custom dialects | fully respected: `with_timestamp_cast_dtype` tz variant
now also applies to tz literals |
## Regression test
The existing `test_timestamp_with_tz_format` and `expr_to_sql_ok` tests
pinned the buggy output; their tz-literal expectations are updated to
`TIMESTAMP WITH TIME ZONE`. The BigQuery tz-literal cases in
`test_timestamp_with_tz_format` keep `TIMESTAMP`, documenting the intended
BigQuery dialect behavior; `test_bigquery_dialect_overrides` and the
custom-dialect tests are unchanged and still pass.
## Validation
rustc 1.98.1 (matches rust-toolchain.toml), host Windows:
```
cargo test -p datafusion-sql --lib
test result: ok. 92 passed; 0 failed
cargo test -p datafusion-sql
ok. 598 passed; 0 failed (datafusion-sql integration)
cargo fmt -p datafusion-sql -- --check # exit 0
cargo clippy -p datafusion-sql --all-targets # no warnings
```
`cargo test -p datafusion --lib` fails 118 tests both before and after this
change on this machine: all of them panic on missing
`ARROW_TEST_DATA`/`PARQUET_TEST_DATA` git submodules (unrelated to the
unparser). `datafusion-sql` has no submodule-dependent tests and is the crate
touched here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
