BUG #14479: Casting non-constant values to INTERVAL DAY TO MINUTE doesn't truncate the seconds field

The following bug has been logged on the website:

Bug reference: 14479
Logged by: Piotr Stefaniak
Email address: email(at)piotr-stefaniak(dot)me
PostgreSQL version: 9.4.10
Operating system: FreeBSD, probably irrelevant
Description:

# SELECT x.y::INTERVAL DAY TO MINUTE, (interval '02:47:15.375721')::INTERVAL
DAY TO MINUTE FROM (VALUES (interval '02:47:15.375721')) x (y);
y | interval
-----------------+----------
02:47:15.375721 | 02:47:00

I chose 9.4.10 as the version in the web form because that's what I have,
but Andrew Gierth has said that this is the behavior of versions: 9.2, 9.3,
and 9.4 but not versions 9.1, 9.5, and HEAD.

email(at)piotr-stefaniak(dot)me writes:
> # SELECT x.y::INTERVAL DAY TO MINUTE, (interval '02:47:15.375721')::INTERVAL
> DAY TO MINUTE FROM (VALUES (interval '02:47:15.375721')) x (y);
> y | interval
> -----------------+----------
> 02:47:15.375721 | 02:47:00

> I chose 9.4.10 as the version in the web form because that's what I have,
> but Andrew Gierth has said that this is the behavior of versions: 9.2, 9.3,
> and 9.4 but not versions 9.1, 9.5, and HEAD.

Thanks for the report. It appears that interval_transform is broken:
it's deciding that the cast to INTERVAL DAY TO MINUTE is a no-op and
can be replaced by a RelabelType.

This has *not* been fixed in HEAD, it's just been masked in this
particular example by other changes. You can show it's still broken with

regression=# create table tt (f1 interval);
CREATE TABLE
regression=# insert into tt values('02:47:15.375721');
INSERT 0 1
regression=# select f1 from tt;
f1
-----------------
02:47:15.375721
(1 row)

regression=# select f1::INTERVAL DAY TO MINUTE from tt;
f1
-----------------
02:47:15.375721
(1 row)

This case fails all the way back to 9.2 where transforms were introduced.
The reason your example happens not to fail in 9.5 and up is that those
versions know how to flatten a single-row VALUES construct, so that the
cast gets simplified to a constant (correctly) and interval_transform
is never applied.

I've not figured out exactly what's broken about interval_transform;
the bit-twiddling looks a bit fishy, but it may be that that's fine
and there's simply a thinko in the logic about which conversions
are no-ops.

regards, tom lane

I wrote:
> I've not figured out exactly what's broken about interval_transform;
> the bit-twiddling looks a bit fishy, but it may be that that's fine
> and there's simply a thinko in the logic about which conversions
> are no-ops.

There seem to be multiple problems here.

One is that this innocent-looking assumption:

* Temporally-smaller fields occupy higher positions in the range
* bitmap.

is bunkum, as a quick look into dt.h confirms:

#define MONTH 1
#define YEAR 2
#define DAY 3
#define HOUR 10
#define MINUTE 11
#define SECOND 12

Don't ask me why MONTH was listed before YEAR, but it is, and we don't
really have the freedom to change these codes (especially not in the
back branches). This function will just have to cope. However, that
doesn't impact your specific bug; it means that casting INTERVAL MONTH
to INTERVAL YEAR is mistakenly treated as a no-op.

The other problem is that it's comparing the result of fls() to the
constant SECOND, but that's not right because fls(1 << k) produces k+1
not k. So it's mistakenly reading the cast to INTERVAL DAY TO MINUTE
as a cast to INTERVAL DAY TO SECOND, which indeed would be a no-op.

regards, tom lane