Re: Batch TIDs lookup in ambulkdelete

On Mon, Jun 9, 2025 at 4:03 AM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Sat, Jun 7, 2025 at 4:34 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > BTW I found that the constant 'maxblkno' in test_tidstore.sql actually
> > equals to InvalidBlockNumber, but not MaxBlockNumber. I think it
> > doesn't make sense that TidStore uses InvalidBlockNumber as the key.
> > The attached 0001 patch fixes it. I think we can fix it separately on
> > HEAD as well as back branches.
>
> I don't see a bug here, so I don't see the need for a backpatch -- the
> block numbers in the tests are just numbers, they don't refer to
> actual relations. I understand the desire to make it closer to
> reality, but it seems cosmetic.

Okay, the patch fixes the test which is not a bug. Related to this, I
realized that TidStoreSetBlockOffsets() checks if the given offset
number is a valid OffsetNumber but doesn't do that for the given block
number, which is not a bug neither, but I think we can add a check
along with the test change.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com