>>Hi, >> >>On Sat, Jun 20, 2026 at 10:48?PM ZizhuanLiu X-MAN <[email protected]> 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) [email protected]
