Fix discrepancy in Float64 to timestamp(9) casts for constants

[findepi]

Before the change, when casting Float64 value to Timestamp(Nanosecond, None), the result would depend on whether the source value is constant-foldable scalar. This is because ScalarValue.cast_to had a special treatment for that source & destination type pair, producing a different result from the canonical one.

So, this is not really changing cast(a_double as timestamp) cast. It is only changing a specialized and incorrect code path that's taken by that cast under very specific and narrow circumstances (constant folding and only for nano precision). This should be a non-controversial change.

• fixes Different result of double to timestamp(9) cast when source value is constant #16636
• to_timestamp(double) gives different results depending on scalar/vectorized call context #16678
• rels fix: The inconsistency between scalar and array on the cast decimal to timestamp #16539

[findepi]

cc @chenkovsky @alamb @jatin510

[findepi]

cc @Omega359 @spaydar per #16636 (comment)

[jatin510]

@findepi
How duckdb works in case of decimal/float input:

D  SELECT to_timestamp(1.1) as c1;
┌─────────────────────────────┐
│             c1              │
│  timestamp with time zone   │
├─────────────────────────────┤
│ 1970-01-01 05:30:01.1+05:30 │
└─────────────────────────────┘

what is the expected output ?

[findepi]

@jatin510 thanks for feedback.
to_timestamp(a_double), to_timestamp_seconds(a_double) and cast(a_double as timestamp) are 3 different things that do not have to behave the same. it's nice if they do

however,
cast(a_double as timestamp) must behave the same as cast(a_double as timestamp)... which clearly is a problem now: #16636
moreover, cast(a_double as timestamp(p)) must behave reasonably consistently for various p values (another clear problem today)
also cast from a double (Float64) must behave consistently with a cast from a float (Float32) (will add added test coverage for that)

i am not really changing cast(a_double as timestamp) cast. I am only changing a specialized incorrect code path that's taken by that cast under very specific and narrow cirumstances. this should be a non-controversial

[Omega359]

I just did a comparison of to_timestamp calls that were in the slt file that were changed to what postgresql generates and they are different:

SELECT to_timestamp(1.1) as a, to_timestamp(-1.1) as b, to_timestamp(0.0) as c, to_timestamp(1.23456789) as d, to_timestamp(123456789.123456789) as e;

a	b	c	d	e
1970-01-01 00:00:01.100000 +00:00	1969-12-31 23:59:58.900000 +00:00	1970-01-01 00:00:00.000000 +00:00	1970-01-01 00:00:01.234568 +00:00	1973-11-29 21:33:09.123457 +00:00

[findepi]

@Omega359 thanks. This totally escaped my notice that to_timestamp also changed, not only the casts (that are part of the same test).

[findepi]

I now realized that's exactly what @jatin510 pointed our earlier, just i didn't understand.
I pushed a commit restoring the to_timestamp(double) behavior to whatever it was before.

@Omega359 @jatin510 thanks again

[Omega359]

cast(123456789.123456789 as timestamp) => 1970-01-01T00:00:00.123456789

That strikes me as wrong.

[findepi]

@Omega359 i am not exactly fan of the cast semantics. If i were to choose, i would maybe choose it to be different.
Note that it's pre-existing though:

Consider

• cast source type float 32, float 64
• cast target type timestamp seconds / millis / micros / nanos
• in scalar (constant folding) or query context

from these 16 combinations, this PR changes only one, to make it consistent with other 15

[Omega359]

Ok. Well considering I couldn't even figure out how to cast a float to a timestamp in postgres without using to_timestamp and duckdb outright says it's not implemented (see below) I guess it's consistent (must be nanos?) - it's just going to surprise pretty much everyone.

Use ".open FILENAME" to reopen on a persistent database.
D select 1.1::timestamp;
Conversion Error:
Unimplemented type for cast (DECIMAL(2,1) -> TIMESTAMP)

[alamb]

Thank you for working on this @findepi -- I think we are close

[alamb]

Do I read this difference right as now cast(float_col AS timestamp) is treated as though it is 1.1 ns where as before it was treated as 1.1 sec?

What do we think about simply not supporting explicit conversion from float --> timestamp to follow the duckdb/postgres model. That feels far more defensible to me than this behavior which is both different than it was previously AND not consistent with other engines

[findepi]

both different than it was previously

it's NOT!

it's different ONLY in constant folding context.

BTW to_timestamp(1.1) works the way it seems to work on constant folding context only too, i filed #16678 for this, but this PR fixes this problem as well.

What do we think about simply not supporting explicit conversion from float --> timestamp

I am supportive of that. Same for decimals and perhaps ints.

But it's definitely more work and more controversial change.
So we should first fix #16636, #16531 and #16678 which are code bugs bringing embarrassment to the project and data corruption to the users.

[alamb]

Yes, thank you, I double checked and reminded myself of what was going on:

DataFusion CLI v48.0.0
> select cast(1.1 as timestamp);
+-------------------------+
| Float64(1.1)            |
+-------------------------+
| 1970-01-01T00:00:01.100 |
+-------------------------+
1 row(s) fetched.
Elapsed 0.005 seconds.

> select cast(column1 as timestamp) from values (1.1);
+-------------------------------+
| column1                       |
+-------------------------------+
| 1970-01-01T00:00:00.000000001 |
+-------------------------------+
1 row(s) fetched.
Elapsed 0.001 seconds.

[findepi]

Indeed. I tried to capture this in the linked issue #16636
Thanks for bring this example here.

[alamb]

This PR looks good to me -- thank you @findepi

[alamb]

Yes, thank you, I double checked and reminded myself of what was going on:

DataFusion CLI v48.0.0
> select cast(1.1 as timestamp);
+-------------------------+
| Float64(1.1)            |
+-------------------------+
| 1970-01-01T00:00:01.100 |
+-------------------------+
1 row(s) fetched.
Elapsed 0.005 seconds.

> select cast(column1 as timestamp) from values (1.1);
+-------------------------------+
| column1                       |
+-------------------------------+
| 1970-01-01T00:00:00.000000001 |
+-------------------------------+
1 row(s) fetched.
Elapsed 0.001 seconds.

[alamb]

Thanks again @findepi