I wrote:
> Hmm, yeah. The "stride_usecs > 1" test seems like it's a partial
> attempt to account for this that is probably redundant given the
> additional condition. Also, can we avoid computing tm_diff %
> stride_usecs twice? Maybe the compiler is smart enough to remove the
> common subexpression, but it's a mighty expensive computation if not.
I think we could do it like this:
tm_diff = timestamp - origin;
tm_modulo = tm_diff % stride_usecs;
tm_delta = tm_diff - tm_modulo;
/* We want to round towards -infinity when tm_diff is negative */
if (tm_modulo < 0)
tm_delta -= stride_usecs;
Excluding tm_modulo == 0 from the correction is what fixes the
problem.
> I'm also itching a bit over whether there are integer-overflow
> hazards here. Maybe the range of timestamp is constrained enough
> that there aren't, but I didn't look hard.
Hmm, it's not. For instance this triggers the overflow check in
timestamp_mi:
regression=# select '294000-01-01'::timestamp - '4714-11-24 00:00:00 BC';
ERROR: interval out of range
regression=# \errverbose
ERROR: 22008: interval out of range
LOCATION: timestamp_mi, timestamp.c:2832
So we ought to guard the subtraction that produces tm_diff similarly.
I suspect it's also possible to overflow int64 while computing
stride_usecs.
regards, tom lane
