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@iki.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)