BUG with accessing to temporary tables of other sessions still exists

Started by Daniil Davydov21 days ago12 messageshackers
Jump to latest
#1Daniil Davydov
3danissimo@gmail.com

Hi,

Recently, we have added two commits ([1]40927d458fe1a8c96bcf418b47169f0f6eb15946, [2]ce146621f7860d2e19c509f1466feca3bf777678) that are fixing a bug with
accessing temporary tables of other sessions. I found out that this bug didn't
go away completely :

== session 1 ==

postgres=# CREATE TABLE empty_table (id INT);
CREATE TABLE

== session 2 ==

postgres=# INSERT INTO pg_temp_0.empty_table VALUES (1);
INSERT 0 1

As you can see, the INSERT command completes successfully. This happens because
empty_table has no pages, so the insert path looks like this :
heap_insert -> RelationGetBufferForTuple -> RelationAddBlocks -> ....

The RelationAddBlocks extends relations's smgr and allocates a new temp buffer
without calling ReadBufferExtended. Thus, we are 1) bypassing the
"RELATION_IS_OTHER_TEMP" check and 2) creating a buffer in our own temp buffers
pool. The (2) will lead to an error [3]ERROR: could not open file "base/5/t3_16386": No such file or directory if we attempt to flush such a buffer.

I suggest fixing it by checking whether the relation is other-temp-rel inside
the ExtendBufferedRelLocal function. As far as I can see, all
temp-relation-extend paths include this function.

Please, see the attached patch that fixes a problem and adds a new test.

[1]: 40927d458fe1a8c96bcf418b47169f0f6eb15946
[2]: ce146621f7860d2e19c509f1466feca3bf777678
[3]: ERROR: could not open file "base/5/t3_16386": No such file or directory

P.S.
Jim, I attach you in CC of this thread since you are the co-author of the
original fix. I hope you don't mind :)

--
Best regards,
Daniil Davydov

Attachments:

0001-Prevent-access-to-other-sessions-empty-temp-tables.patchtext/x-patch; charset=US-ASCII; name=0001-Prevent-access-to-other-sessions-empty-temp-tables.patchDownload+30-4
#2Jim Jones
jim.jones@uni-muenster.de
In reply to: Daniil Davydov (#1)
Re: BUG with accessing to temporary tables of other sessions still exists

Hi Daniil,
Good catch!

On 03.06.26 15:23, Daniil Davydov wrote:

Recently, we have added two commits ([1], [2]) that are fixing a bug with
accessing temporary tables of other sessions. I found out that this bug didn't
go away completely :

== session 1 ==

postgres=# CREATE TABLE empty_table (id INT);
CREATE TABLE

== session 2 ==

postgres=# INSERT INTO pg_temp_0.empty_table VALUES (1);
INSERT 0 1

Session 1 here does not create a temporary table (most likely a copy &
paste error), but I could reproduce this error as you suggested:

== session 1 ==

postgres=# CREATE TEMPORARY TABLE t (id int);

== session 2 ==

postgres=# \d pg_temp*.*
Table "pg_temp_2.t"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
id | integer | | |

postgres=# INSERT INTO pg_temp_2.t VALUES (42);
INSERT 0 1
postgres=# INSERT INTO pg_temp_2.t VALUES (42);
ERROR: cannot access temporary tables of other sessions

It only fails after the second INSERT attempt.

With your patch applied it returns an error also in the first query:

psql (19devel)
Type "help" for help.

postgres=# INSERT INTO pg_temp_0.t VALUES (42);
ERROR: cannot access temporary tables of other sessions

As you can see, the INSERT command completes successfully. This happens because
empty_table has no pages, so the insert path looks like this :
heap_insert -> RelationGetBufferForTuple -> RelationAddBlocks -> ....

The RelationAddBlocks extends relations's smgr and allocates a new temp buffer
without calling ReadBufferExtended. Thus, we are 1) bypassing the
"RELATION_IS_OTHER_TEMP" check and 2) creating a buffer in our own temp buffers
pool. The (2) will lead to an error [3] if we attempt to flush such a buffer.

I suggest fixing it by checking whether the relation is other-temp-rel inside
the ExtendBufferedRelLocal function. As far as I can see, all
temp-relation-extend paths include this function.

Please, see the attached patch that fixes a problem and adds a new test.

At a first glance the check seems reasonable. One tiny wording nit: the
comment in ExtendBufferedRelLocal says "... covering any attempt to
extend local relation.", but to avoid any confusing with the meaning of
RELATION_IS_LOCAL I'd argue that "covering any attempt to extend a
temporary relation" would be slightly clearer.

Thanks for the fix!

Best, Jim

#3Daniil Davydov
3danissimo@gmail.com
In reply to: Jim Jones (#2)
Re: BUG with accessing to temporary tables of other sessions still exists

Hi,

On Wed, Jun 3, 2026 at 10:20 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

Session 1 here does not create a temporary table (most likely a copy &
paste error), but I could reproduce this error as you suggested:

Sorry, I wrote it manually and forgot to specify "TEMP". It is implied here,
of course.

At a first glance the check seems reasonable. One tiny wording nit: the
comment in ExtendBufferedRelLocal says "... covering any attempt to
extend local relation.", but to avoid any confusing with the meaning of
RELATION_IS_LOCAL I'd argue that "covering any attempt to extend a
temporary relation" would be slightly clearer.

Thanks for looking into this! I agree with your comment.

Please, see v2 patch with fixed comment.

--
Best regards,
Daniil Davydov

Attachments:

v2-0001-Prevent-access-to-other-sessions-empty-temp-table.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Prevent-access-to-other-sessions-empty-temp-table.patchDownload+30-4
#4Imran Zaheer
imran.zhir@gmail.com
In reply to: Daniil Davydov (#3)
Re: BUG with accessing to temporary tables of other sessions still exists

Hi

During testing this patch I also noticed that other sessions are also
able to drop the temporary table.

postgres=# drop table pg_temp_0.empty_table;
DROP TABLE

Above command works just fine from the other session.

I was able to fix that by adding the same check in
heap_drop_with_catalog in heap.c, but I'm not sure whether it's the
right place to add this check.

@@ -1847,6 +1847,11 @@ heap_drop_with_catalog(Oid relid)
*/
rel = relation_open(relid, AccessExclusiveLock);

+       if (RELATION_IS_OTHER_TEMP(rel))
+               ereport(ERROR,
+                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                               errmsg("cannot access temporary tables
of other sessions")));
+
        /*
         * There can no longer be anyone *else* touching the relation, but we
         * might still have open queries or cursors, or pending
trigger events, in

Let me know if I am missing something.

Thanks
Imran Zaheer

Show quoted text

On Wed, Jun 3, 2026 at 8:51 PM Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Wed, Jun 3, 2026 at 10:20 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

Session 1 here does not create a temporary table (most likely a copy &
paste error), but I could reproduce this error as you suggested:

Sorry, I wrote it manually and forgot to specify "TEMP". It is implied here,
of course.

At a first glance the check seems reasonable. One tiny wording nit: the
comment in ExtendBufferedRelLocal says "... covering any attempt to
extend local relation.", but to avoid any confusing with the meaning of
RELATION_IS_LOCAL I'd argue that "covering any attempt to extend a
temporary relation" would be slightly clearer.

Thanks for looking into this! I agree with your comment.

Please, see v2 patch with fixed comment.

--
Best regards,
Daniil Davydov

#5Daniil Davydov
3danissimo@gmail.com
In reply to: Imran Zaheer (#4)
Re: BUG with accessing to temporary tables of other sessions still exists

Hi,

On Thu, Jun 4, 2026 at 12:43 AM Imran Zaheer <imran.zhir@gmail.com> wrote:

During testing this patch I also noticed that other sessions are also
able to drop the temporary table.

postgres=# drop table pg_temp_0.empty_table;
DROP TABLE

Above command works just fine from the other session.

I was able to fix that by adding the same check in
heap_drop_with_catalog in heap.c, but I'm not sure whether it's the
right place to add this check.

Let me know if I am missing something.

It may be counter intuitive, but we allow dropping other session's temp tables.
You can find the rationale for this in the 013_temp_obj_multisession.pl test.
You can also read this message [1]/messages/by-id/4075754.1774378690@sss.pgh.pa.us in the previous discussion. In short, we
prohibit looking at other-temp-table's pages not because they belong to another
session, but because current temp_buffers implementation doesn't provide the
ability to do so. Moreover, the ability to DROP other temp tables can be useful
for autovacuum (see orphaned temp tables removal logic) and administrators.

[1]: /messages/by-id/4075754.1774378690@sss.pgh.pa.us

--
Best regards,
Daniil Davydov

#6Imran Zaheer
imran.zhir@gmail.com
In reply to: Daniil Davydov (#5)
Re: BUG with accessing to temporary tables of other sessions still exists

Got it, Thanks for making this clean.

Regards,
Imran Zaheer

Show quoted text

On Wed, Jun 3, 2026 at 11:33 PM Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Thu, Jun 4, 2026 at 12:43 AM Imran Zaheer <imran.zhir@gmail.com> wrote:

During testing this patch I also noticed that other sessions are also
able to drop the temporary table.

postgres=# drop table pg_temp_0.empty_table;
DROP TABLE

Above command works just fine from the other session.

I was able to fix that by adding the same check in
heap_drop_with_catalog in heap.c, but I'm not sure whether it's the
right place to add this check.

Let me know if I am missing something.

It may be counter intuitive, but we allow dropping other session's temp tables.
You can find the rationale for this in the 013_temp_obj_multisession.pl test.
You can also read this message [1] in the previous discussion. In short, we
prohibit looking at other-temp-table's pages not because they belong to another
session, but because current temp_buffers implementation doesn't provide the
ability to do so. Moreover, the ability to DROP other temp tables can be useful
for autovacuum (see orphaned temp tables removal logic) and administrators.

[1] /messages/by-id/4075754.1774378690@sss.pgh.pa.us

--
Best regards,
Daniil Davydov

#7ZizhuanLiu X-MAN
44973863@qq.com
In reply to: Daniil Davydov (#3)
Re: BUG with accessing to temporary tables of other sessions still exists

Hi,

On Wed, Jun 3, 2026 at 10:20?PM Jim Jones <jim.jones@uni-muenster.de> wrote:

Session 1 here does not create a temporary table (most likely a copy &
paste error), but I could reproduce this error as you suggested:

Sorry, I wrote it manually and forgot to specify "TEMP". It is implied here,
of course.

At a first glance the check seems reasonable. One tiny wording nit: the
comment in ExtendBufferedRelLocal says "... covering any attempt to
extend local relation.", but to avoid any confusing with the meaning of
RELATION_IS_LOCAL I'd argue that "covering any attempt to extend a
temporary relation" would be slightly clearer.

Thanks for looking into this! I agree with your comment.

Please, see v2 patch with fixed comment.

--
Best regards,
Daniil Davydov

Hi, Daniil , Jim

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().

Meanwhile, we should also update the comment of RELATION_IS_OTHER_TEMP accordingly
to keep the documentation synchronized with code changes.

regards,
--
ZizhuanLiu (X-MAN) 
44973863@qq.com

#8Daniil Davydov
3danissimo@gmail.com
In reply to: ZizhuanLiu X-MAN (#7)
Re: BUG with accessing to temporary tables of other sessions still exists

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

#9ZizhuanLiu X-MAN
44973863@qq.com
In reply to: Daniil Davydov (#8)
Re: BUG with accessing to temporary tables of other sessions still exists

&gt;Hi,
&gt;
&gt;On Sat, Jun 20, 2026 at 10:48?PM ZizhuanLiu X-MAN <44973863@qq.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@qq.com

#10ZizhuanLiu X-MAN
44973863@qq.com
In reply to: ZizhuanLiu X-MAN (#9)
Re: BUG with accessing to temporary tables of other sessions still exists

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.

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@qq.com

#11Daniil Davydov
3danissimo@gmail.com
In reply to: ZizhuanLiu X-MAN (#9)
Re: BUG with accessing to temporary tables of other sessions still exists

Hi,

On Sun, Jun 21, 2026 at 3:08 PM ZizhuanLiu X-MAN <44973863@qq.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

Attachments:

v3-0001-Prevent-access-to-other-sessions-empty-temp-table.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Prevent-access-to-other-sessions-empty-temp-table.patchDownload+34-5
#12ZizhuanLiu X-MAN
44973863@qq.com
In reply to: Daniil Davydov (#11)
Re: BUG with accessing to temporary tables of other sessions still exists

Hi,

On Sun, Jun 21, 2026 at 3:08?PM ZizhuanLiu X-MAN <44973863@qq.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@qq.com