Re: [PG19] btree_gin misses rows for a timestamptz bound in the repeated DST hour

shihao zhong <zhong950419(at)gmail(dot)com> writes:
> Done in v2. cvt_timestamptz_timestamp now moves the key back an hour
> while timestamp_cmp_timestamptz_internal() says it is later than the
> bound.

OK. It occurred to me that another source of a converted timestamp
appearing larger than the given value is if we produce timestamp
infinity from a value close to the maximum finite timestamptz.
I'm not quite convinced whether that can lead to problems, but
at the least it complicates reasoning about the behavior because
we again lose the property that result compares <= val. So just
ignoring the case as your patch does seems risky. I made it
look like this instead:

if (TIMESTAMP_IS_NOEND(result))
result = END_TIMESTAMP;
while (timestamp_cmp_timestamptz_internal(result, val) > 0)
result -= USECS_PER_HOUR;

>> Also, doesn't cvt_timestamptz_date have the same issue?

> I could not make it fail in a test over every backward clock change
> in tzdata, and the code agrees. The key is the local date of the
> bound. An earlier date can only compare as later than the bound if
> the clock goes back by more than a whole day. The biggest step back
> in tzdata is exactly 24 hours, Alaska in 1867. If you'd rather not
> rely on that, the same loop with result-- works there and I can add
> it.

Hm. I feel a little itchy about that, but as long as we can't point
to a failure case I'm okay with leaving cvt_timestamptz_date alone.

Pushed with that correction and some cosmetic foolery.

regards, tom lane