Support to_timestamp with 2 arguments (timestamp format)

[alamb]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
DataFusion strives to be postgres compatible, so when there are differences between postgres and datafusion it is confusing

Today, one argument form of to_timestamp works in datafusion:

❯ select to_timestamp('2014-05-11 01:07:34.779000+01:00');
+-------------------------------------------------------+
| totimestamp(Utf8("2014-05-11 01:07:34.779000+01:00")) |
+-------------------------------------------------------+
| 2014-05-11T00:07:34.779                               |
+-------------------------------------------------------+
1 row in set. Query took 0.000 seconds.

But the 2 argument form does not:

❯  select to_timestamp('2014-05-11 01:07:34.779000+01:00', 'YYYY-MM-DD HH:MI:SS.NS');
Plan("Coercion from [Utf8, Utf8] to the signature Uniform(1, [Int64, Timestamp(Nanosecond, None), Timestamp(Microsecond, None), Timestamp(Millisecond, None), Timestamp(Second, None), Utf8]) failed.")

Describe the solution you'd like

Implement the two argument form of to_timestamp as described in

https://www.postgresql.org/docs/current/functions-formatting.html

to_timestamp ( text, text ) → timestamp with time zone

Converts string to time stamp according to the given format. (See also to_timestamp(double precision) in Table 9.33.)

postgres=# select to_timestamp('2014-05-11 01:07:34.779000+01:00', 'YYYY-MM-DD HH:MI:SS.NS');
      to_timestamp
------------------------
 2014-05-11 01:07:34+00
(1 row)

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Thank you to @cannonpalms and @idclark

[waitingkuo]

hi @alamb

do you think we should following postgresql formatting syntax or chorno's https://docs.rs/chrono/latest/chrono/format/strftime/index.html#specifiers

in your example, the input contains timezone

❯  select to_timestamp('2014-05-11 01:07:34.779000+01:00', 'YYYY-MM-DD HH:MI:SS.NS');
Plan("Coercion from [Utf8, Utf8] to the signature Uniform(1, [Int64, Timestamp(Nanosecond, None), Timestamp(Microsecond, None), Timestamp(Millisecond, None), Timestamp(Second, None), Utf8]) failed.")

postgrseql's to_timestamp return timestamptz but datafusion's returns timestamp
do we want to align postgreql's return type as well?

[alamb]

do you think we should following postgresql formatting syntax or chorno's

🤔 that is an excellent question. I think we should follow postgresql ideally though I realize that may be infeasible given we use chrono underneath

postgrseql's to_timestamp return timestamptz but datafusion's returns timestamp

That is also an excellent question @waitingkuo . I don't have an answer but there is more discussion here #686

[alamb]

We changed to_timestamp to follow postgres semantics for single arguments here: #7844

[jhorstmann]

Having recently implemented this in our proprietary engine, these were my notes regarding postgres, compared to other pattern syntaxes:

• Seems very exhaustive but also super complex
• Features include years with commas, multipe variants of AM/PM indicates, roman numerals, complete sub-syntax for number formatting and padding
• In my opinion much too complex to implement, we’d need to define a small subset, at which point we can no longer claim postgres compatibility

We ended up implementing a pattern syntax closer to the one supported by Java SimpleDateFormat, supporting a subset of those pattern characters. Similar syntax is supported by Presto/TrinoDB/Athena and so should be familiar to our users. My personal opinion was also that the percent-based syntax from MySQL or chrono looked a bit ugly, and does not make the width or padding of fields obvious.

Internally we however translated the user-facing syntax to chrono format items and used chrono::format::parse

[alamb]

I would really like to avoid a massive amount of custom postgres compatible format string logic in DataFusion unless there is a super compelling usecase for it. I think it will likely be a large amount of code / tests (though @jhorstmann can correct me if I am wrong0

I would instead prefer to see using the chrono forma strings directly (via chrono::format::parse) perhaps

[Omega359]

When to_date/to_timestamp/etc is used from spark the format is only passingly similar to Java's SimpleDateFormat with enough differences to be annoyingly incompatible except for simple formats ('EEE' is unsupported without additional work, w/W, Y, O, etc). I would not generally recommend the Java format.

[Omega359]

Question on implementation detail while implementing this: should this throw an Err if the timestamp cannot be parsed given either the default behaviour or with any provided formats? Current behaviour seems to be be to throw an Err which mirrors postgresql behaviour, whereas with spark for example the behaviour is to return null for any unparseable string.

I'm asking because with date/timestamp string parsing you are often not dealing with machine generated data but rather human generated and there are only so many formats that can be tried before you must just set the data to null and move on. Not having that ability could be a hindrance for some use cases.

A thought I did have would be to change the behaviour via flag that would be documented as part of the api and user guide.

[alamb]

Question on implementation detail while implementing this: should this throw an Err if the timestamp cannot be parsed given either the default behaviour or with any provided formats? Current behaviour seems to be be to throw an Err which mirrors postgresql behaviour, whereas with spark for example the behaviour is to return null for any unparseable string.

I think we should keep the existing behavior

A thought I did have would be to change the behaviour via flag that would be documented as part of the api and user guide.

I agree a flag (or maybe another function) could be appropriate

This high level observation is that different SQL implementations have different semantics (Decimal handling is another thing that spark seems to do differently). Given DataFusion's built in functions have only one version, we can't mirror both systems.

My long term hope / plan is to pull as many functions out of the core as possible (e.g #8045 ) so that people can more easily customize the behavior. For example, we could have a to_tmestamp that mirrors postgres and a to_timestamp that mirrors spark, and people could choose which one to use for a particular SessionContext

[tustvold]

whereas with spark for example the behaviour is to return null for any unparseable string

We support this in arrow-rs - https://docs.rs/arrow-cast/latest/arrow_cast/cast/struct.CastOptions.html#structfield.safe

[Omega359]

Thanks for your feedback @alamb! Next question: As best as I can tell it's not possible to overload a function to both accept a single string as well as accept a list of strings (no varargs).

This means to me at least that the choice is either a break in the dataframe api (from to_timestamp(col("a")) to to_timestamp(vec![col("a")]) or a new function name. Is an api break acceptable here or is there a way around this?

[alamb]

Thanks for your feedback @alamb! Next question: As best as I can tell it's not possible to overload a function to both accept a single string as well as accept a list of strings (no varargs).

I think yo ucould do it with TypeSignature::one_of