james-johnston-thumbtack commented on PR #21594: URL: https://github.com/apache/flink/pull/21594#issuecomment-1641319553
**I am uncertain if this is the right mapping.** In fact, my sense is that Flink has this completely backwards, even for the existing `TIMESTAMP WITHOUT TIME ZONE` mapping implementation as well. (Caveat: I have very limited experience /w both Flink and Avro and welcome any criticism if I incorrectly grouped a data type below. For example, the data type documentation in Flink is a little thin compared to e.g. PostgreSQL, so I could have misunderstood.) I have noticed that there seem to be different ways in systems of using data types to store timestamps, based on whether we want to represent an absolute moment in time on the global timeline, or just want to represent a date/time without any particular time zone affiliation or global moment in time. (Most applications would want the former.) **Bare timestamps without any time zone adjustments**: this is like looking at the hands of a clock, without having any other context as to what time zone it is in. When reading/writing the value, we don't convert the value to/from UTC because we aren't making any assumption that the stored value is to be in UTC In the first place. In fact, unless time zone information is being provided in some side channel, or other application-specific assumptions made, we don't really know what global moment in time this corresponds to. Maybe the application doesn't even _want_ it to represent a global moment in time. Examples of this include: - `TIMESTAMP WITHOUT TIME ZONE` in [PostgreSQL](https://www.postgresql.org/docs/15/datatype-datetime.html#DATATYPE-DATETIME-INPUT) (see section 8.5.1.3) - `DATETIME` in [Google BigQuery GoogleSQL](https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#datetime_type) - Local timestamp logical type in [Avro](https://avro.apache.org/docs/1.11.1/specification/#local-timestamp-millisecond-precision) like `local-timestamp-millis`: "represents a timestamp in a local timezone, regardless of what specific time zone is considered local" ... "the long stores the number of milliseconds, from 1 January 1970 00:00:00.000" _[note how the documentation does NOT specify an absolute epoch moment in time by saying "UTC"]_ - `TIMESTAMP WITHOUT TIME ZONE` in Flink Table API **Timestamps that are anchored to UTC-based epoch, with automatic time zone adjustments**: this is meant to represent an absolute moment in time. Commonly a system will store the value as seconds since UTC-based epoch, and then provide automatic conversion to/from time zones; in general it is a "time zone aware" type. Examples: - `TIMESTAMP WITH TIME ZONE` in PostgreSQL - `TIMESTAMP` in [Google BigQuery GoogleSQL](https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#timestamp_type) - Regular timestamp logical type in [Avro](https://avro.apache.org/docs/1.11.1/specification/#timestamp-millisecond-precision) like `timestamp-millis`: "represents an instant on the global timeline, independent of a particular time zone or calendar" ... "the long stores the number of milliseconds from the unix epoch, 1 January 1970 00:00:00.000 UTC" [note the presence of "UTC" time zone specifier in the epoch!] - `TIMESTAMP_LTZ` in Flink Table API **It seems that the latter types are what most applications should use**. For example the PostgreSQL "[Don't do this](https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_timestamp_.28without_time_zone.29)" style guide basically says to never use `TIMESTAMP WITHOUT TIME ZONE`. Thus, _assuming_ I have accurately grouped these data types as shown above, I would expect the Flink Avro data mappings to: - Flink's `TIMESTAMP WITHOUT TIME ZONE` should be mapping to `local-timestamp-millis`. Today it maps to `timestamp-millis`, but this seems wrong: the Flink type seems to not be representing a global moment in time, whereas the Avro type clearly does so. (Perhaps this more correct behavior could be configured with a new format option, in order to preserve backwards-compatibility for apps that depend on the wrong mapping?) - In this pull request, I would expect that Flink's `TIMESTAMP_LTZ` should be mapping to `timestamp-millis`, not `local-timestamp-millis`. That's because both Avro `timestamp-millis` and `TIMESTAMP_LTZ` are anchored to a UTC-based epoch time. Mapping to `local-timestamp-millis` doesn't make sense, because `TIMESTAMP_LTZ` represents a global instant in time, whereas the Avro type does not. -- 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]
