Re: checking snapshot argument for index_beginscan

On Fri, 23 Dec 2022 at 00:36, Ted Yu <yuzhihong(at)gmail(dot)com> wrote:
>>
>> Hi,
>> I was looking at commit 941aa6a6268a6a66f6895401aad6b5329111d412 .
>>
>> I think it would be better to move the assertion into `index_beginscan_internal` because it is called by index_beginscan and index_beginscan_bitmap
>>
>> In the future, when a new variant of `index_beginscan_XX` is added, the assertion would be effective for the new variant.
>>
>> Please let me know what you think.

Hi, Ted!

The two asserts out of four could be combined as you proposed in the patch.
Assert added by 941aa6a6268a6a66f6 to index_parallelscan_initialize
should remain anyway as otherwise, we can have Segfault in
SerializeSnapshot called from this function.
Assert in index_parallelscan_estimate can be omitted as there is the
same assert inside EstimateSnapshotSpace called from this function.
I've included it into version 2 of a patch.

Not sure it's worth the effort but IMO the patch is right and can be committed.

Kind regards,
Pavel Borisov,
Supabase.