ARROW-12232: [Rust][DataFusion] Add SQL support for CAST(.. AS Time) millisecond - #10005

I thought the sql 'Time' format was for hours in the day (aka no date)

Thus I would expect this to be something like DataType::Time32(TimeUnit::Millisecond) or DataType::Time64(TimeUnit::Nanosecond)

I think postgres uses time as only the time in the day as well: https://www.postgresql.org/docs/9.1/datatype-datetime.html

time of day (no date)

@alamb according to the DataFusion README, this shows up:

TIME | Time64(TimeUnit::Millisecond)
-- | --

Is this only for time of the day?

Basically, we have datasets in both nanos and millis, and don't really care for the extra precision of nanos, and a whole bunch of processing code tuned towards millis, so would rather convert everything to millis for convenience. What I really need is a convenient way to cast things to millis, or precisely Timestamp(Milliseconds, None), that works with the SQL parser... what do you recommend?

I'm not sure there is a SQL convention for precision of timestamps like in Arrow. I see some places that have decimal places of precision, which doesn't really map well to our scheme either. We could invent something different, like TIMESTAMP(precision) or allow casting to TIMESTAMP(Nanos/Millis/Micros) etc which would solve the problem... WDYT?

(btw I have a similar problem with the to_timestamp() function too, for which I can create another PR)

@velvia

Is this only for time of the day?

That is my understanding

What I really need is a convenient way to cast things toTimestamp(Milliseconds, None) .. what do you recommend?

I recommend adding a new function that takes a string parameter that is interpreted at an Arrow DataType and calls the appropriate underlying arrow cast function. This way we can specify the arrow type directly without having to worry about an incomplete mapping of SQL type <--> Arrow type

Perhaps something like

select arrow_cast(my_col, 'Timestamp(Milliseconds, None)'

I haven't thought carefully about how we would parse the strings back to DateTime but I think we might be able to press some existing serde code into use, perhaps.

What do you think @jorgecarleitao , @seddonm1 , @returnString ?

There might also be some inspiration in postgres's date/time functions: https://www.postgresql.org/docs/current/functions-datetime.html

Hmmm, I'm def in favour of implementing PostGres compatible support, though there is a lot there, especially with intervals and timestamps, and many operators including math operators to add.

As for date_trunc(), my read of the PostGres functionality is that it is used for date rounding, like I can round something to the nearest hour or minute. This is something definitely useful to add, though I'm more interested in casting to a specific precision.

Here's my proposal:

• add date_trunc('milliseconds', _) -> returns Timestamp(Milliseconds, None)
• add date_trunc('microseconds', _) -> returns Timestamp(Microseconds, None)
• Leave implementation of other time periods to a future PR (there is also a question of what type to return)
• I would also add automatic coercion of Timestamp millis (and micros, and maybe nanos?) to Int64 for fast and easy Int64 comparison, and coercion to nanos for when comparison with nano timestamps matters.

The issue with return types would need to be solved, but the above proposal would work for our needs.
I can also update the bug once we decide on the solution.

thanks,
Evan

I think having the truncation also do the cast for those date_trunc calls is quite reasonable, because:

• it's still a timestamp in sql parlance, and we make no guarantees about the underlying unit of a timestamp presently (afaik? open to corrections here!)
• it doesn't result in any unexpected data loss (by virtue of asking for a truncation in the first place)

In general, for other granularities, it could make sense for date_trunc to internally cast to the most coarse time unit that covers the requested precision (e.g. date_trunc('seconds', timestamp_col) could be converted to TimeUnit::Seconds, but I have no strong opinions there personally 😄

@alamb and others: so I found that there is already an existing date_trunc() function, which always returns Timestamp(Nanos). The problem with the above proposal I put up is:

• return_type() in functions.rs is designed to give the return type knowing only the input argument types. IOW, I cannot produce a different type such as Timestamp(Millis) as I cannot inspect the time granularity value, all I know is that I have args of type utf8 and timestamp(nanos).
• The current function signature is Exact and always takes in inputs of nanosecond timestamp resolution. I suppose we can keep that... but should think about what do people do when they have different timestamp resolutions.

Basically, there is a larger issue in Arrow/Datafusion, in that Timestamp columns with different time resolutions are considered different types, and most of the infrastructure is designed for or works best when there is a single output type (and input type), it is much simpler. Also in the SQL world, for the most part there is just a "Timestamp" type. date_trunc() and other things have been designed to work with nanos, but you can in theory have timestamp columns which have millis, micros, seconds, but they don't have the same support that nanos do.

So the big question (and I'm not sure if this PR is the right forum for this) is which direction should the project be going. There is a mismatch between SQL support, the design to work with one type, and the Arrow support for multiple time resolutions.

I can think of some possible directions/solutions, in no particular order:

• Have most functions work on nano resolution timestamps, and have automatic coercion of other resolutions to nanos when calling functions etc. This imposes a perf penalty, but maybe an acceptable one.
• Add support for specifying the resolution for timestamps. For example, CAST(xx AS TIMESTAMP(Microseconds)) would be possible. There's probably precent for this somewhere. This would need the previous coercion though to work.
• Somehow support just a single timestamp type in Arrow, with multiple possible resolutions. I'm not sure how this would even be represented or implemented, and it'd be a huge change. It might match better how things are implemented in other parts of the SQL world though.

Would love everyone's thoughts on this. For now I'd be super happy with the first two solutions.

Oh, there is also a easier solution. Expand return_type() method in functions.rs to allow passing in the input arrays, so that we could return a different type depending on the time granularity argument.

Here's my proposal:
add date_trunc('milliseconds', _) -> returns Timestamp(Milliseconds, None)
add date_trunc('microseconds', _) -> returns Timestamp(Microseconds, None)
Leave implementation of other time periods to a future PR (there is also a question of what type to return)
I would also add automatic coercion of Timestamp millis (and micros, and maybe nanos?)

I think that sounds reasonable

So the big question (and I'm not sure if this PR is the right forum for this) is which direction should the project be going. There is a mismatch between SQL support, the design to work with one type, and the Arrow support for multiple time resolutions.

I don't think this PR is the right forum -- I think sending something to [email protected] is probably the right place to have that discussion.

Have most functions work on nano resolution timestamps, and have automatic coercion of other resolutions to nanos when calling functions etc. This imposes a perf penalty, but maybe an acceptable one.

this is my personal preferred solution , but I think we should gather input from the larger community.

Oh, there is also a easier solution. Expand return_type() method in functions.rs to allow passing in the input arrays, so that we could return a different type depending on the time granularity argument.

I think this idea is worth exploring some more