On Thu, Jan 26, 2023 at 01:54:08PM -0500, Tom Lane wrote: > After looking closer, I see that TimestampDifferenceMilliseconds > already explicitly states that its output is intended for WaitLatch > and friends, which makes it perfectly sane for it to clamp the result > to [0, INT_MAX] rather than depending on the caller to not pass > out-of-range values.
+1 > * This is typically used to calculate a wait timeout for WaitLatch() > * or a related function. The choice of "long" as the result type > - * is to harmonize with that. It is caller's responsibility that the > - * input timestamps not be so far apart as to risk overflow of "long" > - * (which'd happen at about 25 days on machines with 32-bit "long"). > + * is to harmonize with that; furthermore, we clamp the result to at most > + * INT_MAX milliseconds, because that's all that WaitLatch() allows. > * > - * Both inputs must be ordinary finite timestamps (in current usage, > - * they'll be results from GetCurrentTimestamp()). > + * At least one input must be an ordinary finite timestamp, else the "diff" > + * calculation might overflow. We do support stop_time == > TIMESTAMP_INFINITY, > + * which will result in INT_MAX wait time. I wonder if we should explicitly reject negative timestamps to eliminate any chance of int64 overflow, too. Alternatively, we could detect that the operation will overflow and return either 0 or INT_MAX, but I assume there's minimal use of this function with negative timestamps. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
