On Wed, Feb 26, 2020 at 3:51 PM David Fetter <david@fetter.org> wrote:
>
> I believe the following should error out, but doesn't.
>
> # SELECT date_trunc_interval('1 year 1 ms', TIMESTAMP '2001-02-16 20:38:40');
> date_trunc_interval
> ═════════════════════
> 2001-01-01 00:00:00
> (1 row)
You're quite right. I forgot to add error checking for
second-and-below units. I've added your example to the tests. (I
neglected to mention in my first email that because I chose to convert
the interval to the pg_tm struct (seemed easiest), it's not
straightforward how to allow multiple unit types, and I imagine the
use case is small, so I had it throw an error.)
> Please find attached an update that I believe fixes the bug I found in
> a principled way.
Thanks for that! I made a couple adjustments and incorporated your fix
into v3: While working on v1, I noticed the DTK_FOO macros already had
an idiom for bitmasking (see utils/datetime.h), so I used that instead
of a bespoke enum. Also, since the bitmask is checked once, I removed
the individual member checks, allowing me to remove all the gotos.
There's another small wrinkle: Since we store microseconds internally,
it's neither convenient nor useful to try to error out for things like
'2 ms 500 us', since that is just as well written as '2500 us', and
stored exactly the same. I'm inclined to just skip the millisecond
check and just use microseconds, but I haven't done that yet.
Also, I noticed this bug in v1:
SELECT date_trunc_interval('17 days', TIMESTAMP '2001-02-16 20:38:40.123456');
date_trunc_interval
---------------------
2001-01-31 00:00:00
(1 row)
This is another consequence of month and day being 1-based. Fixed,
with new tests.
--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services