-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Proposal: Casting between timestamp and timestamptz should be disallowed in favor of "at time zone" #16839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
The request in #12040 is interesting and probably pretty easy to do. The discussion in #11570 goes over why this request is difficult. Our current functionality matches Postgres: hawkfish=# select '2020-01-01T00:00:00+0000'::timestamptz as t;
t
------------------------
2019-12-31 16:00:00-08
(1 row) |
And PG matches the standard. But I can see how it is not friendly(!) |
This is not hard for ICU. Do you want it for when ICU is not loaded (it is just a straight copy in that case as the TZ is implicitly |
personally, I don't need it without ICU |
What is "ICU" short for? I have never looked into the DuckDB source code, so I don't know any details here. But I do remember, in previous versions of DuckDB, seeing error messages along the lines of "Cast from XXX to YYY is not implemented", can't you simply remove this cast? I suppose the problem here is that disabling it at runtime is difficult, vs compile time, and that you want the default configuration to match PG? |
ICU is a third-party library, I believe this is the link https://icu.unicode.org/, relevant here is https://icu.unicode.org/tznotice |
Yes (International Components for Unicode). We use it for time zone and collation support. It is built in and usually loaded in most environments (but not e.g., WASM). I was asking because the ICU extension overrides the cast, but when it is not loaded, the default is to just copy the instant, which is like being in UTC. Since the case is not a problem with UTC I was going to leave it alone. The change is going into the binder (because we can't just remove it and extensions get loaded before you can change the setting) but I could also check whether the current tz is UTC? |
* Add test. fixes: duckdb#16839
* PR Feedback: Better message. fixes: duckdb#16839
* PR Feedback: Better message test... fixes: duckdb#16839
* Nail down ICU settings. fixes: duckdb#16839
* Add a setting to disable timestamp => timestamptz casts in ICU (for safety) fixes: duckdb#16839
* Add test. fixes: duckdb#16839
* PR Feedback: Better message. fixes: duckdb#16839
* PR Feedback: Better message test... fixes: duckdb#16839
* Nail down ICU settings. fixes: duckdb#16839
* Add a setting to disable timestamp => timestamptz casts in ICU (for safety) fixes: duckdb#16839
I see this was implemented. Thank you, looking forward to using it! |
Ran into this myself now, in an even more frightening way: >>> duckdb.sql("set timezone='UTC'")
>>> duckdb.sql("select least('2020-01-01T12:00:00+0000'::timestamptz, '2020-01-01T09:00:00')")
┌────────────────────────────────────────────────────────────────────────────────────────────┐
│ least(CAST('2020-01-01T12:00:00+0000' AS TIMESTAMP WITH TIME ZONE), '2020-01-01T09:00:00') │
│ timestamp with time zone │
├────────────────────────────────────────────────────────────────────────────────────────────┤
│ 2020-01-01 09:00:00+00 │
└────────────────────────────────────────────────────────────────────────────────────────────┘ I assume this also disables auto casting. |
Issue duckdb/duckdb#16839: Disable TIMESTAMP Casts (duckdb/duckdb#16899)
Issue duckdb/duckdb#16839: Disable TIMESTAMP Casts (duckdb/duckdb#16899)
Issue duckdb/duckdb#16839: Disable TIMESTAMP Casts (duckdb/duckdb#16899)
Uh oh!
There was an error while loading. Please reload this page.
What happens?
Should the cast operation below really be allowed?
I'm aware that the DuckDB way of working with timezones is definitely not like doing so with pandas/polars etc. I always run
to avoid issues between systems. However, this cast being allowed seems very frightening. It allows for the following bug:
data3
here will now appear as2019-12-31 19:00:00+00
. Is it possible to disable this cast, and force the use ofat time zone
?To Reproduce
...
OS:
Linux
DuckDB Version:
1.2.2-dev96
DuckDB Client:
Python
Hardware:
No response
Full Name:
Jakob Stigenberg
Affiliation:
Qubos Systematic
What is the latest build you tested with? If possible, we recommend testing with the latest nightly build.
I have tested with a nightly build
Did you include all relevant data sets for reproducing the issue?
Not applicable - the reproduction does not require a data set
Did you include all code required to reproduce the issue?
Did you include all relevant configuration (e.g., CPU architecture, Python version, Linux distribution) to reproduce the issue?
The text was updated successfully, but these errors were encountered: