Specific timezone support for `to_timetamp*()`

[velvia]

Describe the bug
I'm not sure I'd call it a "bug" per se, but all of the to_timestamp*() functions output TimestampTypes in Arrow that have a Timezone field of None. The issues here are:

• There is no way to specify a specific timezone currently to convert to
• Timestamp types with different timezones are not comparable / compatible types. For example, if I have data with a timezone of Some("UTC") the following fails due to incompatible types: WHERE timestamp_col > to_timestamp("2021-06-21T12:00Z") (because timestamp_col has Some(UTC) but to_timestamp returns None)

I'm aware there was a recent vote to treat the None timestamp like a local timestamp, but this isn't always the case either. For example, we have been using the output of None timestamps from DataFusion but we treat all our timestamps as UTC internally.

To Reproduce
Steps to reproduce the behavior:
I can create a PR to have a test to show the above comparison failing. Take some data with a timezone with UTC and do the above comparison.

Expected behavior
The best solution I can think of would be for to_timestamp(...) to support a second, optional argument where the timezone can be specified. This allows for casting/conversion and generation of timestamps with a specific timezone requirement. There are some technical challenges which can be overcome:

• Current code in datetime_expressions.rs for example uses macros which assume a static, const type, which makes it difficult to input variable timezone args
• The function signature checks would be really complicated

There are other "hacks" which are possible:

• For our use case, Some("UTC") is equivalent to None, so we could make Some("UTC") cast to None and vice versa, but this would lose precision for many people.

[velvia]

@alamb @westonpace .... your thoughts on this matter would be really welcome. :)

To be specific, if you do to_timestamp(date_string) then it can figure out the timezone from the string, sometimes. However, current architecture of Arrow/DF requires a specific timezone output type.
If you did to_timestamp(int64) then you would not have any timezone info at all, and would require the input in the second argument no matter what for completeness.

There is also the possibility to simplify things and eliminate the timezone field from Arrow, but that's a really big change. For example, Parquet does not store the timezone but simply has a isAdjustedToUTC field. If True, that means timestamps are UTC, and if not that means local timestamp.
https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#timestamp

[westonpace]

I'm not sure I'd call it a "bug" per se, but all of the to_timestamp*() functions output TimestampTypes in Arrow that have a Timezone field of None. The issues here are:

This does seem like an inconsistency with the just agreed upon convention. I would expect all of these methods to convert to Timestamp(unit, "UTC").

I'm aware there was a recent vote to treat the None timestamp like a local timestamp, but this isn't always the case either. For example, we have been using the output of None timestamps from DataFusion but we treat all our timestamps as UTC internally.

Correct, I think DataFusion is interpreting Timestamp(unit, None) as "instant". This is still valid, but no longer "conventional" and could cause interoperability problems. That being said, "local" timestamps don't often get stored or travel between implementations. As long as DataFusion treats both Timestamp(unit, None) and Timestamp(unit, UTC) the same then I wouldn't say it is urgent or must-fix. It's probably a topic for discussion within DF.

Timestamp types with different timezones are not comparable / compatible types. For example, if I have data with a timezone of Some("UTC") the following fails due to incompatible types: WHERE timestamp_col > to_timestamp("2021-06-21T12:00Z") (because timestamp_col has Some(UTC) but to_timestamp returns None)

I'm not sure if you are saying the are not comparable or they should not be comparable. They "should" be comparable if you are comparing two columns with a non-None timestamp (e.g. Timestamp(units, "UTC") should be comparable with Timestamp(units, "+0700") but neither should compare to Timestamp(units, None). This is going by the newly discussed "conventional" logic where None is "local" and UTC is "instant. Since DF has it backwards right now you'd need to invert the logic.

The best solution I can think of would be for to_timestamp(...) to support a second, optional argument where the timezone can be specified.

As long as it is optional that is probably a good idea. However, that sounds like it is adding capability and wouldn't fix anything on its own.

There is also the possibility to simplify things and eliminate the timezone field from Arrow, but that's a really big change. For example, Parquet does not store the timezone but simply has a isAdjustedToUTC field. If True, that means timestamps are UTC, and if not that means local timestamp.

I'm not sure exactly what you are saying here. isAdjustedToUTC basically boils down to timezone != null (I might have that backwards). If you are arguing to avoid using timestamps other than UTC or null then I agree that is a viable option. Implementations can always store timestamp info in a separate column or always represent "zoned date times" as structs (given that the only reason to use "zoned date time" in the first place is typically for field extraction).

[westonpace]

To be specific, if you do to_timestamp(date_string) then it can figure out the timezone from the string, sometimes. However, current architecture of Arrow/DF requires a specific timezone output type.
If you did to_timestamp(int64) then you would not have any timezone info at all, and would require the input in the second argument no matter what for completeness.

I don't think to_timestamp(date_string) should always put the time zone into the data type. It should either never do so and have a second, optional time zone argument (for consistency with the other to_timestamp methods) or it should expose a boolean parameter (keep_timezone or something like that). The time zone is needed in order to figure out the correct instant. However, it is often not important beyond that. What happens if you have strings from different time zones?

[alamb]

@velvia

we have been using the output of None timestamps from DataFusion but we treat all our timestamps as UTC internally.

FWIW this is what we do in IOx as well

The best solution I can think of would be for to_timestamp(...) to support a second, optional argument where the timezone can be specified.

I think a second optional argument for to_timestamp is a fine solution that would be backwards compatible and also allow for improvements going forward

For example, if I have data with a timezone of Some("UTC") the following fails due to incompatible types: WHERE timestamp_col > to_timestamp("2021-06-21T12:00Z") (because timestamp_col has Some(UTC) but to_timestamp returns None)

I think a better solution would be for DataFusion to implement coercion rules between date/time/timestamp types. In your example of a timestamp_col with Timestamp(Milliseconds, Some("UTC")), then I would like DataFusion to handle

WHERE timestamp_col > to_timestamp("2021-06-21T12:00Z")

by adding the following cast (please forgive me the pseudo-SQL)

WHERE timestamp_col > cast (to_timestamp("2021-06-21T12:00Z") as timestamp(UTC))

[velvia]

It sounds like everyone agrees second, optional argument to to_timestamp*() to force a specific timezone result works.

As for casting/coercion, we have:

@alamb

I think a better solution would be for DataFusion to implement coercion rules between date/time/timestamp types. In your example of a timestamp_col with Timestamp(Milliseconds, Some("UTC")), then I would like DataFusion to handle

WHERE timestamp_col > to_timestamp("2021-06-21T12:00Z")

by adding the following cast (please forgive me the pseudo-SQL)

WHERE timestamp_col > cast (to_timestamp("2021-06-21T12:00Z") as timestamp(UTC))

The above is saying that Timestamp(_, None) can be coerced into Timestamp(_, Some("UTC")), is that right? (AFAICT, we can't just say that only functions called in WHERE clauses that output None be coerced?) However, a None implies local timestamp, which is not the same as an instant with UTC (although semantically for UrbanLogiq that definitely works. :)

OTOH @westonpace wrote:

I'm not sure if you are saying the are not comparable or they should not be comparable. They "should" be comparable if you are comparing two columns with a non-None timestamp (e.g. Timestamp(units, "UTC") should be comparable with Timestamp(units, "+0700") but neither should compare to Timestamp(units, None). This is going by the newly discussed "conventional" logic where None is "local" and UTC is "instant. Since DF has it backwards right now you'd need to invert the logic.

It seems to me that coercing Timestamp(_, None) to Timestamp(_, "UTC") would not fit the recent convention, at least automatic coercion without context.

So follow up / action items / possibilities:

1. Should the current output of to_timestamp(...) default to Timestamp(_, Some(UTC)) ? This sounds like a yes (and would solve my use case)
2. to_timestamp(....) can be extended to add a second argument to accept different time zones, or None (not sure how to specify None if the default is UTC, per item 1)
3. Coercion ... the details of this needs to be figured out, semantics
4. Or, simplify everything and have the timestamp types not have timezone, but just if they are UTC or local (similar to Parquet).

BTW I looked up the C/C++ API, and the Python API, and none of those appear to support timezones in the timestamp type, just a unit. It could be argued that 4) would bring the Rust API closer to the other languages APIs, and when arrow datatypes are translated from Flight or the binary representation, how is the timezone encoded, if at all?

(if 4 is a worthy avenue to discuss, I can start a email chain)

cheers and thanks

[alamb]

Should the current output of to_timestamp(...) default to Timestamp(_, Some(UTC)) ? This sounds like a yes (and would solve my use case)

While this might be theoretically the best option, I think this would be a fairly painful change (at least for us in IOx) as our timestamps are Timestamp(Nanoseconds, None)

to_timestamp(....) can be extended to add a second argument to accept different time zones, or None (not sure how to specify None if the default is UTC, per item 1)

Perhaps to_timestamp('....', NULL) could work

Or, simplify everything and have the timestamp types not have timezone, but just if they are UTC or local (similar to Parquet).

