Re: Add a bound check to TidRangeEval

Hi David,

On Sat, Jun 14, 2025 at 11:27 AM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Wed, 11 Jun 2025 at 14:26, Junwang Zhao <zhjwpku@gmail.com> wrote:
> > This is not a common case, it's just a corner case while
> > playing around the TidRangeScan.
> >
> > I'm not saying this is an optimization, it just makes me a little
> > confused when I see the lowerBound > upperBound and
> > it still returns true.
> >
> > The comments say "Returns true if it's possible for the
> > range to contain tuples." This seems not fully accurate?
>
> The word "possible" there is meant to convey that false negatives are
> *not* ok, but false positives are fine. I'm certainly open to making
> the comment better if that's not clear.
>
> How about adding "We don't bother validating that trss_mintid is less
> than or equal to trss_maxtid, as the scan_set_tidrange() table AM
> function will detect that."

Sounds good to me. This will naturally direct readers to the
comments in scan_set_tidrange.

>
> The scan_set_tidrange function does document that it's the
> implementation's responsibility to check this, as the comment in the
> declaration of struct TableAmRoutine reads:
>
> * Implementations of scan_set_tidrange must themselves handle
> * ItemPointers of any value. i.e, they must handle each of the following:
> *
> * 1) mintid or maxtid is beyond the end of the table; and
> * 2) mintid is above maxtid; and
> * 3) item offset for mintid or maxtid is beyond the maximum offset
> * allowed by the AM.
>
> So I'm not keen to add code to TidRangeEval to check for this as it
> would result in additional overhead for valid usages of TID Range
> scans.

Agree, thanks for the explanation.

>
> David



--
Regards
Junwang Zhao