Skip to content
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

Timezone UTC is not considered equal to +00:00 #10957

Closed
samuelcolvin opened this issue Jun 17, 2024 · 6 comments · Fixed by #10960
Closed

Timezone UTC is not considered equal to +00:00 #10957

samuelcolvin opened this issue Jun 17, 2024 · 6 comments · Fixed by #10960
Labels
bug Something isn't working

Comments

@samuelcolvin
Copy link
Contributor

Describe the bug

Running

select now() - start_timestamp from records limit 5;

Results in an error:

Cannot coerce arithmetic expression Timestamp(Nanosecond, Some("+00:00")) - Timestamp(Microsecond, Some("UTC")) to valid types

As far as I know UTC is alwasy identical to +00:00 so these two should be considered equivilant?

To Reproduce

No response

Expected behavior

Timezones which are practically equivalent — e.g. have the same offset and DST semantics should allow subtraction to get an interval.

Additional context

No response

@samuelcolvin samuelcolvin added the bug Something isn't working label Jun 17, 2024
@marvinlanhenke
Copy link
Contributor

marvinlanhenke commented Jun 17, 2024

Probably this is the reason why...both str are simply compared. I'm not sure if it makes sense to just 'add' another rule when checking...or if one has to come up with a better fix / design if there are other cases that have to be supported or checked as well?

@samuelcolvin
Copy link
Contributor Author

Technically now() is wrongly defined, it explicitly states it's in "UTC" in the docs.

@samuelcolvin
Copy link
Contributor Author

I'd be happy to create a pool to start adding special cases, but that sounds like a rabbit hole.

@alamb
Copy link
Contributor

alamb commented Jun 18, 2024

I think the chrono-tz crate has the special cases / knows how to handle converting "UTC" to "+0:00" so I don't think we would have to make a special datafusion map

The arrow crate has a "chrono-tz" feature which activates it. I wonder if we have to turn that on somewhere 🤔 https://crates.io/crates/arrow or maybe use an appropriate check in datafusion

@alamb
Copy link
Contributor

alamb commented Jun 18, 2024

I am responding to @marvinlanhenke 's comment #10960 (comment)

Since "+00:00" and "UTC" are the 'same' - is this viable? Or do we have to extend the logic here in order to provide both variants (and possibly not introduce a breaking change)?

I think that "UTC" == "+00:00" might be a reasonable special case to include

So perhaps we could update the timezone comparison check to compare those two types the same 🤔

@alamb
Copy link
Contributor

alamb commented Jun 20, 2024

#10960 from @marvinlanhenke is up for review 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants