On Sun, Jun 09, 2024 at 09:57:54PM -0400, Tom Lane wrote: > Nathan Bossart <nathandboss...@gmail.com> writes: >> On Sun, Jun 09, 2024 at 04:59:22PM -0400, Joseph Koshakow wrote: >>> The following comment was in the code for parsing timestamps: >>> /* check for just-barely overflow (okay except time-of-day wraps) */ >>> /* caution: we want to allow 1999-12-31 24:00:00 */ >>> >>> I wasn't able to fully understand it even after staring at it for >>> a while. Is the comment suggesting that it is ok for the months field, >>> for example, to wrap around? That doesn't sound right to me I tested >>> the supplied timestamp, 1999-12-31 24:00:00, and it behaves the same >>> before and after the patch. > >> I haven't stared at this for a while like you, but I am likewise confused >> at first glance. This dates back to commit 84df54b, and it looks like this >> comment was present in the first version of the patch in the thread [0]. I >> CTRL+F'd for any discussion about this but couldn't immediately find >> anything. > > I believe this is a copy-and-paste from 841b4a2d5, which added this: > > + *result = (date * INT64CONST(86400000000)) + time; > + /* check for major overflow */ > + if ((*result - time) / INT64CONST(86400000000) != date) > + return -1; > + /* check for just-barely overflow (okay except time-of-day wraps) */ > + if ((*result < 0) ? (date >= 0) : (date < 0)) > + return -1; > > I think you could replace the whole thing by using overflow-aware > multiplication and addition primitives in the result calculation. > Lines 2-4 basically check for mult overflow and 5-7 for addition > overflow.
Ah, I see. Joe's patch does that in one place. It's probably worth doing that in the other places this "just-barefly overflow" comment appears IMHO. I was still confused by the comment about 1999, but I tracked it down to commit 542eeba [0]. IIUC it literally means that we need special handling for that date because POSTGRES_EPOCH_JDATE is 2000-01-01. [0] https://postgr.es/m/CABUevEx5zUO%3DKRUg06a9qnQ_e9EvTKscL6HxAM_L3xj71R7AQw%40mail.gmail.com -- nathan
