Re: BitmapHeapScan streaming read user and prelim refactoring

On 14/03/2024 12:55, Dilip Kumar wrote:
> On Thu, Mar 14, 2024 at 4:07 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> _SPI_execute_plan() has code to deal with the possibility that the
>> active snapshot is not set. That seems fishy; do we really support SPI
>> without any snapshot? I'm inclined to turn that into an error. I ran the
>> regression tests with an "Assert(ActiveSnapshotSet())" there, and
>> everything worked.
>
> IMHO, we can call SPI_Connect() and SPI_Execute() from any C
> extension, so I don't think there we can guarantee that the snapshot
> must be set, do we?

I suppose, although the things you could do without a snapshot would be
pretty limited. The query couldn't access any tables. Could it even look
up functions in the parser? Not sure.

> Maybe for now we can just handle this specific case to remove the
> snapshot serializing for the BitmapHeapScan as you are doing in the
> patch. After looking into the code your theory seems correct that we
> are just copying the ActiveSnapshot while building the query
> descriptor and from there we are copying into the Estate so logically
> there should not be any reason for these two to be different.

Ok, committed that for now. Thanks for looking!

--
Heikki Linnakangas
Neon (https://neon.tech)