| 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 12:35:50 |
| Message-ID: | tencent_3E6DECB4FE1370DF6396DFD4168D48F08E08@qq.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
>Hi,
>
>On Sun, Jun 21, 2026 at 3:08?PM ZizhuanLiu X-MAN <44973863(at)qq(dot)com> wrote:
>>
>> 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`.
>
>OK, now I see your point, thank you.
>
>I agree that localbuf.c is a "callee layer focused solely on implementing core
>logic". Personally, I don't think that this fact somehow prevents us from
>performing some checks related to the temporary table inside localbuf.c, even
>if they look like some kind of "top-level" logic.
>
>But we already have an example where RELATION_IS_OTHER_TEMP is processed inside
>a function common to local and regular tables - PrefetchBuffer. I'll move the
>RELATION_IS_OTHER_TEMP into the bufmgr.c in order to be consistent with an
>existing pattern.
>
>Thank you for pointing it out! Please, see proposed change in the v3 patch.
>
>--
>Best regards,
>Daniil Davydov
Hi,Daniil
Indeed, compared with migrating all the checks into `localbuf.c`, this adjustment involves far fewer code changes.
Thanks for updating the patch to v3. I've reviewed the changes, and everything looks correct
and consistent with the existing buffer manager architecture.
This placement of the RELATION_IS_OTHER_TEMP check in bufmgr.c perfectly follows
the existing code pattern, so I have no further comments.
regards,
--
ZizhuanLiu (X-MAN)
44973863(at)qq(dot)com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | vignesh C | 2026-06-21 12:55:28 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Álvaro Herrera | 2026-06-21 12:31:55 | Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks |