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:25:46
Message-ID: tencent_2F367DDA8F09D16F25CB62EA91C66FF8FE07@qq.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>Hi,
>>
>>On Sat, Jun 20, 2026 at 10:48?PM ZizhuanLiu X-MAN <44973863(at)qq(dot)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.

Alternatively, there is another approach:
migrate all existing `RELATION_IS_OTHER_TEMP` validation logic from `bufmgr.c` into `localbuf.c`,
so that `localbuf.c` takes responsibility for performing these legitimacy checks.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Mats Kindahl 2026-06-21 09:09:35 Re: pg_rewind does not rewind diverging timelines
Previous Message ZizhuanLiu X-MAN 2026-06-21 08:08:55 Re: BUG with accessing to temporary tables of other sessions still exists