On Wed, Feb 26, 2020 at 3:51 PM David Fetter <da...@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
v3-datetrunc_interval.patch
Description: Binary data