Hi Daniil
On 10/04/2026 09:10, Daniil Davydov wrote:
> Having both checks might look a bit redundant since the read stream will
> eventually call the StartReadBuffersImpl function. On the other hand, there are
> many places which are checking this restriction even if subsequent functions
> (from bufmgr) also have this check.
>
> So, I'll keep both checks and a bit reduce the comments in the bufmgr.c .
Putting this check a level deeper sounds good to me.
> BTW, what do you think about making this comment less "concrete"? :
> # SELECT via index scan from other session.
> # Sequential scans are blocked at read_stream_begin_relation(); index scans
> # bypass that path entirely and reach ReadBufferExtended() in bufmgr.c
> # (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches).
> # enable_seqscan=off forces the planner to use the index.
>
> I mean that if the described logic changes, this comment will become confusing.
> We can describe the test in general words. For example :
> # Index scans can use a different code path from the one sequential scans are
> # following. Make sure that we cannot access other sessions' temp tables during
> # index scan either.
+1
Yeah, it's indeed too verbose. I guess these comments were originally
just for me so I wouldn't get too confused along the way :)
I don't have anything else to add at this point. Unless there are any
objections, I'll mark the CF entry as 'Ready for Committer.'
Thanks!
Best, Jim