I am actually a fan of this direction -- I am fairly sure the use of a Timezone that is something other than None will not work well in Arrow / DataFusion (Because I tried using Timestamp(.., Some("UTC") in IOx and basic query stuff didn't work

I think it would keep things simpler to convert timestamps on the edge (aka if it was "local time" convert to UTC prior to putting into DataFusion) and then have all the processing machinery deal with UTC timestamps only

[adamhooper]

The best solution I can think of would be for to_timestamp(...) to support a second, optional argument where the timezone can be specified.

I think a second optional argument for to_timestamp is a fine solution that would be backwards compatible and also allow for improvements going forward

I'm not even a user (yet); I see two big great reasons not to add an argument:

1. Ambiguous calling pattern. When converting string to Arrow timestamp, there are two timezones to consider: the input-string timezone and the output-timestamp timezone. Which one would this argument mean?

If I see the call, TO_TIMESTAMP('2021-01-01T00:00:00', 'America/Montreal'), then only result I can imagine as "useful" is 2021-01-01T05:00Z -- that is, "the input is from my clock." (@westonpace, I think we're aligned here.)

Someone else might expect 2020-12-31T19:00-0500, saying, "I have UTC input and I want Montreal wall-clock time." A third person might expect 2021-01-01Z with timezone-metadata America/Montreal.

If three smart people reading the same line of code would interpret it three different ways, that's a design problem.

2. PostgreSQL compatibility. Postgres has two timestamp types; Arrow timestamp+tz-metadata columns are neither.

In Postgres, TO_TIMESTAMP() creates a TIMESTAMP WITH TIME ZONE -- that is, a moment on our shared, global timeline, stored as 64-bit integer since the UNIX epoch. It has no timezone: each moment happens once everywhere in the world, regardless of timezone.

Postgres also has a TIMESTAMP WITHOUT TIME ZONE type -- that is, a calendar-date-plus-wall-clock type ... but Postgres' own TO_TIMESTAMP() doesn't output it. That's on purpose. I think most people using TIMESTAMP WITHOUT TIME ZONE are using it in error, misled by the name. It represents the very opposite of what it sounds like. In Postgres, it's handy for calendaring apps and not much else. (There are other uses in Pandas/R.)

Postgres has gone 30 years without Arrow's timestamp+tz-metadata types.

In the words of Tom Lane, "if we simply took away to_timestamp(), most users would be better off".

Be careful about adding a feature: removing one might help more users accomplish more tasks, more smoothly.

[alamb]

@adamhooper thank you for the perspectives. I see the challenges with adding a second argument to to_timestamp and would love to consider other alternatives.

Can you suggest a strategy for the usecase described in this ticket, namely "writing a query using SQL against data stored in parquet that has an explicit timezone?"

In other words, how do you suggest supporting the following query?

SELECT ...
FROM my_parquet_file_where_timestamp_col_has_a_specified_timezone
WHERE timestamp_col > to_timestamp("2021-06-21T12:00Z")

Which currently fails because the arrow type of to_timestamp has None as the timezone, but the type of of timestamp_col has a specific timezone?

[adamhooper]

@alamb would anybody be horrified/appalled at the following:

• to_timestamp() always returns TIMESTAMP WITH TIMEZONE -- i.e., UTC.
• my_parquet_file_with_isAdjustedToUTC_true are always TIMESTAMP WITH TIMEZONE -- i.e., UTC. (Parquet doesn't store timezones anyway.)
• my_arrow_ipc_file_with_timezone_america_montreal produces TIMESTAMP WITH TIMEZONE -- i.e., UTC. (DataFusion simply drops the metadata, without warning.)
• my_parquet_file_with_isAdjustedToUTC_false produces an error for now, saying DataFusion doesn't support TIMESTAMP WITHOUT TIME ZONE.
• my_arrow_ipc_file_with_timezone_null gives a deprecation warning and TIMESTAMP WITH TIMEZONE; then, some version in the future, DataFusion re-implements it as TIMESTAMP WITHOUT TIMEZONE.

To me, dropping timezone info is an easy (if rude) decision. DataFusion implements a subset of Postgres, and Postgres has no timezone metadata. There's an argument to be made for passing the timezone through unmodified; but that would be hard/convoluted, probably needing new syntax; and does anybody actually want that? IPC with datetime+timezone tuples is nonstandard and heavyweight; Spark doesn't even have a type for that. Forcing UTC won't stop anybody from accomplishing anything.

So the only hairy decision is what to do with Arrow timezone=null. Up until today, DataFusion users have been using it as TIMESTAMP WITH TIMEZONE, right? If that's the case, then there's great news: nobody who's using DataFusion is using TIMESTAMP WITHOUT TIMEZONE :). Sounds like that type would be a new feature ... and I expect it would be a low priority, since nobody asked for it. (Again, Spark has no such type.)

(Primer on Postgres, for the uninitiated: TIMESTAMP WITH TIME ZONE means UTC; it doesn't store a timezone.)

[velvia]

@AdamHopper personally I'd be OK with your recommendations, but some thoughts:

• There is some interop considerations between TIMESTAMP WITH TIMEZONE ie TimestampType(, UTC) vs TimestampType(, None). The reason is that so far DataFusion has been generating TimestampType(_, None). If this changes to UTC, which makes sense as far as conventions go, then we need some way for people with data with Timestamp(, None) to be able to convert it over to (, UTC).
• I'm not sure it would be acceptable to throw an error at isAdjustedToUTC = false as I'm sure people have Parquet files out there with this, and then DataFusion/Arrow wouldn't be able to read it. But maybe it's OK if you give people a way to cast / convert the timestamps over.

Also, I see your point about to_timestamp() having a timezone argument being confusing. However there is a second use of to_timestamp() right now, which is for conversion purposes, from other types to Timestamp(resolution, tz). The reason it exists like this is because there is no standard SQL/PG way to work with multiple timestamp resolutions, which are required and supported by all Arrow implementations.

Thus, while TO_TIMESTAMP('2021-01-01T00:00:00', 'America/Montreal') might be confusing, I believe the following would be more clear:

• to_timestamp_millis(my_timestamp_col, 'UTC') -> cast my_timestamp_col from int or another timestamp resolution to millisecond resolution, and make sure it has UTC timezone

[adamhooper]

@velvia Great points

My last two suggestions were exactly about interop -- and a transition period. Today, DataFusion users interpret timezone=null to mean TIMESTAMP WITH TIMEZONE; but eventually, DataFusion must consider timezone=null to mean TIMESTAMP WITHOUT TIMEZONE, right? Users will need to change what they're doing; the transition path will be hard and it depends on the DataFusion community.

I hadn't thought of timestamp resolution. Again, I'm out of my element (for now) :).

The crux of my suggestion is to ignore the timezone metadata field (treat it as a boolean, timezone=null or timezone=UTC). That translates Parquet <=> Arrow <=> PostgreSQL cleanly. Treating it like a boolean keeps the feature list small and saves people from confusion.

As for the TO_TIMESTAMP() parameter: Postgres has a different function might do exactly what you're suggesting. AT TIME ZONE toggles the "WITH TIMEZONE-ness" of a timestamp. It's also callable as a function, TIMEZONE(zone, timestamp).

But -- sidetracking here -- how important are these types? TIMESTAMP WITH TIME ZONE is clearly mission-critical. I think TIMESTAMP WITHOUT TIME ZONE has its place; but it's in a different ballpark, right. (Even Spark doesn't have TIMESTAMP WITHOUT TIME ZONE; and, well, does DataFusion?) Arrow's timezone metadata column is even more obscure: I've only heard of it in Pandas and R.

[velvia]

@alamb @westonpace and others: just to continue this conversation and try and bring it to a close.

1. It sounds like the Arrow/Rust community would like to transition to supporting Timestamp(_, UTC) as the standard for UTC-based processing, rather than Timestamp(_, None).
2. Since some people already use the existing Timestamp(_, None) to really mean Timestamp(_, UTC) (including us in some cases), and there is also data (in Parquet) that has at least UTC timezone info, we should provide interop to convert between the two, or at least cast things between.
3. A timezone() function or with_timezone() or at_timezone() function would be clearer than adding a second argument to to_timestamp(...)

Do the above sound about right?

I would also like to observe current obstacles to 1) and 2) in the Arrow/DF codebase (there are more, just what I've found in the last ~2 weeks):

• ScalarValue in DataFusion only supports Timestamp types with None timezone
• Coercion of Timestamp(, UTC) to Timestamp(, None) (or the other way around) is potentially wrong and also has some schema type check issues

[velvia]

To add to the above list, to_timestamp(string) should really return a Timestamp Scalar with the proper timezone derived from parsing the string. Currently no TZ info is returned, but for example if the date time string has "Z" in it, then the resulting scalar timezone should be "UTC".

The bigger question really is how much effort to pour into all this. Since the other Arrow implementations and the Arrow spec already specifies TimeZone, and due to many issues mentioned above, it seems to me that at least we should try to get notion of UTC vs local time zone, and be able to maintain that distinction, correctly. IE, Rust DataFusion should IMHO have first class support of both Timestamp(_, None) as well as Timestamp(_, UTC), everywhere.

[alamb]

Do the above sound about right?

yes that sounds right

I am very supportive of an effort to have first class (and correct) timestamp support in Rust both for Arrow and DataFusion.