On Wed, Mar 13, 2024 at 11:39 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > Andres already commented on the snapshot stuff on an earlier patch > > version, and that's much nicer with this version. However, I don't > > understand why a parallel bitmap heap scan needs to do anything at all > > with the snapshot, even before these patches. The parallel worker > > infrastructure already passes the active snapshot from the leader to the > > parallel worker. Why does bitmap heap scan code need to do that too? > > Yeah thinking on this now it seems you are right that the parallel > infrastructure is already passing the active snapshot so why do we > need it again. Then I checked other low scan nodes like indexscan and > seqscan and it seems we are doing the same things there as well. > Check for SerializeSnapshot() in table_parallelscan_initialize() and > index_parallelscan_initialize() which are being called from > ExecSeqScanInitializeDSM() and ExecIndexScanInitializeDSM() > respectively.
I remember thinking about this when I was writing very early parallel query code. It seemed to me that there must be some reason why the EState has a snapshot, as opposed to just using the active snapshot, and so I took care to propagate that snapshot, which is used for the leader's scans, to the worker scans also. Now, if the EState doesn't need to contain a snapshot, then all of that mechanism is unnecessary, but I don't see how it can be right for the leader to do table_beginscan() using estate->es_snapshot and the worker to use the active snapshot. -- Robert Haas EDB: http://www.enterprisedb.com
