Re: BUG with accessing to temporary tables of other sessions still exists - Mailing list pgsql-hackers

>Hi,

>

>On Sat, Jun 20, 2026 at 10:48?PM ZizhuanLiu X-MAN <44973863@qq.com> wrote:

>>

>> Just as described in the comment of RELATION_IS_OTHER_TEMP macro in rel.h,

>> existing buffer manager routines including ReadBuffer_common(), StartReadBuffersImpl()

>> and PrefetchBuffer() have already invoked this macro to check cross-session temporary

>> table access. All these functions are located in bufmgr.c.

>>

>> For code consistency, I suggest adding the same RELATION_IS_OTHER_TEMP check in

>> ExtendBufferedRelCommon() (also in bufmgr.c), right before calling ExtendBufferedRelLocal().

>

>Thank you for your feedback!

>

>Do you mean that this check should be moved to "ExtendBufferedRelCommon"

>because this function is in the bufmgr.c file? If so, I don't see the point in

>that. Buffer manager is not only the bufmgr.c file, but also all the files in

>the "storage/buffer" folder. So we can say that localbuf.c also contains logic

>related to the buffer manager.

>

>Moreover, ExtendBufferedRelCommon should contain only the logic that is really

>common for both ordinary and temp tables. All other specific code is

>encapsulated inside the ExtendBufferedRelLocal and ExtendBufferedRelShared. It

>allows us to keep the ExtendBufferedRelCommon function pretty short.

>

>So, I suggest leaving RELATION_IS_OTHER_TEMP as it is.

>

>> Meanwhile, we should also update the comment of RELATION_IS_OTHER_TEMP

>> accordingly to keep the documentation synchronized with code changes.

>

>This comment is already updated - it now mentions the ExtendBufferedRelLocal

>function.

>

>--

>Best regards,

>Daniil Davydov

Hi,Danii

Thank you very much for your feedback, as well as the original patch and test cases. 

I have learned a lot about the internals of temporary tables from this discussion.

First, I would like to clarify that the buffer manager is not limited to `bufmgr.c` alone, 

but covers all source files under the `storage/buffer` directory. 

I’ve analyzed the existing architecture and the relationship between `bufmgr.c`, 

the `RELATION_IS_OTHER_TEMP` macro and `localbuf.c`, and my observations 

are summarised below:

1. Within the `storage/buffer` module, `RELATION_IS_OTHER_TEMP` is currently referenced 

in three places, all of which reside in `bufmgr.c`; there are no usages in `localbuf.c` at present.

2. Looking at the function call graph between `bufmgr.c` and `localbuf.c`, most routines in 

`localbuf.c` serve as helper functions invoked from `bufmgr.c`. (A tiny number of calls exist 

within `src/test/modules/test_io.c`, which I believe can be disregarded for this discussion.)

3. Under the current design, `localbuf.c` acts as a callee layer focused solely on implementing 

core logic without introducing `RELATION_IS_OTHER_TEMP` checks, keeping this module clean. 

All access validation decisions using `RELATION_IS_OTHER_TEMP` are handled upstream in 

`bufmgr.c` as the main caller.

4. Placing the `RELATION_IS_OTHER_TEMP` check inside `ExtendBufferedRelCommon()` in `bufmgr.c` 

(rather than `ExtendBufferedRelLocal()` in `localbuf.c`) aligns with this existing architectural principle: 

validation rules are enforced by the caller layer. Admittedly, this approach has a downside — if new 

caller entrypoints are added in the future, each would need to remember to perform this security check. 

Fortunately, all relevant call paths currently originate from `bufmgr.c`.

Please feel free to point out any flaws or oversights in my reasoning.

Below is a partial summary of the call relationships among `bufmgr.c`, `RELATION_IS_OTHER_TEMP` 

and `localbuf.c` (this is not an exhaustive list):

PrefetchBuffer()  

   --RELATION_IS_OTHER_TEMP + PrefetchLocalBuffer()  /* pass it off to localbuf.c */

   --PrefetchSharedBuffer()

ReadBuffer_common()

   RELATION_IS_OTHER_TEMP               ExtendBufferedRel()==>ExtendBufferedRel()===>ExtendBufferedRelBy()==>ExtendBufferedRelCommon()==>ExtendBufferedRelCommon()

   PinBufferForBlock()

   StartReadBuffer()==>StartReadBuffersImpl()

   WaitReadBuffers()

static ExtendBufferedRelCommon()

   --if bmr.relpersistence == RELPERSISTENCE_TEMP call ExtendBufferedRelLocal()  /* @localbuf.c */

   -- else ExtendBufferedRelShared()

static StartReadBuffersImpl()

   RELATION_IS_OTHER_TEMP

static PinBufferForBlock()

  if (persistence == RELPERSISTENCE_TEMP)  LocalBufferAlloc()   /* @localbuf.c */

  else BufferAlloc()

FlushRelationBuffers()

      PinLocalBuffer()       /* @localbuf.c */

      FlushLocalBuffer()    /* @localbuf.c */

      UnpinLocalBuffer     /* @localbuf.c */

MarkBufferDirty()

    MarkLocalBufferDirty()     /* @localbuf.c */

MarkBufferDirtyHint()

    MarkLocalBufferDirty()     /* @localbuf.c */

BufferSetHintBits16()

    MarkLocalBufferDirty()     /* @localbuf.c */

ZeroAndLockBuffer()

   StartLocalBufferIO()           /* @localbuf.c */

   TerminateLocalBufferIO()  /* @localbuf.c */

StartBufferIO()

     StartLocalBufferIO()       /* @localbuf.c */

     

buffer_readv_complete_one()

    TerminateLocalBufferIO()       /* @localbuf.c */