Re: BUG with accessing to temporary tables of other sessions still exists

From: ZizhuanLiu X-MAN <44973863(at)qq(dot)com>
To: Daniil Davydov <3danissimo(at)gmail(dot)com>
Cc: Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: BUG with accessing to temporary tables of other sessions still exists
Date: 2026-06-21 08:08:55
Message-ID: tencent_9CF0C26DD2B44713FCDF6C9634D4158D9908@qq.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

&gt;Hi,
&gt;
&gt;On Sat, Jun 20, 2026 at 10:48?PM ZizhuanLiu X-MAN <44973863(at)qq(dot)com&gt; wrote:
&gt;&gt;
&gt;&gt; Just as described in the comment of RELATION_IS_OTHER_TEMP macro in rel.h,
&gt;&gt; existing buffer manager routines including ReadBuffer_common(), StartReadBuffersImpl()
&gt;&gt; and PrefetchBuffer() have already invoked this macro to check cross-session temporary
&gt;&gt; table access. All these functions are located in bufmgr.c.
&gt;&gt;
&gt;&gt; For code consistency, I suggest adding the same RELATION_IS_OTHER_TEMP check in
&gt;&gt; ExtendBufferedRelCommon() (also in bufmgr.c), right before calling ExtendBufferedRelLocal().
&gt;
&gt;Thank you for your feedback!
&gt;
&gt;Do you mean that this check should be moved to "ExtendBufferedRelCommon"
&gt;because this function is in the bufmgr.c file? If so, I don't see the point in
&gt;that. Buffer manager is not only the bufmgr.c file, but also all the files in
&gt;the "storage/buffer" folder. So we can say that localbuf.c also contains logic
&gt;related to the buffer manager.
&gt;
&gt;Moreover, ExtendBufferedRelCommon should contain only the logic that is really
&gt;common for both ordinary and temp tables. All other specific code is
&gt;encapsulated inside the ExtendBufferedRelLocal and ExtendBufferedRelShared. It
&gt;allows us to keep the ExtendBufferedRelCommon function pretty short.
&gt;
&gt;So, I suggest leaving RELATION_IS_OTHER_TEMP as it is.
&gt;
&gt;&gt; Meanwhile, we should also update the comment of RELATION_IS_OTHER_TEMP
&gt;&gt; accordingly to keep the documentation synchronized with code changes.
&gt;
&gt;This comment is already updated - it now mentions the ExtendBufferedRelLocal
&gt;function.
&gt;
&gt;--
&gt;Best regards,
&gt;Daniil Davydov

Hi,Danii

Thank you very much for your feedback, as well as the original patch and test cases.&nbsp;
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,&nbsp;
but covers all source files under the `storage/buffer` directory.&nbsp;

I’ve analyzed the existing architecture and the relationship between `bufmgr.c`,&nbsp;
the `RELATION_IS_OTHER_TEMP` macro and `localbuf.c`, and my observations&nbsp;
are summarised below:

1. Within the `storage/buffer` module, `RELATION_IS_OTHER_TEMP` is currently referenced&nbsp;
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&nbsp;
`localbuf.c` serve as helper functions invoked from `bufmgr.c`. (A tiny number of calls exist&nbsp;
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&nbsp;
core logic without introducing `RELATION_IS_OTHER_TEMP` checks, keeping this module clean.&nbsp;
All access validation decisions using `RELATION_IS_OTHER_TEMP` are handled upstream in&nbsp;
`bufmgr.c` as the main caller.

4. Placing the `RELATION_IS_OTHER_TEMP` check inside `ExtendBufferedRelCommon()` in `bufmgr.c`&nbsp;
(rather than `ExtendBufferedRelLocal()` in `localbuf.c`) aligns with this existing architectural principle:&nbsp;
validation rules are enforced by the caller layer. Admittedly, this approach has a downside — if new&nbsp;
caller entrypoints are added in the future, each would need to remember to perform this security check.&nbsp;
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`&nbsp;
and `localbuf.c` (this is not an exhaustive list):

PrefetchBuffer() &nbsp;
&nbsp; &nbsp;--RELATION_IS_OTHER_TEMP + PrefetchLocalBuffer() &nbsp;/* pass it off to localbuf.c */
&nbsp; &nbsp;--PrefetchSharedBuffer()

ReadBuffer_common()
&nbsp; &nbsp;RELATION_IS_OTHER_TEMP&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;ExtendBufferedRel()==&gt;ExtendBufferedRel()===&gt;ExtendBufferedRelBy()==&gt;ExtendBufferedRelCommon()==&gt;ExtendBufferedRelCommon()
&nbsp; &nbsp;PinBufferForBlock()
&nbsp; &nbsp;StartReadBuffer()==&gt;StartReadBuffersImpl()
&nbsp; &nbsp;WaitReadBuffers()

static ExtendBufferedRelCommon()
&nbsp; &nbsp;--if bmr.relpersistence == RELPERSISTENCE_TEMP call ExtendBufferedRelLocal() &nbsp;/* @localbuf.c */
&nbsp; &nbsp;-- else ExtendBufferedRelShared()

static StartReadBuffersImpl()
&nbsp; &nbsp;RELATION_IS_OTHER_TEMP

static PinBufferForBlock()
&nbsp; if (persistence == RELPERSISTENCE_TEMP) &nbsp;LocalBufferAlloc() &nbsp; /* @localbuf.c */
&nbsp; else BufferAlloc()

FlushRelationBuffers()
&nbsp; &nbsp; &nbsp; PinLocalBuffer()&nbsp; &nbsp; &nbsp; &nbsp;/* @localbuf.c */
&nbsp; &nbsp; &nbsp; FlushLocalBuffer()&nbsp; &nbsp; /* @localbuf.c */
&nbsp; &nbsp; &nbsp; UnpinLocalBuffer &nbsp; &nbsp; /* @localbuf.c */

MarkBufferDirty()
&nbsp; &nbsp; MarkLocalBufferDirty() &nbsp; &nbsp; /* @localbuf.c */

MarkBufferDirtyHint()
&nbsp; &nbsp; MarkLocalBufferDirty() &nbsp; &nbsp; /* @localbuf.c */

BufferSetHintBits16()
&nbsp; &nbsp; MarkLocalBufferDirty() &nbsp; &nbsp; /* @localbuf.c */

ZeroAndLockBuffer()
&nbsp; &nbsp;StartLocalBufferIO()&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;/* @localbuf.c */
&nbsp; &nbsp;TerminateLocalBufferIO() &nbsp;/* @localbuf.c */

StartBufferIO()
&nbsp; &nbsp; &nbsp;StartLocalBufferIO() &nbsp; &nbsp; &nbsp; /* @localbuf.c */
&nbsp; &nbsp; &nbsp;
buffer_readv_complete_one()
&nbsp; &nbsp; TerminateLocalBufferIO() &nbsp; &nbsp; &nbsp; /* @localbuf.c */

regards,
--
ZizhuanLiu&nbsp;(X-MAN)&nbsp;
44973863(at)qq(dot)com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message ZizhuanLiu X-MAN 2026-06-21 08:25:46 Re: BUG with accessing to temporary tables of other sessions still exists
Previous Message Zsolt Parragi 2026-06-21 07:57:45 Re: PG20 Minimum Dependency Thread