public inbox for [email protected]help / color / mirror / Atom feed
Re: Fix bug with accessing to temporary tables of other sessions 52+ messages / 7 participants [nested] [flat]
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-02-27 14:02 Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-02-27 14:02 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Stepan Neretin <[email protected]>; pgsql-hackers Hi, Posting rebased set of patches. -- Best regards, Daniil Davydov Attachments: [text/x-patch] v12-0001-Fix-accessing-other-sessions-temp-tables.patch (6.4K, 2-v12-0001-Fix-accessing-other-sessions-temp-tables.patch) download | inline diff: From 3dc50f99139e7d83b54c97eed6d10d0a96702c05 Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Tue, 28 Oct 2025 20:22:48 +0700 Subject: [PATCH v12 1/2] Fix accessing other sessions temp tables --- src/backend/catalog/namespace.c | 56 ++++++++++++++++++++------------ src/backend/commands/tablecmds.c | 3 +- src/backend/nodes/makefuncs.c | 6 +++- src/backend/parser/gram.y | 11 ++++++- src/include/catalog/namespace.h | 2 ++ 5 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 56b87d878e8..b672bcf5f63 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -500,28 +500,44 @@ RangeVarGetRelidExtended(const RangeVar *relation, LOCKMODE lockmode, */ if (relation->relpersistence == RELPERSISTENCE_TEMP) { - if (!OidIsValid(myTempNamespace)) - relId = InvalidOid; /* this probably can't happen? */ - else - { - if (relation->schemaname) - { - Oid namespaceId; + Oid namespaceId; - namespaceId = LookupExplicitNamespace(relation->schemaname, missing_ok); + if (relation->schemaname) + { + namespaceId = LookupExplicitNamespace(relation->schemaname, missing_ok); + /* + * If the user has specified an existing temporary schema + * owned by another user. + */ + if (OidIsValid(namespaceId) && namespaceId != myTempNamespace) + { /* - * For missing_ok, allow a non-existent schema name to - * return InvalidOid. + * We don't allow users to access temp tables of other + * sessions except for the case of dropping tables. */ - if (namespaceId != myTempNamespace) + if (!(flags & RVR_OTHER_TEMP_OK)) ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("temporary tables cannot specify a schema name"))); + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); } + } + else + { + namespaceId = myTempNamespace; - relId = get_relname_relid(relation->relname, myTempNamespace); + /* + * If this table was recognized as temporary, it means that we + * found it because backend's temporary namespace was specified + * in search_path. Thus, MyTempNamespace must contain valid oid. + */ + Assert(OidIsValid(namespaceId)); } + + if (missing_ok && !OidIsValid(namespaceId)) + relId = InvalidOid; + else + relId = get_relname_relid(relation->relname, namespaceId); } else if (relation->schemaname) { @@ -3625,21 +3641,19 @@ get_namespace_oid(const char *nspname, bool missing_ok) RangeVar * makeRangeVarFromNameList(const List *names) { - RangeVar *rel = makeRangeVar(NULL, NULL, -1); + RangeVar *rel; switch (list_length(names)) { case 1: - rel->relname = strVal(linitial(names)); + rel = makeRangeVar(NULL, strVal(linitial(names)), -1); break; case 2: - rel->schemaname = strVal(linitial(names)); - rel->relname = strVal(lsecond(names)); + rel = makeRangeVar(strVal(linitial(names)), strVal(lsecond(names)), -1); break; case 3: + rel = makeRangeVar(strVal(lsecond(names)), strVal(lthird(names)), -1); rel->catalogname = strVal(linitial(names)); - rel->schemaname = strVal(lsecond(names)); - rel->relname = strVal(lthird(names)); break; default: ereport(ERROR, @@ -3846,6 +3860,8 @@ GetTempNamespaceProcNumber(Oid namespaceId) return INVALID_PROC_NUMBER; /* no such namespace? */ if (strncmp(nspname, "pg_temp_", 8) == 0) result = atoi(nspname + 8); + else if (strcmp(nspname, "pg_temp") == 0) + result = MyProcNumber; else if (strncmp(nspname, "pg_toast_temp_", 14) == 0) result = atoi(nspname + 14); else diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b04b0dbd2a0..a1544b86221 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1639,7 +1639,8 @@ RemoveRelations(DropStmt *drop) state.heapOid = InvalidOid; state.partParentOid = InvalidOid; - relOid = RangeVarGetRelidExtended(rel, lockmode, RVR_MISSING_OK, + relOid = RangeVarGetRelidExtended(rel, lockmode, + RVR_MISSING_OK | RVR_OTHER_TEMP_OK, RangeVarCallbackForDropRelation, &state); diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index 2caec621d73..40f93239c8c 100644 --- a/src/backend/nodes/makefuncs.c +++ b/src/backend/nodes/makefuncs.c @@ -478,10 +478,14 @@ makeRangeVar(char *schemaname, char *relname, int location) r->schemaname = schemaname; r->relname = relname; r->inh = true; - r->relpersistence = RELPERSISTENCE_PERMANENT; r->alias = NULL; r->location = location; + if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0) + r->relpersistence = RELPERSISTENCE_TEMP; + else + r->relpersistence = RELPERSISTENCE_PERMANENT; + return r; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index c567252acc4..299adebae37 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -19597,7 +19597,11 @@ makeRangeVarFromAnyName(List *names, int position, core_yyscan_t yyscanner) break; } - r->relpersistence = RELPERSISTENCE_PERMANENT; + if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0) + r->relpersistence = RELPERSISTENCE_TEMP; + else + r->relpersistence = RELPERSISTENCE_PERMANENT; + r->location = position; return r; @@ -19637,6 +19641,11 @@ makeRangeVarFromQualifiedName(char *name, List *namelist, int location, break; } + if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0) + r->relpersistence = RELPERSISTENCE_TEMP; + else + r->relpersistence = RELPERSISTENCE_PERMANENT; + return r; } diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index 1a25973685c..bf627ab6577 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -90,6 +90,8 @@ typedef enum RVROption RVR_MISSING_OK = 1 << 0, /* don't error if relation doesn't exist */ RVR_NOWAIT = 1 << 1, /* error if relation cannot be locked */ RVR_SKIP_LOCKED = 1 << 2, /* skip if relation cannot be locked */ + RVR_OTHER_TEMP_OK = 1 << 3 /* don't error if relation is temp relation of + other session (needed for DROP command) */ } RVROption; typedef void (*RangeVarGetRelidCallback) (const RangeVar *relation, Oid relId, -- 2.43.0 [text/x-patch] v12-0002-Test-cross-session-access-restrictions-on-tempor.patch (5.1K, 3-v12-0002-Test-cross-session-access-restrictions-on-tempor.patch) download | inline diff: From 09113ecb70104c59c5da6187462058a8bc654a75 Mon Sep 17 00:00:00 2001 From: Jim Jones <[email protected]> Date: Fri, 26 Sep 2025 12:58:19 +0200 Subject: [PATCH v12 2/2] Test cross-session access restrictions on temporary tables These tests create a temporary table in one session and verify that another session cannot perform SELECT, UPDATE, DELETE, TRUNCATE, INSERT, ALTER, COPY, or LOCK operations on that table. It also confirms that DROP TABLE from another session succeeds. --- src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/011_temp_obj_multisession.pl | 126 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 src/test/modules/test_misc/t/011_temp_obj_multisession.pl diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 6e8db1621a7..356121673a1 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -19,6 +19,7 @@ tests += { 't/008_replslot_single_user.pl', 't/009_log_temp_files.pl', 't/010_index_concurrently_upsert.pl', + 't/011_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/011_temp_obj_multisession.pl b/src/test/modules/test_misc/t/011_temp_obj_multisession.pl new file mode 100644 index 00000000000..9b0deaaf485 --- /dev/null +++ b/src/test/modules/test_misc/t/011_temp_obj_multisession.pl @@ -0,0 +1,126 @@ +# Copyright (c) 2025, PostgreSQL Global Development Group + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +# Set up a fresh node +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Create a long-lived session +my $psql1 = $node->background_psql('postgres'); + +$psql1->query_safe( + q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + + +# SELECT TEMPORARY TABLE from other session +my ($stdout, $stderr); +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'SELECT on other session temp table is not allowed'); + +# UPDATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'UPDATE on other session temp table is not allowed'); + +# DELETE records from TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "DELETE FROM $tempschema.foo;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'DELETE on other session temp table is not allowed'); + +# TRUNCATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'TRUNCATE on other session temp table is not allowed'); + +# INSERT INTO TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'INSERT INTO on other session temp table is not allowed'); + +# ALTER TABLE .. RENAME TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "ALTER TABLE $tempschema.foo RENAME TO bar;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'ALTER TABLE ... RENAME on other session temp table is blocked'); + +# ALTER TABLE .. ADD COLUMN in TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "ALTER TABLE $tempschema.foo ADD COLUMN bar int;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'ALTER TABLE ... ADD COLUMN on other session temp table is blocked'); + +# COPY TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "COPY $tempschema.foo TO '/tmp/x';", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'COPY on other session temp table is blocked'); + +# LOCK TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "BEGIN; LOCK TABLE $tempschema.foo IN ACCESS EXCLUSIVE MODE;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'LOCK on other session temp table is blocked'); + +# DROP TEMPORARY TABLE from other session +my $ok = $node->psql( + 'postgres', + "DROP TABLE $tempschema.foo;" +); +ok($ok == 0, 'DROP TABLE executed successfully'); + +# Clean up +$psql1->quit; + +done_testing(); -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-03-23 10:22 Soumya S Murali <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Soumya S Murali @ 2026-03-23 10:22 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Jim Jones <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi all, Thank you for the updated patches. On Mon, Mar 23, 2026 at 12:56 PM Daniil Davydov <[email protected]> wrote: > > Hi, > > Posting rebased set of patches. I have gone through the patches and tested the patch series. I successfully reproduced the bug before applying the patch and found that the cross-session SELECT via schema-qualified temp table did not raise an error; instead, it returned an empty result, indicating inconsistent behaviour. After applying the patch, regression tests passed successfully and new regression tests behave as expected. Temporary tables from other sessions are now no longer visible and all the attempts to access them result in error. Also I verified:- Schema-qualified SELECT, INSERT / UPDATE / DELETE, JOIN queries, Subqueries and EXPLAIN worked as expected. Same-session access also works as expected. Overall the patch LGTM. Regards, Soumya ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-03-24 17:26 Daniil Davydov <[email protected]> parent: Soumya S Murali <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-03-24 17:26 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Mon, Mar 23, 2026 at 8:31 PM Jim Jones <[email protected]> wrote: > > This is a step forward in really isolating contents of temp tables from > other sessions, but the more I think about it, the more I'm concerned > with the current approach -- I spent some time investigating this > problem a bit deeper last week. > > My main concern is the usage of gram.y, as a parser is arguably fragile > for this kind of things. I don't actually want to use gram.y as a main solver of this issue. But gram.y is setting the "relpersistence" field for the RangeVar and all subsequent code is treating this value as truthful. In particular, v12 patch is relying on this field in RangeVar during the resolution of access issues, and IMHO it's not out of line with the current code base. For instance, we are considering RangeVar to determine whether we can perform a CREATE operation within the specified namespace (see RangeVarAdjustRelationPersistence). Even if we decide not to touch the gram.y in this patch, I still think that leaving the "relpesistence" field misleading may lead to more bugs appearing in the future. I.e. it should be fixed anyway (maybe in another thread?). > For instance, one can always change the > search_path and bypass this restriction: > > (table t was created in a different session) > > postgres=# SELECT * FROM pg_temp_81.t; > ERROR: cannot access temporary relations of other sessions > LINE 1: SELECT * FROM pg_temp_81.t; > ^ > postgres=# SET search_path = pg_temp_81, public; > SET > postgres=# SELECT * FROM t; > ?column? > ---------- > (0 rows) > > * See: if (relation->relpersistence == RELPERSISTENCE_TEMP) in > namespace.c for more details. Yeah, you are right. It turns out that the current patch doesn't fully protect other temp tables from superuser. The first thought that comes to me is to forbid setting other-temp-namespaces in a search_path parameter. I know it's starting to look ugly. But actually, such a restriction seems quite logical. > > IMO, since it is an access control issue, I guess we better treat it as > such and modify aclchk.c instead. > > Something like this the file attached. This breaks an unrelated test, > which is potentially a bug in REPACK ... but I'll describe it in another > thread. > > Thoughts? Thank you for the patch! It seems much more beautiful and convenient to maintain, but I have a little concern about it. Actually, in your implementation we can DROP other temp tables not because we make an exception to the general rule ("don't touch other temp tables"), but because we just can perform such operations with every object in pg_temp_0. If other operations with the same access checking as DROP command appear in the future, the user will be able to perform these operations for other-temp-tables. I.e. we will need to manually prevent such new operations from accessing other-temp-tables. Have I gone too far in my reasoning? BTW, your implementation allows calling a VACUUM for other-temp-tables. I think that we should forbid that too. On Mon, Mar 23, 2026 at 8:44 PM Tom Lane <[email protected]> wrote: > > Jim Jones <[email protected]> writes: > > This is a step forward in really isolating contents of temp tables from > > other sessions, but the more I think about it, the more I'm concerned > > with the current approach -- I spent some time investigating this > > problem a bit deeper last week. > > Yeah. I think this entire approach is wrongheaded: we do not enforce > permissions checks against superusers. Moreover, if we try to fix it > at the permissions level, it seems nearly certain that there will be > bypass paths, simply because superusers bypass so many other checks. > Actually, v12 patch is not about a superuser rights restriction, but about forbidding such operations for everyone. Anyway, we have a new perl test that will prevent adding a code that will allow superuser to (somehow) break the protection of both mine and Jim's patches. Isn't that a sufficient guarantee that superuser will not bypass checks that it must not bypass? > The actual problem is that the buffer manager is incapable of dealing > with other sessions' temp tables, and we need to un-break the buffer > manager's defense for that implementation restriction. So I feel the > correct approach is something similar to what I described here: > > https://www.postgresql.org/message-id/flat/2736425.1758475979%40sss.pgh.pa.us > > I'm not wedded to that specific patch, but that is the implementation > level where the fix is needed. > Handling access to other-temp-tables on the buffer manager level seems to me like fighting the symptom, not the cause. Protection of other-temp-tables is kinda "upper-level logical restriction". At the same time, buffer manager is a lower-level implementation which shouldn't face such upper-level issues. Am I missing something? -- Best regards, Daniil Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-03-25 07:07 Daniil Davydov <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-03-25 07:07 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Wed, Mar 25, 2026 at 1:58 AM Tom Lane <[email protected]> wrote: > > > Actually, v12 patch is not about a superuser rights restriction, but about > > forbidding such operations for everyone. > > ... including superusers, who bypass permissions restrictions > everywhere else. You are going to have to contort the ACL system > badly to make that happen at all, and I would not be surprised > if you introduce new bugs. > I've never dealt with the ACL system before, so it's hard for me to assess the scale of the problem. I am inclined to believe you on this issue. > >> The actual problem is that the buffer manager is incapable of dealing > >> with other sessions' temp tables, and we need to un-break the buffer > >> manager's defense for that implementation restriction. So I feel the > >> correct approach is something similar to what I described here: > >> https://www.postgresql.org/message-id/flat/2736425.1758475979%40sss.pgh.pa.us > > > Handling access to other-temp-tables on the buffer manager level seems to me > > like fighting the symptom, not the cause. > > No, it IS the cause. If someday someone were to reimplement buffer > management in a way that didn't have this implementation restriction, > we would surely not arbitrarily restrict superusers from looking at > tables that they then would physically be able to look at. > OK, now I fully understand your point (I hope so) : We want to restrict backend access to other-temp-tables just because it is physically impossible for them to read the pages of such tables. And if some users has enough privileges, it is OK for us to allow them to lock/vacuum/drop/... other-temp-tables. I.e. only operations with heap pages access must be forbidden, and the buffer manager layer is an appropriate place for it. > > Am I missing something? > > Mainly, that we had a setup that was working fine for decades, > until somebody made holes in it with careless refactoring. > We should fix that mistake, not introduce inconsistent-with- > decades-of-practice permissions behavior to hide the mistake > at an unrelated logical level. > Yeah, we have a few checks in the bufmgr (PrefetchBuffer, ReadBufferExtended), but they stopped coping with their task. > Also, we need a defense at the buffer manager level anyway, because > otherwise C code could try to access another session's temp table > and we'd not realize it was getting bogus answers. (Whether such > an attempt is a bug or not is a different discussion; but we at > least need some logic that detects that it won't work, and the ACL > system cannot be expected to stop C-level code from trying.) > > Also, we really need a patch that's simple and non-invasive enough > to be back-patched into v17 and v18. This proposal is not that. > OK > > > I don't actually want to use gram.y as a main solver of this issue. But > > gram.y is setting the "relpersistence" field for the RangeVar and all > > subsequent code is treating this value as truthful. > > I do kind of agree with this concern, but the v12 patch simply moves > the untruthfulness around. Reality is that we cannot know whether an > unqualified-name RangeVar references a temp table until we do a > catalog lookup, ... Yep, Jim's example shows us that we cannot always rely on the "relpersistence" field. > ...so IMO we should not have a relpersistence field there > at all. At best it means something quite different from what it means > elsewhere, and that's a recipe for confusion. But changing that would > not be a bug fix (AFAIK) but refactoring to reduce the probability of > future bugs. > I agree with the idea to get rid of this field. By now I cannot say for sure whether we can fix a bug without modifying the RangeVar structure. But I'll try to implement proposed logic only within the bufmgr. Thank you very much for your comments! I'll post a new patch in the near future. -- Best regards, Daniil Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-08 09:17 Soumya S Murali <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Soumya S Murali @ 2026-04-08 09:17 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Tom Lane <[email protected]>; Jim Jones <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi all, On Wed, Mar 25, 2026 at 12:38 PM Daniil Davydov <[email protected]> wrote: > > Hi, > > On Wed, Mar 25, 2026 at 1:58 AM Tom Lane <[email protected]> wrote: > > > > > Actually, v12 patch is not about a superuser rights restriction, but about > > > forbidding such operations for everyone. > > > > ... including superusers, who bypass permissions restrictions > > everywhere else. You are going to have to contort the ACL system > > badly to make that happen at all, and I would not be surprised > > if you introduce new bugs. > > > > I've never dealt with the ACL system before, so it's hard for me to assess > the scale of the problem. I am inclined to believe you on this issue. > > > >> The actual problem is that the buffer manager is incapable of dealing > > >> with other sessions' temp tables, and we need to un-break the buffer > > >> manager's defense for that implementation restriction. So I feel the > > >> correct approach is something similar to what I described here: > > >> https://www.postgresql.org/message-id/flat/2736425.1758475979%40sss.pgh.pa.us > > > > > Handling access to other-temp-tables on the buffer manager level seems to me > > > like fighting the symptom, not the cause. > > > > No, it IS the cause. If someday someone were to reimplement buffer > > management in a way that didn't have this implementation restriction, > > we would surely not arbitrarily restrict superusers from looking at > > tables that they then would physically be able to look at. > > > > OK, now I fully understand your point (I hope so) : > We want to restrict backend access to other-temp-tables just because it is > physically impossible for them to read the pages of such tables. And if some > users has enough privileges, it is OK for us to allow them to > lock/vacuum/drop/... other-temp-tables. I.e. only operations with heap pages > access must be forbidden, and the buffer manager layer is an appropriate place > for it. > > > > Am I missing something? > > > > Mainly, that we had a setup that was working fine for decades, > > until somebody made holes in it with careless refactoring. > > We should fix that mistake, not introduce inconsistent-with- > > decades-of-practice permissions behavior to hide the mistake > > at an unrelated logical level. > > > > Yeah, we have a few checks in the bufmgr (PrefetchBuffer, ReadBufferExtended), > but they stopped coping with their task. > > > Also, we need a defense at the buffer manager level anyway, because > > otherwise C code could try to access another session's temp table > > and we'd not realize it was getting bogus answers. (Whether such > > an attempt is a bug or not is a different discussion; but we at > > least need some logic that detects that it won't work, and the ACL > > system cannot be expected to stop C-level code from trying.) > > > > Also, we really need a patch that's simple and non-invasive enough > > to be back-patched into v17 and v18. This proposal is not that. > > > > OK > > > > > > I don't actually want to use gram.y as a main solver of this issue. But > > > gram.y is setting the "relpersistence" field for the RangeVar and all > > > subsequent code is treating this value as truthful. > > > > I do kind of agree with this concern, but the v12 patch simply moves > > the untruthfulness around. Reality is that we cannot know whether an > > unqualified-name RangeVar references a temp table until we do a > > catalog lookup, ... > > Yep, Jim's example shows us that we cannot always rely on the "relpersistence" > field. > > > ...so IMO we should not have a relpersistence field there > > at all. At best it means something quite different from what it means > > elsewhere, and that's a recipe for confusion. But changing that would > > not be a bug fix (AFAIK) but refactoring to reduce the probability of > > future bugs. > > > > I agree with the idea to get rid of this field. By now I cannot say for sure > whether we can fix a bug without modifying the RangeVar structure. But I'll > try to implement proposed logic only within the bufmgr. > > > Thank you very much for your comments! I'll post a new patch in the near > future. I worked on the issue of accessing temporary tables belonging to other sessions and tried implementing the fix at the buffer manager level, as suggested. I added checks in ReadBuffer_common() and PrefetchBuffer() to reject access when a relation is temporary (relpersistence = TEMP) but does not use local buffers (!RelationUsesLocalBuffers) so that it ensures only heap page access is blocked, while catalog lookups and other metadata operations continue to work as before. While testing, I observed that in many cases the query does not reach the buffer manager because name resolution fails earlier with “relation does not exist”. However, the added checks ensure that even if execution reaches the buffer layer, access to other sessions’ temporary tables is safely rejected. The change is minimal, and did not modify parser/ACL behavior and all regression tests got passed successfully too. Kindly review the attached patch herewith. Please let me know if this approach aligns with expectations or if further adjustments are needed. Regards, Soumya Attachments: [text/x-patch] 0001-prevent-access-to-other-sessions-temporary-tables.patch (2.5K, 2-0001-prevent-access-to-other-sessions-temporary-tables.patch) download | inline diff: From 88ea675233f3078c77740ca3ef2493d11381c3d9 Mon Sep 17 00:00:00 2001 From: Soumya <[email protected]> Date: Wed, 8 Apr 2026 12:47:45 +0530 Subject: [PATCH] prevent access to other sessions' temporary tables Signed-off-by: Soumya <[email protected]> --- src/backend/storage/buffer/bufmgr.c | 32 +++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..ec72fa97ea6 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -791,11 +791,16 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RelationUsesLocalBuffers(reln)) { - /* see comments in ReadBufferExtended */ - if (RELATION_IS_OTHER_TEMP(reln)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + /* ACCESS DENIED CHECK */ + if (reln != NULL && + reln->rd_rel != NULL && + reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP && + !RelationUsesLocalBuffers(reln)) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + } /* pass it off to localbuf.c */ return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum); @@ -933,7 +938,8 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, * likely to get wrong data since we have no visibility into the owning * session's local buffers. */ - if (RELATION_IS_OTHER_TEMP(reln)) + if (reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP && + !RelationUsesLocalBuffers(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary tables of other sessions"))); @@ -1313,9 +1319,19 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, } if (rel) - persistence = rel->rd_rel->relpersistence; + persistence = rel->rd_rel->relpersistence; else - persistence = smgr_persistence; + persistence = smgr_persistence; + + /* ACCESS DENIED */ + if (rel != NULL && + persistence == RELPERSISTENCE_TEMP && + ! RelationUsesLocalBuffers(rel)) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + } if (unlikely(mode == RBM_ZERO_AND_CLEANUP_LOCK || mode == RBM_ZERO_AND_LOCK)) -- 2.34.1 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-08 10:41 Jim Jones <[email protected]> parent: Soumya S Murali <[email protected]> 0 siblings, 2 replies; 52+ messages in thread From: Jim Jones @ 2026-04-08 10:41 UTC (permalink / raw) To: Soumya S Murali <[email protected]>; Daniil Davydov <[email protected]>; +Cc: Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi On 08/04/2026 11:17, Soumya S Murali wrote: > I worked on the issue of accessing temporary tables belonging to other > sessions and tried implementing the fix at the buffer manager level, > as suggested. I added checks in ReadBuffer_common() and > PrefetchBuffer() to reject access when a relation is temporary > (relpersistence = TEMP) but does not use local buffers > (!RelationUsesLocalBuffers) so that it ensures only heap page access > is blocked, while catalog lookups and other metadata operations > continue to work as before. While testing, I observed that in many > cases the query does not reach the buffer manager because name > resolution fails earlier with “relation does not exist”. However, the > added checks ensure that even if execution reaches the buffer layer, > access to other sessions’ temporary tables is safely rejected. The > change is minimal, and did not modify parser/ACL behavior and all > regression tests got passed successfully too. > Kindly review the attached patch herewith. Please let me know if this > approach aligns with expectations or if further adjustments are > needed. A few comments: == PrefetchBuffer == The condition nested inside the if (RelationUsesLocalBuffers(reln)) tests the opposite of the main if !RelationUsesLocalBuffers(reln): if (RelationUsesLocalBuffers(reln)) { /* ACCESS DENIED CHECK */ if (reln != NULL && reln->rd_rel != NULL && reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP && !RelationUsesLocalBuffers(reln)) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary tables of other sessions"))); } ... } So it'll be always false, making the ereport unreachable. == ReadBufferExtended == These conditions cancel each other out: if (reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP && !RelationUsesLocalBuffers(reln)) RelationUsesLocalBuffers(reln) expands to ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP), making the error message unreachable. Perhaps you meant RELATION_IS_OTHER_TEMP? == ReadBuffer_common == Same as in ReadBufferExtended and PrefetchBuffer. == tests == You excluded the tests from the patch. == patch version == You forgot to add the patch version. Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-08 14:04 Jim Jones <[email protected]> parent: Jim Jones <[email protected]> 1 sibling, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-04-08 14:04 UTC (permalink / raw) To: Soumya S Murali <[email protected]>; Daniil Davydov <[email protected]>; +Cc: Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, I did some digging: It seems that b7b0f3f2724 is what actually broke it. By switching from ReadBufferExtended() to read_stream_next_buffer(), it silently routed all SELECT/UPDATE/DELETE/COPY away from the check that was sitting in ReadBufferExtended(). @@ -528,25 +599,23 @@ heap_fetch_next_buffer(HeapScanDesc scan, ScanDirection dir) */ CHECK_FOR_INTERRUPTS(); - if (unlikely(!scan->rs_inited)) + /* + * If the scan direction is changing, reset the prefetch block to the + * current block. Otherwise, we will incorrectly prefetch the blocks + * between the prefetch block and the current block again before + * prefetching blocks in the new, correct scan direction. + */ + if (unlikely(scan->rs_dir != dir)) { - scan->rs_cblock = heapgettup_initial_block(scan, dir); + scan->rs_prefetch_block = scan->rs_cblock; + read_stream_reset(scan->rs_read_stream); + } - /* ensure rs_cbuf is invalid when we get InvalidBlockNumber */ - Assert(scan->rs_cblock != InvalidBlockNumber || - !BufferIsValid(scan->rs_cbuf)); + scan->rs_dir = dir; - scan->rs_inited = true; - } - else - scan->rs_cblock = heapgettup_advance_block(scan, scan->rs_cblock, - dir); - - /* read block if valid */ - if (BlockNumberIsValid(scan->rs_cblock)) - scan->rs_cbuf = ReadBufferExtended(scan->rs_base.rs_rd, MAIN_FORKNUM, - scan->rs_cblock, RBM_NORMAL, - scan->rs_strategy); + scan->rs_cbuf = read_stream_next_buffer(scan->rs_read_stream, NULL); + if (BufferIsValid(scan->rs_cbuf)) + scan->rs_cblock = BufferGetBlockNumber(scan->rs_cbuf); } It simply builds upon 210622c60e1 (introduces StartReadBuffers -> PinBufferForBlock) and b5a9b18cd0b (builds read_stream_begin_relation on top of it). So I thought we can explore the option of adding this check directly in read_stream_begin_relation(): if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary relations of other sessions"))); Thoughts? See v15 attached. Daniil, feel free to revert it to your last patch if you disagree with this approach. Best, Jim Attachments: [text/x-patch] v15-0001-Prevent-access-to-other-sessions-temporary-table.patch (2.8K, 2-v15-0001-Prevent-access-to-other-sessions-temporary-table.patch) download | inline diff: From bd15be73156acc258fd06c7b77a875fb2e154fe8 Mon Sep 17 00:00:00 2001 From: Jim Jones <[email protected]> Date: Wed, 8 Apr 2026 14:53:06 +0200 Subject: [PATCH v15 1/2] Prevent access to other sessions' temporary tables --- src/backend/storage/aio/read_stream.c | 10 ++++++++++ src/backend/storage/buffer/bufmgr.c | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index b6fce4e7cc6..e64c74040e8 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -972,6 +972,16 @@ read_stream_begin_relation(int flags, void *callback_private_data, size_t per_buffer_data_size) { + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + return read_stream_begin_impl(flags, strategy, rel, diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..4c464d51f3c 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -795,7 +795,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* pass it off to localbuf.c */ return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum); @@ -936,7 +936,7 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* * Read the buffer, and update pgstat counters to reflect a cache hit or @@ -1317,6 +1317,16 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, else persistence = smgr_persistence; + /* + * Safety net for callers (e.g. via ExtendBufferedRelTo) that reach here + * without going through ReadBufferExtended. See ReadBufferExtended for + * the primary check and full explanation. + */ + if (rel != NULL && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + if (unlikely(mode == RBM_ZERO_AND_CLEANUP_LOCK || mode == RBM_ZERO_AND_LOCK)) { -- 2.43.0 [text/x-patch] v15-0002-Test-cross-session-access-on-temporary-tables.patch (4.7K, 3-v15-0002-Test-cross-session-access-on-temporary-tables.patch) download | inline diff: From 3a463f9c3eff8aab838efe1b571a9117b96106cd Mon Sep 17 00:00:00 2001 From: Jim Jones <[email protected]> Date: Wed, 8 Apr 2026 14:56:57 +0200 Subject: [PATCH v15 2/2] Test cross-session access on temporary tables --- src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/012_temp_obj_multisession.pl | 117 ++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 src/test/modules/test_misc/t/012_temp_obj_multisession.pl diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 1b25d98f7f3..a54599cc301 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -20,6 +20,7 @@ tests += { 't/009_log_temp_files.pl', 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', + 't/012_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/012_temp_obj_multisession.pl b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl new file mode 100644 index 00000000000..4d599152740 --- /dev/null +++ b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl @@ -0,0 +1,117 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +# Set up a fresh node +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Create a long-lived session +my $psql1 = $node->background_psql('postgres'); + +$psql1->query_safe( + q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +# Create an index so the index-scan path (ReadBufferExtended) can be tested +$psql1->query_safe( + q(CREATE INDEX ON foo(val);)); + +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + + +# SELECT TEMPORARY TABLE from other session +my ($stdout, $stderr); +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'SELECT on other session temp table is not allowed'); + +# UPDATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'UPDATE on other session temp table is not allowed'); + +# DELETE records from TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "DELETE FROM $tempschema.foo;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'DELETE on other session temp table is not allowed'); + +# TRUNCATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr +); +like($stderr, qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE on other session temp table is not allowed'); + +# INSERT INTO TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'INSERT INTO on other session temp table is not allowed'); + +# COPY TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'COPY on other session temp table is blocked'); + +# SELECT via index scan from other session. +# Sequential scans are blocked at read_stream_begin_relation(); index scans +# bypass that path entirely and reach ReadBufferExtended() in bufmgr.c +# (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches). +# enable_seqscan=off forces the planner to use the index. +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'index scan on other session temp table is not allowed (exercises ReadBufferExtended path)'); + +# DROP TEMPORARY TABLE from other session +my $ok = $node->psql( + 'postgres', + "DROP TABLE $tempschema.foo;" +); +ok($ok == 0, 'DROP TABLE executed successfully'); + +# Clean up +$psql1->quit; + +done_testing(); -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-09 11:46 Daniil Davydov <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-04-09 11:46 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Wed, Apr 8, 2026 at 9:04 PM Jim Jones <[email protected]> wrote: > > I did some digging: > > It seems that b7b0f3f2724 is what actually broke it. By switching from > ReadBufferExtended() to read_stream_next_buffer(), it silently routed > all SELECT/UPDATE/DELETE/COPY away from the check that was sitting in > ReadBufferExtended(). > Yeah, I agree with your conclusions. > It simply builds upon 210622c60e1 (introduces StartReadBuffers -> > PinBufferForBlock) and b5a9b18cd0b (builds read_stream_begin_relation on > top of it). > > So I thought we can explore the option of adding this check directly in > read_stream_begin_relation(): > > > if (RELATION_IS_OTHER_TEMP(rel)) > ereport(ERROR, > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > errmsg("cannot access temporary relations of other sessions"))); > > > Thoughts? > Thank you very much for the patch! A few comments: 1) Right now, read stream seems like an appropriate place for this restriction. But actually the StartReadBuffers is not "binded" to the read stream logic. I mean that if someone calls it bypassing the "read_stream_begin_relation" function (it is OK to do so), then our restriction will be violated again. I think that it will be more reliable to add the restriction directly to the StartReadBuffers. Also, we can add an assertion ("relation is not other temp table") to the PinBufferForBlock. What do you think? 2) If we decide to leave restriction in the "read_stream_begin_relation" function, I would suggest adding a "rel != NULL" check here (read_stream.c): + if (RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); 3) The "rel != NULL" checks may use the "RelationIsValid" macro, which seems more pretty to me. > Daniil, feel free to revert it to your last patch if you disagree with > this approach. This approach looks good to me after Tom's explanations. -- Best regards, Daniil Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-09 14:35 Jim Jones <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-04-09 14:35 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi Daniil On 09/04/2026 13:46, Daniil Davydov wrote: > 1) Right now, read stream seems like an appropriate place for this restriction. > But actually the StartReadBuffers is not "binded" to the read stream logic. I > mean that if someone calls it bypassing the "read_stream_begin_relation" > function (it is OK to do so), then our restriction will be violated again. > I think that it will be more reliable to add the restriction directly to the > StartReadBuffers. Also, we can add an assertion ("relation is not other temp > table") to the PinBufferForBlock. What do you think?> 2) If we decide to leave restriction in the "read_stream_begin_relation" > function, I would suggest adding a "rel != NULL" check here (read_stream.c): > + if (RELATION_IS_OTHER_TEMP(rel)) > + ereport(ERROR, > + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > + errmsg("cannot access temporary relations of other > sessions"))); > 3) The "rel != NULL" checks may use the "RelationIsValid" macro, which seems > more pretty to me. Mm, not so sure... AFAICT moving the check to StartReadBuffersImpl would require an extra NULL guard that isn't needed in read_stream_begin_relation, as the callers already pass valid Relations. So, rel != NULL is not needed. Also, wouldn't it potentially make this check multiple times in a table scan? Am I missing something? Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-09 15:29 Daniil Davydov <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-04-09 15:29 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Thu, Apr 9, 2026 at 9:35 PM Jim Jones <[email protected]> wrote: > > On 09/04/2026 13:46, Daniil Davydov wrote: > > 1) Right now, read stream seems like an appropriate place for this restriction. > > But actually the StartReadBuffers is not "binded" to the read stream logic. I > > mean that if someone calls it bypassing the "read_stream_begin_relation" > > function (it is OK to do so), then our restriction will be violated again. > > I think that it will be more reliable to add the restriction directly to the > > StartReadBuffers. Also, we can add an assertion ("relation is not other temp > > table") to the PinBufferForBlock. What do you think?> 2) If we decide to leave restriction in the "read_stream_begin_relation" > > function, I would suggest adding a "rel != NULL" check here (read_stream.c): > > + if (RELATION_IS_OTHER_TEMP(rel)) > > + ereport(ERROR, > > + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > > + errmsg("cannot access temporary relations of other > > sessions"))); > > 3) The "rel != NULL" checks may use the "RelationIsValid" macro, which seems > > more pretty to me. > > > Mm, not so sure... > AFAICT moving the check to StartReadBuffersImpl would require an extra > NULL guard that isn't needed in read_stream_begin_relation, as the > callers already pass valid Relations. So, rel != NULL is not needed. Hm. I see that read_stream_begin_relation immediately calls read_stream_begin_impl, where we have a "rel != NULL" check (read_stream.c:787). Anyway, I think that we shouldn't rely on the fact that a given Relation will always be valid. Please, correct me if I am wrong. I see that you don't really like the idea of moving this check. But since a vectored variant of ReadBuffer() may be used by anyone, don't we need to take it into account? > Also, wouldn't it potentially make this check multiple times in a table > scan? Yep, it will. It is exactly the same logic as for ReadBuffer_common, PrefetchBuffer and ReadBufferExtended (i.e. checking this constraint before each buffer read). I don't see anything wrong with this approach. More precisely, it would be good to avoid multiple checks, but I don't see a way to do that. -- Best regards, Daniil Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-09 17:46 Jim Jones <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-04-09 17:46 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers On 09/04/2026 17:29, Daniil Davydov wrote: > On Thu, Apr 9, 2026 at 9:35 PM Jim Jones <[email protected]> wrote: > Hm. I see that read_stream_begin_relation immediately calls > read_stream_begin_impl, where we have a "rel != NULL" check (read_stream.c:787). > Anyway, I think that we shouldn't rely on the fact that a given Relation will > always be valid. Please, correct me if I am wrong. > > I see that you don't really like the idea of moving this check. But since a > vectored variant of ReadBuffer() may be used by anyone, don't we need to take > it into account? >> Also, wouldn't it potentially make this check multiple times in a table >> scan? > Yep, it will. It is exactly the same logic as for ReadBuffer_common, > PrefetchBuffer and ReadBufferExtended (i.e. checking this constraint before > each buffer read). I don't see anything wrong with this approach. More > precisely, it would be good to avoid multiple checks, but I don't see a way to > do that. This check exists because read_stream_begin_smgr_relation() passes NULL, but I see your point. I guess a check in read_stream_begin_relation() and in StartReadBuffersImpl() would be the best solution? If you agree, could you add it in v16? Thanks! Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-10 07:10 Daniil Davydov <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 2 replies; 52+ messages in thread From: Daniil Davydov @ 2026-04-10 07:10 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Fri, Apr 10, 2026 at 12:46 AM Jim Jones <[email protected]> wrote: > > I guess a check in read_stream_begin_relation() > and in StartReadBuffersImpl() would be the best solution? If you agree, > could you add it in v16? Having both checks might look a bit redundant since the read stream will eventually call the StartReadBuffersImpl function. On the other hand, there are many places which are checking this restriction even if subsequent functions (from bufmgr) also have this check. So, I'll keep both checks and a bit reduce the comments in the bufmgr.c . BTW, what do you think about making this comment less "concrete"? : # SELECT via index scan from other session. # Sequential scans are blocked at read_stream_begin_relation(); index scans # bypass that path entirely and reach ReadBufferExtended() in bufmgr.c # (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches). # enable_seqscan=off forces the planner to use the index. I mean that if the described logic changes, this comment will become confusing. We can describe the test in general words. For example : # Index scans can use a different code path from the one sequential scans are # following. Make sure that we cannot access other sessions' temp tables during # index scan either. Thank you for the comments! Please, see an updated set of patches. -- Best regards, Daniil Davydov Attachments: [text/x-patch] v16-0002-Test-cross-session-access-on-temporary-tables.patch (4.7K, 2-v16-0002-Test-cross-session-access-on-temporary-tables.patch) download | inline diff: From 07dd5ecd204420678a100b64a01347227f4fef4f Mon Sep 17 00:00:00 2001 From: Jim Jones <[email protected]> Date: Wed, 8 Apr 2026 14:56:57 +0200 Subject: [PATCH v16 2/2] Test cross-session access on temporary tables --- src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/012_temp_obj_multisession.pl | 117 ++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 src/test/modules/test_misc/t/012_temp_obj_multisession.pl diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 1b25d98f7f3..a54599cc301 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -20,6 +20,7 @@ tests += { 't/009_log_temp_files.pl', 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', + 't/012_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/012_temp_obj_multisession.pl b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl new file mode 100644 index 00000000000..4d599152740 --- /dev/null +++ b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl @@ -0,0 +1,117 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +# Set up a fresh node +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Create a long-lived session +my $psql1 = $node->background_psql('postgres'); + +$psql1->query_safe( + q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +# Create an index so the index-scan path (ReadBufferExtended) can be tested +$psql1->query_safe( + q(CREATE INDEX ON foo(val);)); + +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + + +# SELECT TEMPORARY TABLE from other session +my ($stdout, $stderr); +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'SELECT on other session temp table is not allowed'); + +# UPDATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'UPDATE on other session temp table is not allowed'); + +# DELETE records from TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "DELETE FROM $tempschema.foo;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'DELETE on other session temp table is not allowed'); + +# TRUNCATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr +); +like($stderr, qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE on other session temp table is not allowed'); + +# INSERT INTO TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'INSERT INTO on other session temp table is not allowed'); + +# COPY TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'COPY on other session temp table is blocked'); + +# SELECT via index scan from other session. +# Sequential scans are blocked at read_stream_begin_relation(); index scans +# bypass that path entirely and reach ReadBufferExtended() in bufmgr.c +# (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches). +# enable_seqscan=off forces the planner to use the index. +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr +); +like($stderr, qr/cannot access temporary relations of other sessions/, + 'index scan on other session temp table is not allowed (exercises ReadBufferExtended path)'); + +# DROP TEMPORARY TABLE from other session +my $ok = $node->psql( + 'postgres', + "DROP TABLE $tempschema.foo;" +); +ok($ok == 0, 'DROP TABLE executed successfully'); + +# Clean up +$psql1->quit; + +done_testing(); -- 2.43.0 [text/x-patch] v16-0001-Prevent-access-to-other-sessions-temp-tables.patch (3.2K, 3-v16-0001-Prevent-access-to-other-sessions-temp-tables.patch) download | inline diff: From 6b25b30bc981dd6420788014face14a13bb9461d Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Fri, 10 Apr 2026 13:51:14 +0700 Subject: [PATCH v16 1/2] Prevent access to other sessions' temp tables --- src/backend/storage/aio/read_stream.c | 10 ++++++++++ src/backend/storage/buffer/bufmgr.c | 17 +++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index b6fce4e7cc6..213f6206ba2 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. That * currently means advice to the kernel to tell it that we will soon read. diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..653278140fb 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -795,7 +795,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* pass it off to localbuf.c */ return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum); @@ -936,7 +936,7 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* * Read the buffer, and update pgstat counters to reflect a cache hit or @@ -1317,6 +1317,12 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, else persistence = smgr_persistence; + /* see comments in ReadBufferExtended */ + if (rel != NULL && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + if (unlikely(mode == RBM_ZERO_AND_CLEANUP_LOCK || mode == RBM_ZERO_AND_LOCK)) { @@ -1393,6 +1399,13 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, io_object = IOOBJECT_RELATION; } + + /* see comments in ReadBufferExtended */ + if (operation->rel != NULL && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + for (int i = 0; i < actual_nblocks; ++i) { bool found; -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-10 10:28 Jim Jones <[email protected]> parent: Daniil Davydov <[email protected]> 1 sibling, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-04-10 10:28 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi Daniil On 10/04/2026 09:10, Daniil Davydov wrote: > Having both checks might look a bit redundant since the read stream will > eventually call the StartReadBuffersImpl function. On the other hand, there are > many places which are checking this restriction even if subsequent functions > (from bufmgr) also have this check. > > So, I'll keep both checks and a bit reduce the comments in the bufmgr.c . Putting this check a level deeper sounds good to me. > BTW, what do you think about making this comment less "concrete"? : > # SELECT via index scan from other session. > # Sequential scans are blocked at read_stream_begin_relation(); index scans > # bypass that path entirely and reach ReadBufferExtended() in bufmgr.c > # (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches). > # enable_seqscan=off forces the planner to use the index. > > I mean that if the described logic changes, this comment will become confusing. > We can describe the test in general words. For example : > # Index scans can use a different code path from the one sequential scans are > # following. Make sure that we cannot access other sessions' temp tables during > # index scan either. +1 Yeah, it's indeed too verbose. I guess these comments were originally just for me so I wouldn't get too confused along the way :) I don't have anything else to add at this point. Unless there are any objections, I'll mark the CF entry as 'Ready for Committer.' Thanks! Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-10 15:28 Daniil Davydov <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-04-10 15:28 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Fri, Apr 10, 2026 at 5:29 PM Jim Jones <[email protected]> wrote: > > > BTW, what do you think about making this comment less "concrete"? : > > # SELECT via index scan from other session. > > # Sequential scans are blocked at read_stream_begin_relation(); index scans > > # bypass that path entirely and reach ReadBufferExtended() in bufmgr.c > > # (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches). > > # enable_seqscan=off forces the planner to use the index. > > > > I mean that if the described logic changes, this comment will become confusing. > > We can describe the test in general words. For example : > > # Index scans can use a different code path from the one sequential scans are > > # following. Make sure that we cannot access other sessions' temp tables during > > # index scan either. > > +1 > > Yeah, it's indeed too verbose. I guess these comments were originally > just for me so I wouldn't get too confused along the way :) OK :) > > I don't have anything else to add at this point. Unless there are any > objections, I'll mark the CF entry as 'Ready for Committer.' > Great, thank you! Please, see an updated set of patches (only perl test has been changed) : 1) Rephrase the discussed comment. 2) Use safe_psql whenever possible. 3) Run pgperltidy. -- Best regards, Daniil Davydov Attachments: [text/x-patch] v17-0001-Prevent-access-to-other-sessions-temp-tables.patch (3.3K, 2-v17-0001-Prevent-access-to-other-sessions-temp-tables.patch) download | inline diff: From 9045b387f7722193d4239dc2dbb73793f3978866 Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Fri, 10 Apr 2026 13:51:14 +0700 Subject: [PATCH v17 1/2] Prevent access to other sessions' temp tables --- src/backend/storage/aio/read_stream.c | 10 ++++++++++ src/backend/storage/buffer/bufmgr.c | 16 ++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index b6fce4e7cc6..213f6206ba2 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. That * currently means advice to the kernel to tell it that we will soon read. diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..70353f1f8e3 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -795,7 +795,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* pass it off to localbuf.c */ return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum); @@ -936,7 +936,7 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* * Read the buffer, and update pgstat counters to reflect a cache hit or @@ -1292,6 +1292,12 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, int flags; char persistence; + /* see comments in ReadBufferExtended */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + /* * Backward compatibility path, most code should use ExtendBufferedRel() * instead, as acquiring the extension lock inside ExtendBufferedRel() @@ -1382,6 +1388,12 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, Assert(*nblocks > 0); Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + /* see comments in ReadBufferExtended */ + if (operation->rel && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + if (operation->persistence == RELPERSISTENCE_TEMP) { io_context = IOCONTEXT_NORMAL; -- 2.43.0 [text/x-patch] v17-0002-Test-cross-session-access-on-temporary-tables.patch (4.3K, 3-v17-0002-Test-cross-session-access-on-temporary-tables.patch) download | inline diff: From 91a6caa33c68afd73e0418972d5a582fe6fd7bdf Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Fri, 10 Apr 2026 22:21:23 +0700 Subject: [PATCH v17 2/2] Test cross session access on temporary tables --- src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/012_temp_obj_multisession.pl | 109 ++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 src/test/modules/test_misc/t/012_temp_obj_multisession.pl diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 1b25d98f7f3..a54599cc301 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -20,6 +20,7 @@ tests += { 't/009_log_temp_files.pl', 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', + 't/012_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/012_temp_obj_multisession.pl b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl new file mode 100644 index 00000000000..fa8037e4d1e --- /dev/null +++ b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl @@ -0,0 +1,109 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +# Set up a fresh node +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Create a long-lived session +my $psql1 = $node->background_psql('postgres'); + +$psql1->query_safe(q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +$psql1->query_safe(q(CREATE INDEX ON foo(val);)); + +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + + +# SELECT TEMPORARY TABLE from other session +my ($stdout, $stderr); +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'SELECT on other session temp table is not allowed'); + +# UPDATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'UPDATE on other session temp table is not allowed'); + +# DELETE records from TEMPORARY TABLE from other session +$node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'DELETE on other session temp table is not allowed'); + +# TRUNCATE TEMPORARY TABLE from other session +$node->psql('postgres', "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr); +like( + $stderr, + qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE on other session temp table is not allowed'); + +# INSERT INTO TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'INSERT INTO on other session temp table is not allowed'); + +# COPY TEMPORARY TABLE from other session +$node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'COPY on other session temp table is blocked'); + +# Index scans can use a different code path from the one sequential scans are +# following. Make sure that we cannot access other sessions' temp tables during +# index scan either. +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'index scan on other session temp table is not allowed (exercises ReadBufferExtended path)' +); + +# DROP TEMPORARY TABLE from other session +$node->safe_psql('postgres', "DROP TABLE $tempschema.foo;"); + +# Clean up +$psql1->quit; + +done_testing(); -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-13 12:36 Soumya S Murali <[email protected]> parent: Jim Jones <[email protected]> 1 sibling, 0 replies; 52+ messages in thread From: Soumya S Murali @ 2026-04-13 12:36 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi all, On Wed, Apr 8, 2026 at 4:11 PM Jim Jones <[email protected]> wrote: > > Hi > > On 08/04/2026 11:17, Soumya S Murali wrote: > > I worked on the issue of accessing temporary tables belonging to other > > sessions and tried implementing the fix at the buffer manager level, > > as suggested. I added checks in ReadBuffer_common() and > > PrefetchBuffer() to reject access when a relation is temporary > > (relpersistence = TEMP) but does not use local buffers > > (!RelationUsesLocalBuffers) so that it ensures only heap page access > > is blocked, while catalog lookups and other metadata operations > > continue to work as before. While testing, I observed that in many > > cases the query does not reach the buffer manager because name > > resolution fails earlier with “relation does not exist”. However, the > > added checks ensure that even if execution reaches the buffer layer, > > access to other sessions’ temporary tables is safely rejected. The > > change is minimal, and did not modify parser/ACL behavior and all > > regression tests got passed successfully too. > > Kindly review the attached patch herewith. Please let me know if this > > approach aligns with expectations or if further adjustments are > > needed. > > A few comments: > > == PrefetchBuffer == > > The condition nested inside the if (RelationUsesLocalBuffers(reln)) > tests the opposite of the main if !RelationUsesLocalBuffers(reln): > > if (RelationUsesLocalBuffers(reln)) > { > /* ACCESS DENIED CHECK */ > if (reln != NULL && > reln->rd_rel != NULL && > reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP && > !RelationUsesLocalBuffers(reln)) > { > ereport(ERROR, > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > errmsg("cannot access temporary tables of other sessions"))); > } > ... > } > > So it'll be always false, making the ereport unreachable. > > == ReadBufferExtended == > > These conditions cancel each other out: > > if (reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP && > !RelationUsesLocalBuffers(reln)) > > RelationUsesLocalBuffers(reln) expands to > ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP), making the > error message unreachable. Perhaps you meant RELATION_IS_OTHER_TEMP? > > == ReadBuffer_common == > > Same as in ReadBufferExtended and PrefetchBuffer. > > == tests == > > You excluded the tests from the patch. > > == patch version == > > You forgot to add the patch version. Thank you for the detailed review and for pointing out these issues. I understood that the condition I used with RelationUsesLocalBuffers() was incorrect and made the checks unreachable. Also the RELATION_IS_OTHER_TEMP() was not the appropriate macro to use here. Regarding PrefetchBuffer, placing the check inside the RelationUsesLocalBuffers() branch was also logically inconsistent. Apologies for missing the tests and patch versioning in my submission. I will ensure these are included properly in future iterations. Based on the subsequent discussion and the analysis of the regression introduced by routing through read_stream_next_buffer(), I agree that the fix is better in the read stream / buffer read entry points rather than only in bufmgr. Thank you for your guidance. It really helped clarify both the root cause and the correct placement of the fix. Regards, Soumya ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-13 12:40 Soumya S Murali <[email protected]> parent: Daniil Davydov <[email protected]> 1 sibling, 2 replies; 52+ messages in thread From: Soumya S Murali @ 2026-04-13 12:40 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi all, On Fri, Apr 10, 2026 at 12:40 PM Daniil Davydov <[email protected]> wrote: > > Hi, > > On Fri, Apr 10, 2026 at 12:46 AM Jim Jones <[email protected]> wrote: > > > > I guess a check in read_stream_begin_relation() > > and in StartReadBuffersImpl() would be the best solution? If you agree, > > could you add it in v16? > > Having both checks might look a bit redundant since the read stream will > eventually call the StartReadBuffersImpl function. On the other hand, there are > many places which are checking this restriction even if subsequent functions > (from bufmgr) also have this check. > > So, I'll keep both checks and a bit reduce the comments in the bufmgr.c . > > BTW, what do you think about making this comment less "concrete"? : > # SELECT via index scan from other session. > # Sequential scans are blocked at read_stream_begin_relation(); index scans > # bypass that path entirely and reach ReadBufferExtended() in bufmgr.c > # (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches). > # enable_seqscan=off forces the planner to use the index. > > I mean that if the described logic changes, this comment will become confusing. > We can describe the test in general words. For example : > # Index scans can use a different code path from the one sequential scans are > # following. Make sure that we cannot access other sessions' temp tables during > # index scan either. > > > Thank you for the comments! Please, see an updated set of patches. > I tested the v16 patch on a clean tree and verified the behavior across multiple execution paths. Same-session access to temporary tables works as expected. Cross-session access is consistently blocked; all attempts result in an error - "relation does not exist" and no incorrect or empty result sets were observed. In many cases, access is blocked earlier with “relation does not exist”, while the patch ensures that deeper execution paths are also protected. Verified both sequential and index scan paths (after disabling seqscan), and access is correctly rejected in all the cases. Tested various query forms including SELECT, COUNT(*), JOINs, subqueries, DML operations, and EXPLAIN ANALYZE none allowed access to other sessions’ temporary tables. pg_relation_size() returns metadata as expected and does not expose incorrect data. Please let me know if there are additional scenarios I should validate. Looking forward to more feedback. Regards, Soumya ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-13 13:56 Jim Jones <[email protected]> parent: Soumya S Murali <[email protected]> 1 sibling, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-04-13 13:56 UTC (permalink / raw) To: Soumya S Murali <[email protected]>; Daniil Davydov <[email protected]>; +Cc: Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi On 13/04/2026 14:40, Soumya S Murali wrote: > Please let me know if there are additional scenarios I should > validate. Looking forward to more feedback. Thanks for testing it. You can take a look at 012_temp_obj_multisession.pl and check if we missed any path. Due to changes introduced in b2a17ba7a5d the patch was no longer applying. See rebased v18 attached. Best, Jim Attachments: [text/x-patch] v18-0002-Test-cross-session-access-on-temporary-tables.patch (4.3K, 2-v18-0002-Test-cross-session-access-on-temporary-tables.patch) download | inline diff: From 88a3cd38d035075ef09c923ab3c7f358389e1022 Mon Sep 17 00:00:00 2001 From: Jim Jones <[email protected]> Date: Mon, 13 Apr 2026 15:28:35 +0200 Subject: [PATCH v18 2/2] Test cross-session access on temporary tables --- src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/012_temp_obj_multisession.pl | 109 ++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 src/test/modules/test_misc/t/012_temp_obj_multisession.pl diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 1b25d98f7f3..a54599cc301 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -20,6 +20,7 @@ tests += { 't/009_log_temp_files.pl', 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', + 't/012_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/012_temp_obj_multisession.pl b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl new file mode 100644 index 00000000000..fa8037e4d1e --- /dev/null +++ b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl @@ -0,0 +1,109 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +# Set up a fresh node +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Create a long-lived session +my $psql1 = $node->background_psql('postgres'); + +$psql1->query_safe(q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +$psql1->query_safe(q(CREATE INDEX ON foo(val);)); + +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + + +# SELECT TEMPORARY TABLE from other session +my ($stdout, $stderr); +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'SELECT on other session temp table is not allowed'); + +# UPDATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'UPDATE on other session temp table is not allowed'); + +# DELETE records from TEMPORARY TABLE from other session +$node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'DELETE on other session temp table is not allowed'); + +# TRUNCATE TEMPORARY TABLE from other session +$node->psql('postgres', "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr); +like( + $stderr, + qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE on other session temp table is not allowed'); + +# INSERT INTO TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'INSERT INTO on other session temp table is not allowed'); + +# COPY TEMPORARY TABLE from other session +$node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'COPY on other session temp table is blocked'); + +# Index scans can use a different code path from the one sequential scans are +# following. Make sure that we cannot access other sessions' temp tables during +# index scan either. +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary relations of other sessions/, + 'index scan on other session temp table is not allowed (exercises ReadBufferExtended path)' +); + +# DROP TEMPORARY TABLE from other session +$node->safe_psql('postgres', "DROP TABLE $tempschema.foo;"); + +# Clean up +$psql1->quit; + +done_testing(); -- 2.43.0 [text/x-patch] v18-0001-Prevent-access-to-other-sessions-temp-tables.patch (3.2K, 3-v18-0001-Prevent-access-to-other-sessions-temp-tables.patch) download | inline diff: From 635e924e26a772ddccaecc52cb994650f261c290 Mon Sep 17 00:00:00 2001 From: Jim Jones <[email protected]> Date: Mon, 13 Apr 2026 15:17:20 +0200 Subject: [PATCH v18 1/2] Prevent access to other sessions' temp tables --- src/backend/storage/aio/read_stream.c | 10 ++++++++++ src/backend/storage/buffer/bufmgr.c | 16 ++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 2374b4cd507..6ba6774af95 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. This * number also affects how far we look ahead for opportunities to start diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..70353f1f8e3 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -795,7 +795,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* pass it off to localbuf.c */ return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum); @@ -936,7 +936,7 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* * Read the buffer, and update pgstat counters to reflect a cache hit or @@ -1292,6 +1292,12 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, int flags; char persistence; + /* see comments in ReadBufferExtended */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + /* * Backward compatibility path, most code should use ExtendBufferedRel() * instead, as acquiring the extension lock inside ExtendBufferedRel() @@ -1382,6 +1388,12 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, Assert(*nblocks > 0); Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + /* see comments in ReadBufferExtended */ + if (operation->rel && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary relations of other sessions"))); + if (operation->persistence == RELPERSISTENCE_TEMP) { io_context = IOCONTEXT_NORMAL; -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-13 14:18 Daniil Davydov <[email protected]> parent: Soumya S Murali <[email protected]> 1 sibling, 0 replies; 52+ messages in thread From: Daniil Davydov @ 2026-04-13 14:18 UTC (permalink / raw) To: Soumya S Murali <[email protected]>; +Cc: Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Mon, Apr 13, 2026 at 7:39 PM Soumya S Murali <[email protected]> wrote: > > > I tested the v16 patch on a clean tree and verified the behavior > across multiple execution paths. Thanks! > Cross-session access is consistently > blocked; all attempts result in an error - "relation does not exist" Hm, our patch should give an error like "cannot access temporary relations of other sessions". If you see "relation does not exist" I guess you try to access other session's temp table only by its name. It will not work since postgres will search this name within the "pg_temp" schema of the current session. You should explicitly specify other session's temp schema name in order to access its temp tables. You can find examples in our new test. Please, let me know if I am wrong and the current patch actually allows "relation does not exist" error to occur. In this case, it should be fixed. -- Best regards, Daniil Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-16 05:56 Soumya S Murali <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 0 replies; 52+ messages in thread From: Soumya S Murali @ 2026-04-16 05:56 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi all, Thank you for the clarification and the updated patch. On Fri, Apr 10, 2026 at 8:58 PM Daniil Davydov <[email protected]> wrote: > > Hi, > > On Fri, Apr 10, 2026 at 5:29 PM Jim Jones <[email protected]> wrote: > > > > > BTW, what do you think about making this comment less "concrete"? : > > > # SELECT via index scan from other session. > > > # Sequential scans are blocked at read_stream_begin_relation(); index scans > > > # bypass that path entirely and reach ReadBufferExtended() in bufmgr.c > > > # (nbtree's _bt_getbuf calls ReadBuffer directly for individual page fetches). > > > # enable_seqscan=off forces the planner to use the index. > > > > > > I mean that if the described logic changes, this comment will become confusing. > > > We can describe the test in general words. For example : > > > # Index scans can use a different code path from the one sequential scans are > > > # following. Make sure that we cannot access other sessions' temp tables during > > > # index scan either. > > > > +1 > > > > Yeah, it's indeed too verbose. I guess these comments were originally > > just for me so I wouldn't get too confused along the way :) > > OK :) > > > > > I don't have anything else to add at this point. Unless there are any > > objections, I'll mark the CF entry as 'Ready for Committer.' > > > > Great, thank you! > > Please, see an updated set of patches (only perl test has been changed) : > 1) Rephrase the discussed comment. > 2) Use safe_psql whenever possible. > 3) Run pgperltidy. You were right. In my earlier testing I was not using the explicit temporary schema, which resulted in “relation does not exist” due to namespace resolution. I have now re-tested using the correct temporary schema of the owning session, and I can confirm that the patch behaves as expected. Cross-session access consistently throws: ERROR: cannot access temporary relations of other sessions Verified across multiple execution paths including SELECT, COUNT(*), JOINs, subqueries, and DML operations. Index scan paths (with seqscan disabled) are also correctly blocked with ERROR: cannot access temporary relations of other sessions. Same-session access continues to work as expected. Metadata access (pg_relation_size) behaves correctly and does not expose incorrect data. Cases where “relation does not exist” appears are due to referencing an incorrect temp schema, which is expected. Overall, the patch correctly prevents access across all tested paths. Thank you for pointing this out. Regards, Soumya ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-16 06:41 Soumya S Murali <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Soumya S Murali @ 2026-04-16 06:41 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi all, Thank you for the guidance and the updated patch. On Mon, Apr 13, 2026 at 7:26 PM Jim Jones <[email protected]> wrote: > > Hi > > On 13/04/2026 14:40, Soumya S Murali wrote: > > Please let me know if there are additional scenarios I should > > validate. Looking forward to more feedback. > > Thanks for testing it. You can take a look at > 012_temp_obj_multisession.pl and check if we missed any path. > > Due to changes introduced in b2a17ba7a5d the patch was no longer > applying. See rebased v18 attached. > I tested the rebased v18 patch on a clean tree and verified that it applies cleanly and behaves consistently with previous results. Cross-session access is correctly blocked with: ERROR: cannot access temporary relations of other sessions Index scan paths are also properly restricted, and same-session access continues to work as expected. The updated test changes look good. Everything works as expected, +1 from my side. Regards, Soumya ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-20 20:07 Alexander Korotkov <[email protected]> parent: Soumya S Murali <[email protected]> 0 siblings, 2 replies; 52+ messages in thread From: Alexander Korotkov @ 2026-04-20 20:07 UTC (permalink / raw) To: Soumya S Murali <[email protected]>; +Cc: Jim Jones <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi! I've checked the thread. Thanks to all the participants for their work. I think there is a general agreement on the design. On Thu, Apr 16, 2026 at 9:41 AM Soumya S Murali <[email protected]> wrote: > Thank you for the guidance and the updated patch. > > On Mon, Apr 13, 2026 at 7:26 PM Jim Jones <[email protected]> wrote: > > > > Hi > > > > On 13/04/2026 14:40, Soumya S Murali wrote: > > > Please let me know if there are additional scenarios I should > > > validate. Looking forward to more feedback. > > > > Thanks for testing it. You can take a look at > > 012_temp_obj_multisession.pl and check if we missed any path. > > > > Due to changes introduced in b2a17ba7a5d the patch was no longer > > applying. See rebased v18 attached. > > > > > I tested the rebased v18 patch on a clean tree and verified that it > applies cleanly and behaves consistently with previous results. > Cross-session access is correctly blocked with: ERROR: cannot access > temporary relations of other sessions > Index scan paths are also properly restricted, and same-session access > continues to work as expected. > The updated test changes look good. Everything works as expected, +1 > from my side. I see the patch changes the error wording. Previously the error was "cannot access temporary tables of other sessions", but we change it to "cannot access temporary relation of other sessions". I see the intention here: we trigger an error while accessing some relation (not necessarily a table) then we should reflect this directly to the error message. However, old message is already here for quite a while and translated into many languages. Also, is old message incorrect? We trigger an error on buffer access. That is, we trigger an error only for relation with a storage: table, index, sequence or matview. Matview can't be temporary. Also, if you access an index with a query, that means you're querying its table. But sequence can be temporary and it can be not directly associated with a table. So, yes, new error message is more correct. But I would prefer to make it a separate patch, and replace all the occurrences including contrib. ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-21 05:45 Soumya S Murali <[email protected]> parent: Alexander Korotkov <[email protected]> 1 sibling, 1 reply; 52+ messages in thread From: Soumya S Murali @ 2026-04-21 05:45 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Jim Jones <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi all, Thank you for reviewing the patch and for the detailed explanation. On Tue, Apr 21, 2026 at 1:37 AM Alexander Korotkov <[email protected]> wrote: > > Hi! > > I've checked the thread. Thanks to all the participants for their > work. I think there is a general agreement on the design. > > On Thu, Apr 16, 2026 at 9:41 AM Soumya S Murali > <[email protected]> wrote: > > Thank you for the guidance and the updated patch. > > > > On Mon, Apr 13, 2026 at 7:26 PM Jim Jones <[email protected]> wrote: > > > > > > Hi > > > > > > On 13/04/2026 14:40, Soumya S Murali wrote: > > > > Please let me know if there are additional scenarios I should > > > > validate. Looking forward to more feedback. > > > > > > Thanks for testing it. You can take a look at > > > 012_temp_obj_multisession.pl and check if we missed any path. > > > > > > Due to changes introduced in b2a17ba7a5d the patch was no longer > > > applying. See rebased v18 attached. > > > > > > > > > I tested the rebased v18 patch on a clean tree and verified that it > > applies cleanly and behaves consistently with previous results. > > Cross-session access is correctly blocked with: ERROR: cannot access > > temporary relations of other sessions > > Index scan paths are also properly restricted, and same-session access > > continues to work as expected. > > The updated test changes look good. Everything works as expected, +1 > > from my side. > > I see the patch changes the error wording. Previously the error was > "cannot access temporary tables of other sessions", but we change it > to "cannot access temporary relation of other sessions". I see the > intention here: we trigger an error while accessing some relation (not > necessarily a table) then we should reflect this directly to the error > message. However, old message is already here for quite a while and > translated into many languages. Also, is old message incorrect? We > trigger an error on buffer access. That is, we trigger an error only > for relation with a storage: table, index, sequence or matview. > Matview can't be temporary. Also, if you access an index with a > query, that means you're querying its table. But sequence can be > temporary and it can be not directly associated with a table. So, > yes, new error message is more correct. But I would prefer to make it > a separate patch, and replace all the occurrences including contrib. > This makes sense. While the new wording is indeed more precise, I agree that changing an existing error message, especially one that has been present for a long time and is already translated, should be handled separately from the bug fix. Keeping the current message for this patch and addressing wording improvements in a dedicated follow-up patch sounds like the right approach. Thanks for pointing this out. Regards, Soumya ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-21 10:54 Alexander Korotkov <[email protected]> parent: Soumya S Murali <[email protected]> 0 siblings, 2 replies; 52+ messages in thread From: Alexander Korotkov @ 2026-04-21 10:54 UTC (permalink / raw) To: Soumya S Murali <[email protected]>; +Cc: Jim Jones <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers On Tue, Apr 21, 2026 at 8:44 AM Soumya S Murali <[email protected]> wrote: > Thank you for reviewing the patch and for the detailed explanation. > > On Tue, Apr 21, 2026 at 1:37 AM Alexander Korotkov <[email protected]> wrote: > > > > Hi! > > > > I've checked the thread. Thanks to all the participants for their > > work. I think there is a general agreement on the design. > > > > On Thu, Apr 16, 2026 at 9:41 AM Soumya S Murali > > <[email protected]> wrote: > > > Thank you for the guidance and the updated patch. > > > > > > On Mon, Apr 13, 2026 at 7:26 PM Jim Jones <[email protected]> wrote: > > > > > > > > Hi > > > > > > > > On 13/04/2026 14:40, Soumya S Murali wrote: > > > > > Please let me know if there are additional scenarios I should > > > > > validate. Looking forward to more feedback. > > > > > > > > Thanks for testing it. You can take a look at > > > > 012_temp_obj_multisession.pl and check if we missed any path. > > > > > > > > Due to changes introduced in b2a17ba7a5d the patch was no longer > > > > applying. See rebased v18 attached. > > > > > > > > > > > > > I tested the rebased v18 patch on a clean tree and verified that it > > > applies cleanly and behaves consistently with previous results. > > > Cross-session access is correctly blocked with: ERROR: cannot access > > > temporary relations of other sessions > > > Index scan paths are also properly restricted, and same-session access > > > continues to work as expected. > > > The updated test changes look good. Everything works as expected, +1 > > > from my side. > > > > I see the patch changes the error wording. Previously the error was > > "cannot access temporary tables of other sessions", but we change it > > to "cannot access temporary relation of other sessions". I see the > > intention here: we trigger an error while accessing some relation (not > > necessarily a table) then we should reflect this directly to the error > > message. However, old message is already here for quite a while and > > translated into many languages. Also, is old message incorrect? We > > trigger an error on buffer access. That is, we trigger an error only > > for relation with a storage: table, index, sequence or matview. > > Matview can't be temporary. Also, if you access an index with a > > query, that means you're querying its table. But sequence can be > > temporary and it can be not directly associated with a table. So, > > yes, new error message is more correct. But I would prefer to make it > > a separate patch, and replace all the occurrences including contrib. > > > > > This makes sense. While the new wording is indeed more precise, I > agree that changing an existing error message, especially one that has > been present for a long time and is already translated, should be > handled > separately from the bug fix. Keeping the current message for this > patch and addressing wording improvements in a dedicated follow-up > patch sounds like the right approach. > Thanks for pointing this out. OK. I'm going to push and backpatch if no objections. ------ Regards, Alexander Korotkov Supabase Attachments: [application/octet-stream] v19-0001-Prevent-access-to-other-sessions-temp-tables.patch (7.9K, 2-v19-0001-Prevent-access-to-other-sessions-temp-tables.patch) download | inline diff: From d5395013b151ce8494e4f2086f4f87f0d04a6a0d Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Tue, 21 Apr 2026 13:17:32 +0300 Subject: [PATCH v19] Prevent access to other sessions' temp tables Commit b7b0f3f2724 ("Use streaming I/O in sequential scans") routed sequential scans through read_stream_next_buffer(), bypassing the RELATION_IS_OTHER_TEMP() check in ReadBufferExtended(). As a result, a superuser can attempt to access temp tables of other sessions, even though the buffer manager is incapable of dealing with them correctly. Fix by adding the same check at two additional points: - read_stream_begin_impl() covers sequential and bitmap scans that go through the read-stream path. - StartReadBuffersImpl() and ReadBuffer_common() cover index scans and any remaining direct buffer accesses. Also add a TAP test in src/test/modules/test_misc exercising access to another session's temp table. Author: Jim Jones <[email protected]> Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com Author: Daniil Davydov <[email protected]> Co-authored-by: Jim Jones <[email protected]> Reviewed-by: Soumya S Murali <[email protected]> Reviewed-by: Tom Lane <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> Backpatch-through: 17 --- src/backend/storage/aio/read_stream.c | 10 ++ src/backend/storage/buffer/bufmgr.c | 12 ++ src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/012_temp_obj_multisession.pl | 117 ++++++++++++++++++ 4 files changed, 140 insertions(+) create mode 100644 src/test/modules/test_misc/t/012_temp_obj_multisession.pl diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 2374b4cd507..a318539e56c 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. This * number also affects how far we look ahead for opportunities to start diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..676d8000e1e 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1292,6 +1292,12 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, int flags; char persistence; + /* see comments in ReadBufferExtended */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Backward compatibility path, most code should use ExtendBufferedRel() * instead, as acquiring the extension lock inside ExtendBufferedRel() @@ -1382,6 +1388,12 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, Assert(*nblocks > 0); Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + /* see comments in ReadBufferExtended */ + if (operation->rel && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + if (operation->persistence == RELPERSISTENCE_TEMP) { io_context = IOCONTEXT_NORMAL; diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 1b25d98f7f3..a54599cc301 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -20,6 +20,7 @@ tests += { 't/009_log_temp_files.pl', 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', + 't/012_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/012_temp_obj_multisession.pl b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl new file mode 100644 index 00000000000..43cfe625738 --- /dev/null +++ b/src/test/modules/test_misc/t/012_temp_obj_multisession.pl @@ -0,0 +1,117 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Verify that one session cannot access another session's temporary table. +# +# A session creates a temporary table and a separate session attempts to +# read and modify it through various code paths: SELECT, UPDATE, DELETE, +# TRUNCATE, INSERT, COPY, and an index scan. Each attempt is expected to +# fail with "cannot access temporary tables of other sessions" (or the +# analogous TRUNCATE error). + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +# Set up a fresh node +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Create a long-lived session +my $psql1 = $node->background_psql('postgres'); + +$psql1->query_safe(q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +$psql1->query_safe(q(CREATE INDEX ON foo(val);)); + +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + + +# SELECT TEMPORARY TABLE from other session +my ($stdout, $stderr); +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'SELECT on other session temp table is not allowed'); + +# UPDATE TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'UPDATE on other session temp table is not allowed'); + +# DELETE records from TEMPORARY TABLE from other session +$node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'DELETE on other session temp table is not allowed'); + +# TRUNCATE TEMPORARY TABLE from other session +$node->psql('postgres', "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr); +like( + $stderr, + qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE on other session temp table is not allowed'); + +# INSERT INTO TEMPORARY TABLE from other session +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'INSERT INTO on other session temp table is not allowed'); + +# COPY TEMPORARY TABLE from other session +$node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'COPY on other session temp table is blocked'); + +# Index scans can use a different code path from the one sequential scans are +# following. Make sure that we cannot access other sessions' temp tables during +# index scan either. +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'index scan on other session temp table is not allowed (exercises ReadBufferExtended path)' +); + +# DROP TEMPORARY TABLE from other session +$node->safe_psql('postgres', "DROP TABLE $tempschema.foo;"); + +# Clean up +$psql1->quit; + +done_testing(); -- 2.39.5 (Apple Git-154) ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-21 12:52 Jim Jones <[email protected]> parent: Alexander Korotkov <[email protected]> 1 sibling, 0 replies; 52+ messages in thread From: Jim Jones @ 2026-04-21 12:52 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; Soumya S Murali <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers On 21/04/2026 12:54, Alexander Korotkov wrote: > OK. I'm going to push and backpatch if no objections. +1 Thanks! Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-21 13:17 Daniil Davydov <[email protected]> parent: Alexander Korotkov <[email protected]> 1 sibling, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-04-21 13:17 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers Hi, On Tue, Apr 21, 2026 at 3:07 AM Alexander Korotkov <[email protected]> wrote: > > I've checked the thread. Thanks to all the participants for their > work. I think there is a general agreement on the design. > > I see the patch changes the error wording. Previously the error was > "cannot access temporary tables of other sessions", but we change it > to "cannot access temporary relation of other sessions". I see the > intention here: we trigger an error while accessing some relation (not > necessarily a table) then we should reflect this directly to the error > message. However, old message is already here for quite a while and > translated into many languages. Also, is old message incorrect? We > trigger an error on buffer access. That is, we trigger an error only > for relation with a storage: table, index, sequence or matview. > Matview can't be temporary. Also, if you access an index with a > query, that means you're querying its table. But sequence can be > temporary and it can be not directly associated with a table. So, > yes, new error message is more correct. Thank you very much for the review! > But I would prefer to make it > a separate patch, and replace all the occurrences including contrib. OK, no problem. BTW, do I understand correctly that I don't need to touch the .po files? I have also noticed this code in the localbuf.c : ``` if (IsParallelWorker()) ereport(ERROR, (errcode(ERRCODE_INVALID_TRANSACTION_STATE), errmsg("cannot access temporary tables during a parallel operation"))); ``` This code is related to initialization of local buffers which can be performed not only for tables, obviously. Since this is a small step away from our original idea, I'll fix it in a separate patch. IMHO it will be an appropriate fix since we have already taken on this task. Looking forward to your comments. -- Best regards, Daniil Davydov Attachments: [text/x-patch] v1-0001-Improve-error-message-for-other-sessions-temporar.patch (6.2K, 2-v1-0001-Improve-error-message-for-other-sessions-temporar.patch) download | inline diff: From a003429c8a1049955462ab50df4dba47b62320e6 Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Tue, 21 Apr 2026 03:43:40 +0700 Subject: [PATCH v1 1/2] Improve error message for other sessions temporary relation access Replace the word "table" with "relation", which is more correct term in postgres as it covers not only the tables, but also indexes, sequences, etc. --- contrib/amcheck/verify_common.c | 2 +- contrib/pageinspect/btreefuncs.c | 4 ++-- contrib/pageinspect/hashfuncs.c | 2 +- contrib/pageinspect/rawpage.c | 2 +- contrib/pgstattuple/pgstatapprox.c | 2 +- contrib/pgstattuple/pgstatindex.c | 2 +- contrib/pgstattuple/pgstattuple.c | 2 +- src/backend/storage/buffer/bufmgr.c | 4 ++-- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/contrib/amcheck/verify_common.c b/contrib/amcheck/verify_common.c index 54ce901716b..b680e0f8af5 100644 --- a/contrib/amcheck/verify_common.c +++ b/contrib/amcheck/verify_common.c @@ -176,7 +176,7 @@ index_checkable(Relation rel, Oid am_id) if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"), + errmsg("cannot access temporary relations of other sessions"), errdetail("Index \"%s\" is associated with temporary relation.", RelationGetRelationName(rel)))); diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index 0585b7cee40..62836ff8a1d 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -239,7 +239,7 @@ bt_index_block_validate(Relation rel, int64 blkno) if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); if (blkno == 0) ereport(ERROR, @@ -872,7 +872,7 @@ bt_metap(PG_FUNCTION_ARGS) if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); buffer = ReadBuffer(rel, 0); LockBuffer(buffer, BUFFER_LOCK_SHARE); diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index 7fc97d043ce..52f3062f1ca 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -430,7 +430,7 @@ hash_bitmap_info(PG_FUNCTION_ARGS) if (RELATION_IS_OTHER_TEMP(indexRel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); if (ovflblkno < 0 || ovflblkno > MaxBlockNumber) ereport(ERROR, diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index f3996dc39fc..cde33c9e792 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -173,7 +173,7 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno) if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); if (blkno >= RelationGetNumberOfBlocksInFork(rel, forknum)) ereport(ERROR, diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c index 21e0b50fb4b..a759aae6312 100644 --- a/contrib/pgstattuple/pgstatapprox.c +++ b/contrib/pgstattuple/pgstatapprox.c @@ -330,7 +330,7 @@ pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo) if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* * We support only relation kinds with a visibility map and a free space diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index 3a3f2637bd9..fe6d970c064 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -236,7 +236,7 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo) if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* * A !indisready index could lead to ERRCODE_DATA_CORRUPTED later, so exit diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c index 6a7f8cb4a7c..b2716173b01 100644 --- a/contrib/pgstattuple/pgstattuple.c +++ b/contrib/pgstattuple/pgstattuple.c @@ -252,7 +252,7 @@ pgstat_relation(Relation rel, FunctionCallInfo fcinfo) if (RELATION_IS_OTHER_TEMP(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); if (RELKIND_HAS_TABLE_AM(rel->rd_rel->relkind) || rel->rd_rel->relkind == RELKIND_SEQUENCE) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..5d282cbf449 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -795,7 +795,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* pass it off to localbuf.c */ return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum); @@ -936,7 +936,7 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); + errmsg("cannot access temporary relations of other sessions"))); /* * Read the buffer, and update pgstat counters to reflect a cache hit or -- 2.43.0 [text/x-patch] v1-0002-Improve-error-message-for-accessing-temp-buffers-.patch (1.6K, 3-v1-0002-Improve-error-message-for-accessing-temp-buffers-.patch) download | inline diff: From 86486e1a942c68c528205037c8244ef9046a8b83 Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Tue, 21 Apr 2026 20:03:52 +0700 Subject: [PATCH v1 2/2] Improve error message for accessing temp buffers in parallel worker Replace the word "table" with "relaton" as we do in the previous patch, since temp buffers initialization may be caused by accessing any type of object (table, index, etc.) --- src/backend/storage/buffer/localbuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index 396da84b25c..4572511c044 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -748,7 +748,7 @@ InitLocalBuffers(void) int i; /* - * Parallel workers can't access data in temporary tables, because they + * Parallel workers can't access data in temporary relations, because they * have no visibility into the local buffers of their leader. This is a * convenient, low-cost place to provide a backstop check for that. Note * that we don't wish to prevent a parallel worker from accessing catalog @@ -758,7 +758,7 @@ InitLocalBuffers(void) if (IsParallelWorker()) ereport(ERROR, (errcode(ERRCODE_INVALID_TRANSACTION_STATE), - errmsg("cannot access temporary tables during a parallel operation"))); + errmsg("cannot access temporary relations during a parallel operation"))); /* Allocate and zero buffer headers and auxiliary arrays */ LocalBufferDescriptors = (BufferDesc *) calloc(nbufs, sizeof(BufferDesc)); -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-21 23:41 Michael Paquier <[email protected]> parent: Alexander Korotkov <[email protected]> 1 sibling, 3 replies; 52+ messages in thread From: Michael Paquier @ 2026-04-21 23:41 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Jim Jones <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Tue, Apr 21, 2026 at 01:54:47PM +0300, Alexander Korotkov wrote: > OK. I'm going to push and backpatch if no objections. Timing is interesting here. Last week I have been doing a on-site patch review with a couple of colleagues and myself, and this thread has been chosen as part of this exercise. I am attaching them in CC of this thread, and replying to this thread was an action item I had to act on. Here is some feedback, based on v18 posted here: https://www.postgresql.org/message-id/[email protected] We have found that the thread does not state clearly what it aims to fix. The subject states that it is a bug, perhaps it is but the thread does not seem to do a good job in explaining why the current superuser behavior is bad, while the behavior of the patch to block some commands is better. This should be clearer in explaining why this new behavior is better. The test script is under-documented. There are zero comments that actually explain why the patch does what it does. The few comments present could be removed: they are copy-pasted of the descriptions of the test cases. A much worse thing is this thing: +# DROP TEMPORARY TABLE from other session +$node->safe_psql('postgres', "DROP TABLE $tempschema.foo;"); DROP TABLE on a temporary relation for a superuser is a very useful behavior to unstuck autovacuum if a temp relation has been orphaned. It looks critical to me to explain that we want to keep this behavior for this reason. Using safe_psql() is not really adapted. You should use a psql() where we check that the command does not fail, so as the test can generate a full report. The test coverage actually has holes. The three DML patterns INSERT, UPDATE and DELETE are covered, and it is missing MERGE. Also, what about other patterns like ALTER TABLE, ALTER INDEX, ALTER FUNCTION, DROP FUNCTION and the like? There are many object patterns that can be schema-qualified, and none of this is covered. What about the new REPACK and a bunch of other maintenance commands? There is nothing about VACUUM, TRUNCATE, CLUSTER, etc. Just to name a few. Another question is how do we make sure that new command patterns follow the same rule as what is enforced here. It looks like this should be at least documented somewhere to be less surprising, but the patch does nothing of the kind. As a whole the patch lacks documentation, comments, and explanations, making it difficult to act on. As a whole, we were not really convinced that this is something that needs any kind of specific fix, especially not something that should be backpatched. After saying all that, there is some value in what you are doing here: it is true that we lack test coverage in terms of interactions of temporary objects across multiple sessions, and that we should have some. TAP is adapted for this purpose, isolation tests could be an extra one but the schema names make that unpredictible in output. The patch unfortunately does a poor job in showing what it wants to change. One thing that I would suggest is to *reverse* the order of the patches: - First have a patch that introduces new tests, that shows the original behavior. This needs to be more complete in terms of command patterns. The DROP TABLE is one case that we want to keep. This should be kept as-is, and it is critical to document the reason why we want to keep things this way (aka autovacuum and orphaned tables, AFAIK). - Then implement the second patch that updates the tests introduced in the first patch, so as one can track *what* has changed, and so as one does not have to test manually what the original behavior was. As a whole, this patch needs more work, based on what has been currently posted on the lists. That's not ready yet. The backpatch question is a separate matter. Thanks, -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-24 22:09 Jim Jones <[email protected]> parent: Michael Paquier <[email protected]> 2 siblings, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-04-24 22:09 UTC (permalink / raw) To: Michael Paquier <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi Michael, Thanks for the review. On 22/04/2026 01:41, Michael Paquier wrote: > As a whole, we were not really convinced that this is something that > needs any kind of specific fix, especially not something that should > be backpatched. This surprises me a little bit. Until now I was under the impression that fixing it was the consensus here. Just the "how" was being discussed. > After saying all that, there is some value in what you are doing here: > it is true that we lack test coverage in terms of interactions of > temporary objects across multiple sessions, and that we should have > some. TAP is adapted for this purpose, isolation tests could be an > extra one but the schema names make that unpredictible in output. The > patch unfortunately does a poor job in showing what it wants to > change. One thing that I would suggest is to *reverse* the order of > the patches: > - First have a patch that introduces new tests, that shows the > original behavior. This needs to be more complete in terms of command > patterns. The DROP TABLE is one case that we want to keep. This > should be kept as-is, and it is critical to document the reason why we > want to keep things this way (aka autovacuum and orphaned tables, > AFAIK). > - Then implement the second patch that updates the tests introduced in > the first patch, so as one can track *what* has changed, and so as one > does not have to test manually what the original behavior was. Fair point on tests comments and coverage. But before we start with the refactoring, I'd like to make sure I understood your suggestion correctly. You're suggesting: 0001 - TAP tests with improved coverage and comments that pass on current master, documenting the existing behaviour, which means broken commands silently succeed (e.g. SELECT returns 0 rows, no error) 0002 - read_stream.c and bufmgr.c fix + updated test expectations (the same commands now raise errors) Is it what you had in mind? Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-24 22:30 David G. Johnston <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: David G. Johnston @ 2026-04-24 22:30 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alexander Korotkov <[email protected]>; Soumya S Murali <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Fri, Apr 24, 2026 at 3:09 PM Jim Jones <[email protected]> wrote: > > 0001 - TAP tests with improved coverage and comments that pass on > current master, documenting the existing behaviour, which means broken > commands silently succeed (e.g. SELECT returns 0 rows, no error) > 0002 - read_stream.c and bufmgr.c fix + updated test expectations (the > same commands now raise errors) > If you can run the tests against v17 (that is the behavior we are trying to restore here, correct?) and v18 that would help demonstrate why the backpatch is needed. David J. ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-24 23:01 Jim Jones <[email protected]> parent: David G. Johnston <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-04-24 23:01 UTC (permalink / raw) To: David G. Johnston <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alexander Korotkov <[email protected]>; Soumya S Murali <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi David On 25/04/2026 00:30, David G. Johnston wrote: > If you can run the tests against v17 (that is the behavior we are trying > to restore here, correct?) and v18 that would help demonstrate why the > backpatch is needed. Tests for PG18 and PG17: == PG 18 == psql (18.3 (Debian 18.3-1.pgdg13+1)) Type "help" for help. postgres=# \d pg_temp*.* Table "pg_temp_36.t" Column | Type | Collation | Nullable | Default -----------------+---------+-----------+----------+--------- generate_series | integer | | | postgres=# SELECT * FROM pg_temp_36.t; generate_series ----------------- (0 rows) == PG 17 == psql (17.7 (Debian 17.7-3.pgdg13+1)) Geben Sie »help« für Hilfe ein. postgres=# \d pg_temp*.* Tabelle »pg_temp_13.t« Spalte | Typ | Sortierfolge | NULL erlaubt? | Vorgabewert -----------------+---------+--------------+---------------+------------- generate_series | integer | | | postgres=# SELECT * FROM pg_temp_13.t; generate_series ----------------- (0 Zeilen) Until PG16 an error message was raised: psql (16.13 (Debian 16.13-1.pgdg13+1)) Type "help" for help. postgres=# \d pg_temp*.* Table "pg_temp_3.t" Column | Type | Collation | Nullable | Default -----------------+---------+-----------+----------+--------- generate_series | integer | | | postgres=# SELECT * FROM pg_temp_3.t; ERROR: cannot access temporary tables of other sessions Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-25 08:34 Daniil Davydov <[email protected]> parent: Michael Paquier <[email protected]> 2 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-04-25 08:34 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Soumya S Murali <[email protected]>; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, On Wed, Apr 22, 2026 at 6:41 AM Michael Paquier <[email protected]> wrote: > > On Tue, Apr 21, 2026 at 01:54:47PM +0300, Alexander Korotkov wrote: > > OK. I'm going to push and backpatch if no objections. > > Timing is interesting here. Last week I have been doing a on-site > patch review with a couple of colleagues and myself, and this thread > has been chosen as part of this exercise. I am attaching them in CC > of this thread, and replying to this thread was an action item I had > to act on. > > Here is some feedback, based on v18 posted here: > https://www.postgresql.org/message-id/[email protected] > Thank you for looking into this! > We have found that the thread does not state clearly what it aims to > fix. The subject states that it is a bug, perhaps it is but the > thread does not seem to do a good job in explaining why the current > superuser behavior is bad, while the behavior of the patch to block > some commands is better. This should be clearer in explaining why > this new behavior is better. This patch doesn't provide any new behavior. It returns consistent behavior, which we have lost after the appearance of streaming I/O in sequential scans. Jim wrote about it here [1]. You can also find this in the commit message of the v19 patch. IMHO, both thread and attached patch do the job of explaining why this fix is necessary. But I think I understand what confused you. Please, see my comments below. > The test script is under-documented. There are zero comments that > actually explain why the patch does what it does. The few comments > present could be removed: they are copy-pasted of the descriptions of > the test cases. A much worse thing is this thing: > > +# DROP TEMPORARY TABLE from other session > +$node->safe_psql('postgres', "DROP TABLE $tempschema.foo;"); Yeah, I agree. It can be (and will be) improved. > DROP TABLE on a temporary relation for a superuser is a very useful > behavior to unstuck autovacuum if a temp relation has been orphaned. > It looks critical to me to explain that we want to keep this behavior > for this reason. Do you mean that we should add something to the documentation? Again, postgres has always allowed DROP TABLE operation to be done for other temp tables, if the user has enough privileges. We are disallowing everyone to look at other temp tables' pages because there is no capability to do it. DROP TABLE doesn't require access to backend-private data, so it is OK. Tom Lane wrote about it here [2]. I want to say that described behavior is pretty natural. Why should we additionally describe that the user can do operations that he is allowed to do? On the other hand, we have an error message that says "cannot access...", which may look like every kind of "access" is forbidden. I bet that this is the place that has confused you. More accurate error message would be "cannot access pages..." or "cannot access content...". I think we can change our error message in this way. What do you think? We can also explicitly write in the documentation that users cannot access *pages/content* of other temp tables. But the original patch [3] didn't do it, so I am not sure that we should either. > Using safe_psql() is not really adapted. You should > use a psql() where we check that the command does not fail, so as the > test can generate a full report. Fair enough. > The test coverage actually has holes. The three DML patterns INSERT, > UPDATE and DELETE are covered, and it is missing MERGE. Also, what > about other patterns like ALTER TABLE, ALTER INDEX, ALTER FUNCTION, > DROP FUNCTION and the like? > There are many object patterns that can > be schema-qualified, and none of this is covered. What about the new > REPACK and a bunch of other maintenance commands? There is nothing > about VACUUM, TRUNCATE, CLUSTER, etc. Just to name a few. I have concerns about it. If we try the "brute force" approach (i.e. test all available commands), the test will increase unreasonably and will need constant support. I guess that covering all available code paths that lead to reading temp table's pages would be enough. > As a whole, we were not really convinced that this is something that > needs any kind of specific fix It is a bug, obviously. TBH, I don't see any reason for not fixing it. > especially not something that should > be backpatched. I couldn't answer for a long time, and Jim had already clearly demonstrated why the backpatch is needed (for which I am grateful). [1] https://www.postgresql.org/message-id/b13dc5ba-6c11-429c-b6fe-673c1c767bcf%40uni-muenster.de [2] https://www.postgresql.org/message-id/4075754.1774378690%40sss.pgh.pa.us [3] 948d6ec90fd35d6e1a59d0b1af8d6efd8442f0ad -- Best regards, Danill Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-25 13:53 Jim Jones <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 0 replies; 52+ messages in thread From: Jim Jones @ 2026-04-25 13:53 UTC (permalink / raw) To: David G. Johnston <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alexander Korotkov <[email protected]>; Soumya S Murali <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On 25/04/2026 01:01, Jim Jones wrote: > Until PG16 an error message was raised: > > psql (16.13 (Debian 16.13-1.pgdg13+1)) > Type "help" for help. > > postgres=# \d pg_temp*.* > Table "pg_temp_3.t" > Column | Type | Collation | Nullable | Default > -----------------+---------+-----------+----------+--------- > generate_series | integer | | | > > postgres=# SELECT * FROM pg_temp_3.t; > ERROR: cannot access temporary tables of other sessions The PG16 test above was against a non-empty TEMPORARY TABLE. If the table is empty, the same behaviour from PG17 and PG18 can be observed: psql (16.13 (Debian 16.13-1.pgdg13+1)) Type "help" for help. postgres=# \d pg_temp*.* Table "pg_temp_4.t" Column | Type | Collation | Nullable | Default --------+---------+-----------+----------+--------- id | integer | | | postgres=# SELECT * FROM pg_temp_4.t; id ---- (0 rows) The same applies for PG14 and PG15 psql (14.22 (Debian 14.22-1.pgdg13+1)) Type "help" for help. postgres=# \d pg_temp*.* Table "pg_temp_3.t" Column | Type | Collation | Nullable | Default --------+---------+-----------+----------+--------- id | integer | | | postgres=# SELECT * FROM pg_temp_3.t; id ---- (0 rows) psql (15.17 (Debian 15.17-1.pgdg13+1)) Type "help" for help. postgres=# \d pg_temp*.* Table "pg_temp_3.t" Column | Type | Collation | Nullable | Default --------+---------+-----------+----------+--------- id | integer | | | postgres=# SELECT * FROM pg_temp_3.t; id ---- (0 rows) Since the table is indeed empty, the result is actually correct. But I'd argue that we should raise an ERROR even if the table is empty. IMHO, getting an error message or a "0 rows" result depending on the table row count isn't ideal. After populating the TEMPORARY TABLE the expected error message appears: ERROR: cannot access temporary tables of other sessions Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-04-27 00:50 Michael Paquier <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 0 replies; 52+ messages in thread From: Michael Paquier @ 2026-04-27 00:50 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Soumya S Murali <[email protected]>; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers On Tue, Apr 21, 2026 at 08:17:15PM +0700, Daniil Davydov wrote: > BTW, do I understand correctly that I don't need to touch the .po files? The .po files in the tree are refreshed periodically based on the work done by pgsql-translators. There is no need to touch them while working on the in-core code. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-01 19:16 Jim Jones <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 0 replies; 52+ messages in thread From: Jim Jones @ 2026-05-01 19:16 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; Michael Paquier <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi On 25/04/2026 10:34, Daniil Davydov wrote: > On Wed, Apr 22, 2026 at 6:41 AM Michael Paquier <[email protected]> wrote: >> >> On Tue, Apr 21, 2026 at 01:54:47PM +0300, Alexander Korotkov wrote: >>> OK. I'm going to push and backpatch if no objections. >> >> As a whole, we were not really convinced that this is something that >> needs any kind of specific fix > > It is a bug, obviously. TBH, I don't see any reason for not fixing it. > >> especially not something that should >> be backpatched. I see there are differing opinions among committers on this. How should we proceed? Do we really consider the current behaviour acceptable, or should we invest effort in improving the tests (and potentially backpatch it)? Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-02 14:16 Alexander Korotkov <[email protected]> parent: Michael Paquier <[email protected]> 2 siblings, 1 reply; 52+ messages in thread From: Alexander Korotkov @ 2026-05-02 14:16 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Soumya S Murali <[email protected]>; Jim Jones <[email protected]>; Daniil Davydov <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, Michael! On Wed, Apr 22, 2026 at 2:41 AM Michael Paquier <[email protected]> wrote: > > On Tue, Apr 21, 2026 at 01:54:47PM +0300, Alexander Korotkov wrote: > > OK. I'm going to push and backpatch if no objections. > > Timing is interesting here. Last week I have been doing a on-site > patch review with a couple of colleagues and myself, and this thread > has been chosen as part of this exercise. I am attaching them in CC > of this thread, and replying to this thread was an action item I had > to act on. > > Here is some feedback, based on v18 posted here: > https://www.postgresql.org/message-id/[email protected] > > We have found that the thread does not state clearly what it aims to > fix. The subject states that it is a bug, perhaps it is but the > thread does not seem to do a good job in explaining why the current > superuser behavior is bad, while the behavior of the patch to block > some commands is better. This should be clearer in explaining why > this new behavior is better. > > The test script is under-documented. There are zero comments that > actually explain why the patch does what it does. The few comments > present could be removed: they are copy-pasted of the descriptions of > the test cases. A much worse thing is this thing: > > +# DROP TEMPORARY TABLE from other session > +$node->safe_psql('postgres', "DROP TABLE $tempschema.foo;"); > > DROP TABLE on a temporary relation for a superuser is a very useful > behavior to unstuck autovacuum if a temp relation has been orphaned. > It looks critical to me to explain that we want to keep this behavior > for this reason. Using safe_psql() is not really adapted. You should > use a psql() where we check that the command does not fail, so as the > test can generate a full report. > > The test coverage actually has holes. The three DML patterns INSERT, > UPDATE and DELETE are covered, and it is missing MERGE. Also, what > about other patterns like ALTER TABLE, ALTER INDEX, ALTER FUNCTION, > DROP FUNCTION and the like? There are many object patterns that can > be schema-qualified, and none of this is covered. What about the new > REPACK and a bunch of other maintenance commands? There is nothing > about VACUUM, TRUNCATE, CLUSTER, etc. Just to name a few. > > Another question is how do we make sure that new command patterns > follow the same rule as what is enforced here. It looks like this > should be at least documented somewhere to be less surprising, but the > patch does nothing of the kind. > > As a whole the patch lacks documentation, comments, and explanations, > making it difficult to act on. > > As a whole, we were not really convinced that this is something that > needs any kind of specific fix, especially not something that should > be backpatched. > > After saying all that, there is some value in what you are doing here: > it is true that we lack test coverage in terms of interactions of > temporary objects across multiple sessions, and that we should have > some. TAP is adapted for this purpose, isolation tests could be an > extra one but the schema names make that unpredictible in output. The > patch unfortunately does a poor job in showing what it wants to > change. One thing that I would suggest is to *reverse* the order of > the patches: > - First have a patch that introduces new tests, that shows the > original behavior. This needs to be more complete in terms of command > patterns. The DROP TABLE is one case that we want to keep. This > should be kept as-is, and it is critical to document the reason why we > want to keep things this way (aka autovacuum and orphaned tables, > AFAIK). > - Then implement the second patch that updates the tests introduced in > the first patch, so as one can track *what* has changed, and so as one > does not have to test manually what the original behavior was. > > As a whole, this patch needs more work, based on what has been > currently posted on the lists. That's not ready yet. The backpatch > question is a separate matter. Thank you for your feedback. I think the scope of this patch is well described in [1]. We don't want to restrict the superuser from something, but our buffer manager just technically can't access the local buffers of other sessions. Read streams introduced a new code path for reading relations, which was lacking of the proper check for local buffers of other sessions. And this patch attempts to fix that. DROP TABLE is an exclusion. It actually don't need to read contents of buffers, just drop them. And DropRelationBuffers() have a special exclusion for this case. So, DROP TABLE appears to be the only operation that makes sense, it's a conscious exclusion, and there is no intention to forbid it. I've revised the patch. 0001 contains tests and states the current behavior. 0002 contains fix and the corresponding changes in the tests. I made a change in 0001: removed the check in ReadBufferExtended(). We added the same check to ReadBuffer_common(), and I don't think it makes sense to do this check twice in the row. Links. 1. https://www.postgresql.org/message-id/3529398.1774273446%40sss.pgh.pa.us ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-02 15:37 Daniil Davydov <[email protected]> parent: Alexander Korotkov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-05-02 15:37 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, On Sat, May 2, 2026 at 9:16 PM Alexander Korotkov <[email protected]> wrote: > > Thank you for your feedback. I think the scope of this patch is well > described in [1]. We don't want to restrict the superuser from > something, but our buffer manager just technically can't access the > local buffers of other sessions. Read streams introduced a new code > path for reading relations, which was lacking of the proper check for > local buffers of other sessions. And this patch attempts to fix that. > DROP TABLE is an exclusion. It actually don't need to read contents > of buffers, just drop them. And DropRelationBuffers() have a special > exclusion for this case. So, DROP TABLE appears to be the only > operation that makes sense, it's a conscious exclusion, and there is > no intention to forbid it. Yep, exactly. > I've revised the patch. 0001 contains tests and states the current > behavior. 0002 contains fix and the corresponding changes in the > tests. I made a change in 0001: removed the check in > ReadBufferExtended(). We added the same check to ReadBuffer_common(), > and I don't think it makes sense to do this check twice in the row. Thank you! But I'm afraid that you forgot to attach the patches.. BTW, what do you think about this proposal? : > On the other hand, we have an error message that says "cannot access...", which > may look like every kind of "access" is forbidden. I bet that this is the place > that has confused you. More accurate error message would be "cannot access > pages..." or "cannot access content...". I think we can change our error > message in this way. What do you think? We can easily include it in the first patch. -- Best regards, Daniil Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-02 16:34 Alexander Korotkov <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Alexander Korotkov @ 2026-05-02 16:34 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Sat, May 2, 2026 at 6:37 PM Daniil Davydov <[email protected]> wrote: > On Sat, May 2, 2026 at 9:16 PM Alexander Korotkov <[email protected]> wrote: > > > > Thank you for your feedback. I think the scope of this patch is well > > described in [1]. We don't want to restrict the superuser from > > something, but our buffer manager just technically can't access the > > local buffers of other sessions. Read streams introduced a new code > > path for reading relations, which was lacking of the proper check for > > local buffers of other sessions. And this patch attempts to fix that. > > DROP TABLE is an exclusion. It actually don't need to read contents > > of buffers, just drop them. And DropRelationBuffers() have a special > > exclusion for this case. So, DROP TABLE appears to be the only > > operation that makes sense, it's a conscious exclusion, and there is > > no intention to forbid it. > > Yep, exactly. > > > I've revised the patch. 0001 contains tests and states the current > > behavior. 0002 contains fix and the corresponding changes in the > > tests. I made a change in 0001: removed the check in > > ReadBufferExtended(). We added the same check to ReadBuffer_common(), > > and I don't think it makes sense to do this check twice in the row. > > Thank you! But I'm afraid that you forgot to attach the patches.. Here they are. > BTW, what do you think about this proposal? : > > On the other hand, we have an error message that says "cannot access...", which > > may look like every kind of "access" is forbidden. I bet that this is the place > > that has confused you. More accurate error message would be "cannot access > > pages..." or "cannot access content...". I think we can change our error > > message in this way. What do you think? > > We can easily include it in the first patch. This is possible, but I would keep that in a separate patch. We now have clear scope for both patches: 0001 includes additional tests, 0002 fixes the bug and restores old behavior. ------ Regards, Alexander Korotkov Supabase Attachments: [application/octet-stream] v20-0002-Prevent-access-to-other-sessions-temp-tables.patch (8.8K, 2-v20-0002-Prevent-access-to-other-sessions-temp-tables.patch) download | inline diff: From 50d2b998cccd5207151fc0aad3a88d9acb19a53b Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Sat, 2 May 2026 15:23:42 +0300 Subject: [PATCH v20 2/2] Prevent access to other sessions' temp tables Commit b7b0f3f2724 ("Use streaming I/O in sequential scans") routed sequential scans through read_stream_next_buffer(), bypassing the RELATION_IS_OTHER_TEMP() check in ReadBufferExtended(). As a result, a superuser can attempt to read or modify temp tables of other sessions through the read-stream path. When no index is present so the planner cannot pick an index scan, SELECT/UPDATE/DELETE/MERGE silently see no rows / report zero affected rows, and COPY produces an empty output -- because the buffer manager has no visibility into the owning session's local buffers and silently returns nothing. INSERT and any path that goes through nbtree (i.e. an index scan) still error out via the existing check in ReadBufferExtended(), which is reached from hio.c and nbtree respectively, but this is incidental. Fix by enforcing RELATION_IS_OTHER_TEMP() at three additional buffer-manager entry points: - read_stream_begin_impl() rejects the read at stream setup time, covering sequential and bitmap scans that go through the read-stream path. - ReadBuffer_common() becomes the canonical place for the check, consolidating the existing one previously kept in ReadBufferExtended(). All ReadBufferExtended() callers go through ReadBuffer_common(), so the consolidation is behaviour-preserving. - StartReadBuffersImpl() catches direct callers of StartReadBuffers() that bypass both of the above. This is currently defense-in-depth but documents the contract for future code. The companion test in src/test/modules/test_misc was added in the preceding commit; this commit updates the assertions for SELECT, UPDATE, DELETE, MERGE and COPY (which previously documented the bug as silent success) to expect the new error. Author: Jim Jones <[email protected]> Author: Daniil Davydov <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Soumya S Murali <[email protected]> Reviewed-by: Tom Lane <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com Backpatch-through: 17 --- src/backend/storage/aio/read_stream.c | 10 ++++++ src/backend/storage/buffer/bufmgr.c | 33 ++++++++++------- .../test_misc/t/013_temp_obj_multisession.pl | 35 ++++++++++--------- 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 2374b4cd507..a318539e56c 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. This * number also affects how far we look ahead for opportunities to start diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 1878efb4aa9..770209d606c 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -791,7 +791,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RelationUsesLocalBuffers(reln)) { - /* see comments in ReadBufferExtended */ + /* see comments in ReadBuffer_common */ if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -928,19 +928,10 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, { Buffer buf; - /* - * Reject attempts to read non-local temporary relations; we would be - * likely to get wrong data since we have no visibility into the owning - * session's local buffers. - */ - if (RELATION_IS_OTHER_TEMP(reln)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); - /* * Read the buffer, and update pgstat counters to reflect a cache hit or - * miss. + * miss. The other-session temp-relation check is enforced by + * ReadBuffer_common(). */ buf = ReadBuffer_common(reln, RelationGetSmgr(reln), 0, forkNum, blockNum, mode, strategy); @@ -1292,6 +1283,18 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, int flags; char persistence; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. This is the canonical place for the check, + * covering the ReadBufferExtended() entry point and any other caller + * that supplies a Relation. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Backward compatibility path, most code should use ExtendBufferedRel() * instead, as acquiring the extension lock inside ExtendBufferedRel() @@ -1382,6 +1385,12 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, Assert(*nblocks > 0); Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + /* see comments in ReadBuffer_common */ + if (operation->rel && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + if (operation->persistence == RELPERSISTENCE_TEMP) { io_context = IOCONTEXT_NORMAL; diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl index 0d211700977..b4442836bef 100644 --- a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -55,25 +55,20 @@ my ($stdout, $stderr); # DML and SELECT have to read the table's data and therefore go through # the buffer manager. With no index on the table, the planner cannot # use index access, so SELECT/UPDATE/DELETE/MERGE/COPY all run through -# the read-stream path. -# -# XXX: in current code, the read-stream path bypasses the -# RELATION_IS_OTHER_TEMP() check, so these commands silently see no -# rows / report zero affected rows -- the visible symptom of the bug -# this test suite documents. A follow-up patch will route the check -# through read_stream_begin_impl() and these assertions will be -# updated to expect "cannot access temporary tables of other sessions". +# the read-stream path and are caught by read_stream_begin_impl(). $node->psql( 'postgres', "SELECT val FROM $tempschema.foo;", - stdout => \$stdout, stderr => \$stderr); -is($stderr, '', 'SELECT (currently no error -- bug to be fixed)'); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'SELECT (seqscan via read_stream)'); # INSERT goes through hio.c which calls ReadBufferExtended() to find a -# page with free space; that hits the existing check before any data is -# written. This case currently errors as expected. +# page with free space; that hits the existing check before any data +# is written. $node->psql( 'postgres', "INSERT INTO $tempschema.foo VALUES (73);", @@ -86,21 +81,29 @@ $node->psql( 'postgres', "UPDATE $tempschema.foo SET val = NULL;", stderr => \$stderr); -is($stderr, '', 'UPDATE (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'UPDATE'); $node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); -is($stderr, '', 'DELETE (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'DELETE'); $node->psql( 'postgres', "MERGE INTO $tempschema.foo USING (VALUES (42)) AS s(val) " . "ON foo.val = s.val WHEN MATCHED THEN DELETE;", stderr => \$stderr); -is($stderr, '', 'MERGE (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'MERGE'); $node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", stderr => \$stderr); -is($stderr, '', 'COPY (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'COPY'); # DDL and maintenance commands have their own command-specific checks # (older than the buffer-manager check above), so they fail with -- 2.39.5 (Apple Git-154) [application/octet-stream] v20-0001-Add-tests-for-cross-session-temp-table-access.patch (13.7K, 3-v20-0001-Add-tests-for-cross-session-temp-table-access.patch) download | inline diff: From de67da121b45e603c81739e273d7cb8cfc059fb1 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Sat, 2 May 2026 15:22:26 +0300 Subject: [PATCH v20 1/2] Add tests for cross-session temp table access Add a TAP test in src/test/modules/test_misc that documents what happens when one session attempts to read or modify another session's temporary table. This commit only adds tests; it does not change backend behaviour, so the assertions reflect current behaviour: - SELECT, UPDATE, DELETE, MERGE, COPY on a table without an index silently succeed with no error and zero rows / zero affected rows. These commands run through the read-stream path, which currently bypasses the RELATION_IS_OTHER_TEMP() check. This is the underlying bug to be fixed in a follow-up. - INSERT errors with "cannot access temporary tables of other sessions" because hio.c calls ReadBufferExtended() to find a page with free space and is caught by the existing check there. - Index scan errors via the same existing check, reached through nbtree -> ReadBuffer -> ReadBufferExtended. - TRUNCATE / ALTER TABLE / ALTER INDEX / CLUSTER fail with their command-specific error messages. - VACUUM is silently skipped to avoid noise during database-wide VACUUM (vacuum_rel() returns without warning). - DROP TABLE is intentionally allowed: DROP does not touch the table's contents, and autovacuum relies on this to clean up temp relations orphaned by a crashed backend. - ALTER FUNCTION / DROP FUNCTION on an owner-created function over its own temp row type work as catalog operations -- they don't read the underlying data. - CREATE FUNCTION from a separate session, using another session's temp row type as an argument, is allowed but emits a NOTICE: the function is moved into the creator's pg_temp namespace with an auto-dependency on the borrowed type, so it disappears together with the session that created it. - A bare DROP TABLE on a temp table that has a cross-session dependent function fails with a catalog-level dependency error. - When the owner session ends, the normal session-exit cleanup cascades through DEPENDENCY_NORMAL and removes both the temp objects and any cross-session functions that depended on them. Also document the contract for RELATION_IS_OTHER_TEMP() so that future buffer-access entry points enforce the same rule. Author: Jim Jones <[email protected]> Author: Daniil Davydov <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Soumya S Murali <[email protected]> Reviewed-by: Tom Lane <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com --- src/include/utils/rel.h | 9 + src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/013_temp_obj_multisession.pl | 235 ++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 src/test/modules/test_misc/t/013_temp_obj_multisession.pl diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index cd1e92f2302..ad50e43b801 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -664,6 +664,15 @@ RelationCloseSmgr(Relation relation) * RELATION_IS_OTHER_TEMP * Test for a temporary relation that belongs to some other session. * + * Any code path that reads a relation's data must reject such relations: + * the owning session keeps the data in its private local buffer pool, + * which we cannot inspect. Existing buffer-manager entry points + * (ReadBufferExtended(), ReadBuffer_common(), StartReadBuffersImpl(), + * read_stream_begin_impl(), PrefetchBuffer()) already enforce this; any + * new buffer-access entry point must do the same. Command-level code + * (TRUNCATE, ALTER TABLE, VACUUM, CLUSTER, REINDEX, ...) additionally + * uses this macro for command-specific error messages. + * * Beware of multiple eval of argument */ #define RELATION_IS_OTHER_TEMP(relation) \ diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 356d8454b39..969e90b396d 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -21,6 +21,7 @@ tests += { 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', 't/012_ddlutils.pl', + 't/013_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl new file mode 100644 index 00000000000..0d211700977 --- /dev/null +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -0,0 +1,235 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Tests that one session cannot read or modify data in another session's +# temporary table. Each session keeps its temp data in its own local +# buffer pool, and a different backend has no visibility into those +# buffers, so any command that needs to look at the data must be +# rejected. +# +# DROP TABLE is intentionally allowed: it does not touch the table's +# contents, and autovacuum relies on this to clean up orphaned temp +# relations left behind by a crashed backend. +# +# A regression caught here typically means a new buffer-access entry +# point bypasses the RELATION_IS_OTHER_TEMP() check. See +# ReadBuffer_common(), StartReadBuffersImpl(), and read_stream_begin_impl() +# for the existing checks. When adding a new command or buffer-access +# path, also add a corresponding case below. + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Owner session. Created via background_psql so it stays alive while +# the second session probes its temp objects. +my $psql1 = $node->background_psql('postgres'); + +# Initially create the table without an index, so read paths go straight +# through the read-stream / buffer-manager entry points without being +# masked by an index scan that would hit ReadBuffer_common from nbtree. +$psql1->query_safe(q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +# Resolve the owner's temp schema so the probing session can refer to +# the table by a fully-qualified name. +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + +my ($stdout, $stderr); + +# DML and SELECT have to read the table's data and therefore go through +# the buffer manager. With no index on the table, the planner cannot +# use index access, so SELECT/UPDATE/DELETE/MERGE/COPY all run through +# the read-stream path. +# +# XXX: in current code, the read-stream path bypasses the +# RELATION_IS_OTHER_TEMP() check, so these commands silently see no +# rows / report zero affected rows -- the visible symptom of the bug +# this test suite documents. A follow-up patch will route the check +# through read_stream_begin_impl() and these assertions will be +# updated to expect "cannot access temporary tables of other sessions". + +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr); +is($stderr, '', 'SELECT (currently no error -- bug to be fixed)'); + +# INSERT goes through hio.c which calls ReadBufferExtended() to find a +# page with free space; that hits the existing check before any data is +# written. This case currently errors as expected. +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'INSERT (caught via hio.c)'); + +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr); +is($stderr, '', 'UPDATE (currently no error -- bug to be fixed)'); + +$node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'DELETE (currently no error -- bug to be fixed)'); + +$node->psql( + 'postgres', + "MERGE INTO $tempschema.foo USING (VALUES (42)) AS s(val) " + . "ON foo.val = s.val WHEN MATCHED THEN DELETE;", + stderr => \$stderr); +is($stderr, '', 'MERGE (currently no error -- bug to be fixed)'); + +$node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr); +is($stderr, '', 'COPY (currently no error -- bug to be fixed)'); + +# DDL and maintenance commands have their own command-specific checks +# (older than the buffer-manager check above), so they fail with +# command-specific error messages. Verifying them here documents the +# expected behaviour and guards against accidental removal of those +# checks. + +$node->psql('postgres', "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr); +like($stderr, + qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE'); + +$node->psql( + 'postgres', + "ALTER TABLE $tempschema.foo ALTER COLUMN val TYPE bigint;", + stderr => \$stderr); +like($stderr, + qr/cannot alter temporary tables of other sessions/, + 'ALTER TABLE'); + +# VACUUM silently skips other sessions' temp tables (vacuum_rel() returns +# without warning to avoid noise during database-wide VACUUM). Verify +# that no error is reported, and that no buffer-access path is hit. +$node->psql('postgres', "VACUUM $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'VACUUM is silently skipped'); + +$node->psql('postgres', "CLUSTER $tempschema.foo;", stderr => \$stderr); +like($stderr, + qr/cannot execute CLUSTER on temporary tables of other sessions/, + 'CLUSTER'); + +# Now create an index to exercise the index-scan path. nbtree calls +# ReadBuffer (which is ReadBufferExtended -> ReadBuffer_common), so +# this exercises a different chain of buffer-manager entry points. +$psql1->query_safe(q(CREATE INDEX ON foo(val);)); + +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'index scan (ReadBuffer_common via nbtree)'); + +# ALTER INDEX goes through the same CheckAlterTableIsSafe() path as +# ALTER TABLE, so it produces the same error. +$node->psql( + 'postgres', + "ALTER INDEX $tempschema.foo_val_idx SET (fillfactor = 50);", + stderr => \$stderr); +like($stderr, + qr/cannot alter temporary tables of other sessions/, + 'ALTER INDEX'); + +# A function created by the owner in its own pg_temp using its own +# row type can be observed via the catalog by a separate session. +# ALTER FUNCTION and DROP FUNCTION on it must work as catalog +# operations -- they don't read the underlying table -- which +# documents the boundary between catalog and data access for temp +# objects. +$psql1->query_safe( + q[CREATE FUNCTION pg_temp.foo_id(r foo) RETURNS int LANGUAGE SQL ] + . q[AS 'SELECT r.val';]); + +$node->psql( + 'postgres', + "ALTER FUNCTION $tempschema.foo_id($tempschema.foo) " + . "SET search_path = pg_catalog;", + stderr => \$stderr); +is($stderr, '', 'ALTER FUNCTION on function over other session\'s row type'); + +$node->psql( + 'postgres', + "DROP FUNCTION $tempschema.foo_id($tempschema.foo);", + stderr => \$stderr); +is($stderr, '', 'DROP FUNCTION on function over other session\'s row type'); + +# DROP TABLE on another session's temp table is intentionally permitted. +# DROP doesn't touch the table's contents, and autovacuum relies on this +# to remove temp relations orphaned by a crashed backend. Verify that +# the bare DROP succeeds without error. +$node->psql('postgres', "DROP TABLE $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'DROP TABLE is allowed'); + +# Cross-session CREATE FUNCTION scenario. The owner creates a fresh +# temp table foo2 in its pg_temp namespace, and a separate session +# then creates a function whose argument type is that row type. +# PostgreSQL allows this and emits a NOTICE: the function is moved +# into the creator's pg_temp namespace with an auto-dependency on +# the borrowed type, so it disappears together with the session that +# created it. +$psql1->query_safe(q(CREATE TEMP TABLE foo2 AS SELECT 42 AS val;)); + +$node->psql( + 'postgres', + "CREATE FUNCTION public.cross_session_func(r $tempschema.foo2) " + . "RETURNS int LANGUAGE SQL AS 'SELECT 1';", + stderr => \$stderr); +like( + $stderr, + qr/function "cross_session_func" will be effectively temporary/, + 'CREATE FUNCTION using other session\'s row type is effectively temporary' +); + +# A bare DROP TABLE on foo2 now fails because cross_session_func +# depends on its row type. This is normal SQL dependency behaviour +# and documents that DROP itself is not blocked by buffer-manager +# checks -- we get a catalog-level error instead. +$node->psql('postgres', "DROP TABLE $tempschema.foo2;", stderr => \$stderr); +like( + $stderr, + qr/cannot drop table .*\.foo2 because other objects depend on it/, + 'DROP TABLE blocked by cross-session dependency'); + +# When the owner session ends, its temp objects are dropped via the +# normal session-exit cleanup, which cascades through +# DEPENDENCY_NORMAL and also removes the cross-session function that +# depended on the temp row type. This is the same mechanism +# autovacuum relies on to clean up temp relations left behind by a +# crashed backend. +$psql1->quit; + +$node->poll_query_until( + 'postgres', + "SELECT NOT EXISTS (SELECT 1 FROM pg_proc WHERE proname = 'cross_session_func')" +) or die "cross_session_func was not cleaned up after owner session exit"; + +ok(1, 'cross_session_func cleaned up when owner session ends'); + +done_testing(); -- 2.39.5 (Apple Git-154) ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-02 17:32 Jim Jones <[email protected]> parent: Alexander Korotkov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-05-02 17:32 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; Daniil Davydov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi On 02/05/2026 18:34, Alexander Korotkov wrote: > On Sat, May 2, 2026 at 6:37 PM Daniil Davydov <[email protected]> wrote: >> On Sat, May 2, 2026 at 9:16 PM Alexander Korotkov <[email protected]> wrote: >>> >>> Thank you for your feedback. I think the scope of this patch is well >>> described in [1]. We don't want to restrict the superuser from >>> something, but our buffer manager just technically can't access the >>> local buffers of other sessions. Read streams introduced a new code >>> path for reading relations, which was lacking of the proper check for >>> local buffers of other sessions. And this patch attempts to fix that. >>> DROP TABLE is an exclusion. It actually don't need to read contents >>> of buffers, just drop them. And DropRelationBuffers() have a special >>> exclusion for this case. So, DROP TABLE appears to be the only >>> operation that makes sense, it's a conscious exclusion, and there is >>> no intention to forbid it. >> >> Yep, exactly. +1 >>> I've revised the patch. 0001 contains tests and states the current >>> behavior. 0002 contains fix and the corresponding changes in the >>> tests. I made a change in 0001: removed the check in >>> ReadBufferExtended(). We added the same check to ReadBuffer_common(), >>> and I don't think it makes sense to do this check twice in the row. >> >> Thank you! But I'm afraid that you forgot to attach the patches.. > > Here they are. Thanks for the comprehensive additional tests! In addition to the DROP TABLE exception: It is also possible to LOCK temporary tables from other sessions: postgres=# BEGIN; BEGIN postgres=*# LOCK TABLE pg_temp_91.t IN ACCESS SHARE MODE ; LOCK TABLE pg_temp_91.t lives as long the transaction is open -- even after the origin session closes, which is totally expected. I'd say it falls into the same category of DROP TABLE, where the table contents are never read, so I'd argue it's ok. > >> BTW, what do you think about this proposal? : >>> On the other hand, we have an error message that says "cannot access...", which >>> may look like every kind of "access" is forbidden. I bet that this is the place >>> that has confused you. More accurate error message would be "cannot access >>> pages..." or "cannot access content...". I think we can change our error >>> message in this way. What do you think? >> >> We can easily include it in the first patch. > > This is possible, but I would keep that in a separate patch. We now > have clear scope for both patches: 0001 includes additional tests, > 0002 fixes the bug and restores old behavior. +1 for a separate patch. I think the scope of the current patch is good as-is. Thanks! Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-03 08:53 Daniil Davydov <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-05-03 08:53 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, On Sun, May 3, 2026 at 12:32 AM Jim Jones <[email protected]> wrote: > > In addition to the DROP TABLE exception: > > It is also possible to LOCK temporary tables from other sessions: > > postgres=# BEGIN; > BEGIN > postgres=*# LOCK TABLE pg_temp_91.t IN ACCESS SHARE MODE ; > LOCK TABLE > > pg_temp_91.t lives as long the transaction is open -- even after the > origin session closes, which is totally expected. I'd say it falls into > the same category of DROP TABLE, where the table contents are never > read, so I'd argue it's ok. Autovacuum locks orphaned table's oid before deleting it. LOCK TABLE command also locks the oid of the specified table. If we want to make sure that autovacuum can acquire the mentioned lock (i.e. cover this behavior using tests), I suggest testing it using the LOCK TABLE command. Please, see the attached patch that ensures that cross-session LOCK TABLE works properly. -- Best regards, Daniil Davydov Attachments: [text/x-patch] 0001-Test-cross-session-LOCK-TABLE-scenario.patch (2.1K, 2-0001-Test-cross-session-LOCK-TABLE-scenario.patch) download | inline diff: From 05aaab6a225195246e70ad465a0995c4c0437e23 Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Sun, 3 May 2026 15:51:06 +0700 Subject: [PATCH] Test cross-session LOCK TABLE scenario --- .../test_misc/t/013_temp_obj_multisession.pl | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl index b4442836bef..a7369700580 100644 --- a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -220,14 +220,39 @@ like( qr/cannot drop table .*\.foo2 because other objects depend on it/, 'DROP TABLE blocked by cross-session dependency'); +my $foo2_oid = $node->safe_psql('postgres', + "SELECT oid FROM pg_class WHERE relname='foo2';"); + +# Cross-session LOCK TABLE scenario. Ensure that LockRelationOid is working +# properly for other temp tables since this mechanism is also used by +# autovacuum during orphaned tables cleanup. +my $psql2 = $node->background_psql('postgres'); +$psql2->query_safe( + qq{ + BEGIN; + LOCK TABLE $tempschema.foo2 IN ACCESS SHARE MODE; +}); + # When the owner session ends, its temp objects are dropped via the # normal session-exit cleanup, which cascades through # DEPENDENCY_NORMAL and also removes the cross-session function that # depended on the temp row type. This is the same mechanism # autovacuum relies on to clean up temp relations left behind by a # crashed backend. +# Access share lock on the foo2 will block session-exit cleanup, because an +# owner will try to acquire deletion lock all its temp objects via +# findDependentObjects. +my $log_offset = -s $node->logfile; $psql1->quit; +# Check whether session-exit cleanup is blocked. +$node->wait_for_log(qr/waiting for AccessExclusiveLock on relation $foo2_oid/, + $log_offset); + +# Release lock on foo2 and allow session-exit cleanup to finish. +$psql2->query_safe(q(COMMIT;)); +$psql2->quit; + $node->poll_query_until( 'postgres', "SELECT NOT EXISTS (SELECT 1 FROM pg_proc WHERE proname = 'cross_session_func')" -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-03 12:49 Jim Jones <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-05-03 12:49 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi Daniil On 03/05/2026 10:53, Daniil Davydov wrote: > Autovacuum locks orphaned table's oid before deleting it. LOCK TABLE command > also locks the oid of the specified table. If we want to make sure that > autovacuum can acquire the mentioned lock (i.e. cover this behavior using > tests), I suggest testing it using the LOCK TABLE command. +1 for the extra test. > Please, see the attached patch that ensures that cross-session LOCK TABLE works > properly. I guess you should either add it to the 0001 sent by Alexander, or create a 0003. Right now the cfbot is complaining, as 013_temp_obj_multisession.pl still does not exist :) Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-04 09:31 Daniil Davydov <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Daniil Davydov @ 2026-05-04 09:31 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, On Sun, May 3, 2026 at 7:49 PM Jim Jones <[email protected]> wrote: > > On 03/05/2026 10:53, Daniil Davydov wrote: > > Please, see the attached patch that ensures that cross-session LOCK TABLE works > > properly. > > I guess you should either add it to the 0001 sent by Alexander, or > create a 0003. Right now the cfbot is complaining, as > 013_temp_obj_multisession.pl still does not exist :) > OK, I'll attach this patch as a third one, just for review purposes. After agreement on its content, it should be included into the 0001 and 0002 patches. -- Best regards, Daniil Davydov Attachments: [text/x-patch] v21-0002-Prevent-access-to-other-sessions-temp-tables.patch (8.8K, 2-v21-0002-Prevent-access-to-other-sessions-temp-tables.patch) download | inline diff: From 2ec45b8ec3a0eb56ed1f2d10a0cb89ef57a8f402 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Sat, 2 May 2026 15:23:42 +0300 Subject: [PATCH v21 2/3] Prevent access to other sessions' temp tables Commit b7b0f3f2724 ("Use streaming I/O in sequential scans") routed sequential scans through read_stream_next_buffer(), bypassing the RELATION_IS_OTHER_TEMP() check in ReadBufferExtended(). As a result, a superuser can attempt to read or modify temp tables of other sessions through the read-stream path. When no index is present so the planner cannot pick an index scan, SELECT/UPDATE/DELETE/MERGE silently see no rows / report zero affected rows, and COPY produces an empty output -- because the buffer manager has no visibility into the owning session's local buffers and silently returns nothing. INSERT and any path that goes through nbtree (i.e. an index scan) still error out via the existing check in ReadBufferExtended(), which is reached from hio.c and nbtree respectively, but this is incidental. Fix by enforcing RELATION_IS_OTHER_TEMP() at three additional buffer-manager entry points: - read_stream_begin_impl() rejects the read at stream setup time, covering sequential and bitmap scans that go through the read-stream path. - ReadBuffer_common() becomes the canonical place for the check, consolidating the existing one previously kept in ReadBufferExtended(). All ReadBufferExtended() callers go through ReadBuffer_common(), so the consolidation is behaviour-preserving. - StartReadBuffersImpl() catches direct callers of StartReadBuffers() that bypass both of the above. This is currently defense-in-depth but documents the contract for future code. The companion test in src/test/modules/test_misc was added in the preceding commit; this commit updates the assertions for SELECT, UPDATE, DELETE, MERGE and COPY (which previously documented the bug as silent success) to expect the new error. Author: Jim Jones <[email protected]> Author: Daniil Davydov <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Soumya S Murali <[email protected]> Reviewed-by: Tom Lane <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com Backpatch-through: 17 --- src/backend/storage/aio/read_stream.c | 10 ++++++ src/backend/storage/buffer/bufmgr.c | 33 ++++++++++------- .../test_misc/t/013_temp_obj_multisession.pl | 35 ++++++++++--------- 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 2374b4cd507..a318539e56c 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. This * number also affects how far we look ahead for opportunities to start diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 1878efb4aa9..770209d606c 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -791,7 +791,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RelationUsesLocalBuffers(reln)) { - /* see comments in ReadBufferExtended */ + /* see comments in ReadBuffer_common */ if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -928,19 +928,10 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, { Buffer buf; - /* - * Reject attempts to read non-local temporary relations; we would be - * likely to get wrong data since we have no visibility into the owning - * session's local buffers. - */ - if (RELATION_IS_OTHER_TEMP(reln)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); - /* * Read the buffer, and update pgstat counters to reflect a cache hit or - * miss. + * miss. The other-session temp-relation check is enforced by + * ReadBuffer_common(). */ buf = ReadBuffer_common(reln, RelationGetSmgr(reln), 0, forkNum, blockNum, mode, strategy); @@ -1292,6 +1283,18 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, int flags; char persistence; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. This is the canonical place for the check, + * covering the ReadBufferExtended() entry point and any other caller + * that supplies a Relation. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Backward compatibility path, most code should use ExtendBufferedRel() * instead, as acquiring the extension lock inside ExtendBufferedRel() @@ -1382,6 +1385,12 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, Assert(*nblocks > 0); Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + /* see comments in ReadBuffer_common */ + if (operation->rel && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + if (operation->persistence == RELPERSISTENCE_TEMP) { io_context = IOCONTEXT_NORMAL; diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl index 0d211700977..b4442836bef 100644 --- a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -55,25 +55,20 @@ my ($stdout, $stderr); # DML and SELECT have to read the table's data and therefore go through # the buffer manager. With no index on the table, the planner cannot # use index access, so SELECT/UPDATE/DELETE/MERGE/COPY all run through -# the read-stream path. -# -# XXX: in current code, the read-stream path bypasses the -# RELATION_IS_OTHER_TEMP() check, so these commands silently see no -# rows / report zero affected rows -- the visible symptom of the bug -# this test suite documents. A follow-up patch will route the check -# through read_stream_begin_impl() and these assertions will be -# updated to expect "cannot access temporary tables of other sessions". +# the read-stream path and are caught by read_stream_begin_impl(). $node->psql( 'postgres', "SELECT val FROM $tempschema.foo;", - stdout => \$stdout, stderr => \$stderr); -is($stderr, '', 'SELECT (currently no error -- bug to be fixed)'); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'SELECT (seqscan via read_stream)'); # INSERT goes through hio.c which calls ReadBufferExtended() to find a -# page with free space; that hits the existing check before any data is -# written. This case currently errors as expected. +# page with free space; that hits the existing check before any data +# is written. $node->psql( 'postgres', "INSERT INTO $tempschema.foo VALUES (73);", @@ -86,21 +81,29 @@ $node->psql( 'postgres', "UPDATE $tempschema.foo SET val = NULL;", stderr => \$stderr); -is($stderr, '', 'UPDATE (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'UPDATE'); $node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); -is($stderr, '', 'DELETE (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'DELETE'); $node->psql( 'postgres', "MERGE INTO $tempschema.foo USING (VALUES (42)) AS s(val) " . "ON foo.val = s.val WHEN MATCHED THEN DELETE;", stderr => \$stderr); -is($stderr, '', 'MERGE (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'MERGE'); $node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", stderr => \$stderr); -is($stderr, '', 'COPY (currently no error -- bug to be fixed)'); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'COPY'); # DDL and maintenance commands have their own command-specific checks # (older than the buffer-manager check above), so they fail with -- 2.43.0 [text/x-patch] v21-0003-Test-cross-session-LOCK-TABLE-scenario.patch (2.1K, 3-v21-0003-Test-cross-session-LOCK-TABLE-scenario.patch) download | inline diff: From 05aaab6a225195246e70ad465a0995c4c0437e23 Mon Sep 17 00:00:00 2001 From: Daniil Davidov <[email protected]> Date: Sun, 3 May 2026 15:51:06 +0700 Subject: [PATCH v21 3/3] Test cross-session LOCK TABLE scenario --- .../test_misc/t/013_temp_obj_multisession.pl | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl index b4442836bef..a7369700580 100644 --- a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -220,14 +220,39 @@ like( qr/cannot drop table .*\.foo2 because other objects depend on it/, 'DROP TABLE blocked by cross-session dependency'); +my $foo2_oid = $node->safe_psql('postgres', + "SELECT oid FROM pg_class WHERE relname='foo2';"); + +# Cross-session LOCK TABLE scenario. Ensure that LockRelationOid is working +# properly for other temp tables since this mechanism is also used by +# autovacuum during orphaned tables cleanup. +my $psql2 = $node->background_psql('postgres'); +$psql2->query_safe( + qq{ + BEGIN; + LOCK TABLE $tempschema.foo2 IN ACCESS SHARE MODE; +}); + # When the owner session ends, its temp objects are dropped via the # normal session-exit cleanup, which cascades through # DEPENDENCY_NORMAL and also removes the cross-session function that # depended on the temp row type. This is the same mechanism # autovacuum relies on to clean up temp relations left behind by a # crashed backend. +# Access share lock on the foo2 will block session-exit cleanup, because an +# owner will try to acquire deletion lock all its temp objects via +# findDependentObjects. +my $log_offset = -s $node->logfile; $psql1->quit; +# Check whether session-exit cleanup is blocked. +$node->wait_for_log(qr/waiting for AccessExclusiveLock on relation $foo2_oid/, + $log_offset); + +# Release lock on foo2 and allow session-exit cleanup to finish. +$psql2->query_safe(q(COMMIT;)); +$psql2->quit; + $node->poll_query_until( 'postgres', "SELECT NOT EXISTS (SELECT 1 FROM pg_proc WHERE proname = 'cross_session_func')" -- 2.43.0 [text/x-patch] v21-0001-Add-tests-for-cross-session-temp-table-access.patch (13.7K, 4-v21-0001-Add-tests-for-cross-session-temp-table-access.patch) download | inline diff: From e3739ee30d6dea0630d56a45e109b0d762cda333 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Sat, 2 May 2026 15:22:26 +0300 Subject: [PATCH v21 1/3] Add tests for cross-session temp table access Add a TAP test in src/test/modules/test_misc that documents what happens when one session attempts to read or modify another session's temporary table. This commit only adds tests; it does not change backend behaviour, so the assertions reflect current behaviour: - SELECT, UPDATE, DELETE, MERGE, COPY on a table without an index silently succeed with no error and zero rows / zero affected rows. These commands run through the read-stream path, which currently bypasses the RELATION_IS_OTHER_TEMP() check. This is the underlying bug to be fixed in a follow-up. - INSERT errors with "cannot access temporary tables of other sessions" because hio.c calls ReadBufferExtended() to find a page with free space and is caught by the existing check there. - Index scan errors via the same existing check, reached through nbtree -> ReadBuffer -> ReadBufferExtended. - TRUNCATE / ALTER TABLE / ALTER INDEX / CLUSTER fail with their command-specific error messages. - VACUUM is silently skipped to avoid noise during database-wide VACUUM (vacuum_rel() returns without warning). - DROP TABLE is intentionally allowed: DROP does not touch the table's contents, and autovacuum relies on this to clean up temp relations orphaned by a crashed backend. - ALTER FUNCTION / DROP FUNCTION on an owner-created function over its own temp row type work as catalog operations -- they don't read the underlying data. - CREATE FUNCTION from a separate session, using another session's temp row type as an argument, is allowed but emits a NOTICE: the function is moved into the creator's pg_temp namespace with an auto-dependency on the borrowed type, so it disappears together with the session that created it. - A bare DROP TABLE on a temp table that has a cross-session dependent function fails with a catalog-level dependency error. - When the owner session ends, the normal session-exit cleanup cascades through DEPENDENCY_NORMAL and removes both the temp objects and any cross-session functions that depended on them. Also document the contract for RELATION_IS_OTHER_TEMP() so that future buffer-access entry points enforce the same rule. Author: Jim Jones <[email protected]> Author: Daniil Davydov <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Soumya S Murali <[email protected]> Reviewed-by: Tom Lane <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com --- src/include/utils/rel.h | 9 + src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/013_temp_obj_multisession.pl | 235 ++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 src/test/modules/test_misc/t/013_temp_obj_multisession.pl diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index cd1e92f2302..ad50e43b801 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -664,6 +664,15 @@ RelationCloseSmgr(Relation relation) * RELATION_IS_OTHER_TEMP * Test for a temporary relation that belongs to some other session. * + * Any code path that reads a relation's data must reject such relations: + * the owning session keeps the data in its private local buffer pool, + * which we cannot inspect. Existing buffer-manager entry points + * (ReadBufferExtended(), ReadBuffer_common(), StartReadBuffersImpl(), + * read_stream_begin_impl(), PrefetchBuffer()) already enforce this; any + * new buffer-access entry point must do the same. Command-level code + * (TRUNCATE, ALTER TABLE, VACUUM, CLUSTER, REINDEX, ...) additionally + * uses this macro for command-specific error messages. + * * Beware of multiple eval of argument */ #define RELATION_IS_OTHER_TEMP(relation) \ diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 356d8454b39..969e90b396d 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -21,6 +21,7 @@ tests += { 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', 't/012_ddlutils.pl', + 't/013_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl new file mode 100644 index 00000000000..0d211700977 --- /dev/null +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -0,0 +1,235 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Tests that one session cannot read or modify data in another session's +# temporary table. Each session keeps its temp data in its own local +# buffer pool, and a different backend has no visibility into those +# buffers, so any command that needs to look at the data must be +# rejected. +# +# DROP TABLE is intentionally allowed: it does not touch the table's +# contents, and autovacuum relies on this to clean up orphaned temp +# relations left behind by a crashed backend. +# +# A regression caught here typically means a new buffer-access entry +# point bypasses the RELATION_IS_OTHER_TEMP() check. See +# ReadBuffer_common(), StartReadBuffersImpl(), and read_stream_begin_impl() +# for the existing checks. When adding a new command or buffer-access +# path, also add a corresponding case below. + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Owner session. Created via background_psql so it stays alive while +# the second session probes its temp objects. +my $psql1 = $node->background_psql('postgres'); + +# Initially create the table without an index, so read paths go straight +# through the read-stream / buffer-manager entry points without being +# masked by an index scan that would hit ReadBuffer_common from nbtree. +$psql1->query_safe(q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +# Resolve the owner's temp schema so the probing session can refer to +# the table by a fully-qualified name. +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + +my ($stdout, $stderr); + +# DML and SELECT have to read the table's data and therefore go through +# the buffer manager. With no index on the table, the planner cannot +# use index access, so SELECT/UPDATE/DELETE/MERGE/COPY all run through +# the read-stream path. +# +# XXX: in current code, the read-stream path bypasses the +# RELATION_IS_OTHER_TEMP() check, so these commands silently see no +# rows / report zero affected rows -- the visible symptom of the bug +# this test suite documents. A follow-up patch will route the check +# through read_stream_begin_impl() and these assertions will be +# updated to expect "cannot access temporary tables of other sessions". + +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr); +is($stderr, '', 'SELECT (currently no error -- bug to be fixed)'); + +# INSERT goes through hio.c which calls ReadBufferExtended() to find a +# page with free space; that hits the existing check before any data is +# written. This case currently errors as expected. +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr); +like($stderr, + qr/cannot access temporary tables of other sessions/, + 'INSERT (caught via hio.c)'); + +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr); +is($stderr, '', 'UPDATE (currently no error -- bug to be fixed)'); + +$node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'DELETE (currently no error -- bug to be fixed)'); + +$node->psql( + 'postgres', + "MERGE INTO $tempschema.foo USING (VALUES (42)) AS s(val) " + . "ON foo.val = s.val WHEN MATCHED THEN DELETE;", + stderr => \$stderr); +is($stderr, '', 'MERGE (currently no error -- bug to be fixed)'); + +$node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr); +is($stderr, '', 'COPY (currently no error -- bug to be fixed)'); + +# DDL and maintenance commands have their own command-specific checks +# (older than the buffer-manager check above), so they fail with +# command-specific error messages. Verifying them here documents the +# expected behaviour and guards against accidental removal of those +# checks. + +$node->psql('postgres', "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr); +like($stderr, + qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE'); + +$node->psql( + 'postgres', + "ALTER TABLE $tempschema.foo ALTER COLUMN val TYPE bigint;", + stderr => \$stderr); +like($stderr, + qr/cannot alter temporary tables of other sessions/, + 'ALTER TABLE'); + +# VACUUM silently skips other sessions' temp tables (vacuum_rel() returns +# without warning to avoid noise during database-wide VACUUM). Verify +# that no error is reported, and that no buffer-access path is hit. +$node->psql('postgres', "VACUUM $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'VACUUM is silently skipped'); + +$node->psql('postgres', "CLUSTER $tempschema.foo;", stderr => \$stderr); +like($stderr, + qr/cannot execute CLUSTER on temporary tables of other sessions/, + 'CLUSTER'); + +# Now create an index to exercise the index-scan path. nbtree calls +# ReadBuffer (which is ReadBufferExtended -> ReadBuffer_common), so +# this exercises a different chain of buffer-manager entry points. +$psql1->query_safe(q(CREATE INDEX ON foo(val);)); + +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'index scan (ReadBuffer_common via nbtree)'); + +# ALTER INDEX goes through the same CheckAlterTableIsSafe() path as +# ALTER TABLE, so it produces the same error. +$node->psql( + 'postgres', + "ALTER INDEX $tempschema.foo_val_idx SET (fillfactor = 50);", + stderr => \$stderr); +like($stderr, + qr/cannot alter temporary tables of other sessions/, + 'ALTER INDEX'); + +# A function created by the owner in its own pg_temp using its own +# row type can be observed via the catalog by a separate session. +# ALTER FUNCTION and DROP FUNCTION on it must work as catalog +# operations -- they don't read the underlying table -- which +# documents the boundary between catalog and data access for temp +# objects. +$psql1->query_safe( + q[CREATE FUNCTION pg_temp.foo_id(r foo) RETURNS int LANGUAGE SQL ] + . q[AS 'SELECT r.val';]); + +$node->psql( + 'postgres', + "ALTER FUNCTION $tempschema.foo_id($tempschema.foo) " + . "SET search_path = pg_catalog;", + stderr => \$stderr); +is($stderr, '', 'ALTER FUNCTION on function over other session\'s row type'); + +$node->psql( + 'postgres', + "DROP FUNCTION $tempschema.foo_id($tempschema.foo);", + stderr => \$stderr); +is($stderr, '', 'DROP FUNCTION on function over other session\'s row type'); + +# DROP TABLE on another session's temp table is intentionally permitted. +# DROP doesn't touch the table's contents, and autovacuum relies on this +# to remove temp relations orphaned by a crashed backend. Verify that +# the bare DROP succeeds without error. +$node->psql('postgres', "DROP TABLE $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'DROP TABLE is allowed'); + +# Cross-session CREATE FUNCTION scenario. The owner creates a fresh +# temp table foo2 in its pg_temp namespace, and a separate session +# then creates a function whose argument type is that row type. +# PostgreSQL allows this and emits a NOTICE: the function is moved +# into the creator's pg_temp namespace with an auto-dependency on +# the borrowed type, so it disappears together with the session that +# created it. +$psql1->query_safe(q(CREATE TEMP TABLE foo2 AS SELECT 42 AS val;)); + +$node->psql( + 'postgres', + "CREATE FUNCTION public.cross_session_func(r $tempschema.foo2) " + . "RETURNS int LANGUAGE SQL AS 'SELECT 1';", + stderr => \$stderr); +like( + $stderr, + qr/function "cross_session_func" will be effectively temporary/, + 'CREATE FUNCTION using other session\'s row type is effectively temporary' +); + +# A bare DROP TABLE on foo2 now fails because cross_session_func +# depends on its row type. This is normal SQL dependency behaviour +# and documents that DROP itself is not blocked by buffer-manager +# checks -- we get a catalog-level error instead. +$node->psql('postgres', "DROP TABLE $tempschema.foo2;", stderr => \$stderr); +like( + $stderr, + qr/cannot drop table .*\.foo2 because other objects depend on it/, + 'DROP TABLE blocked by cross-session dependency'); + +# When the owner session ends, its temp objects are dropped via the +# normal session-exit cleanup, which cascades through +# DEPENDENCY_NORMAL and also removes the cross-session function that +# depended on the temp row type. This is the same mechanism +# autovacuum relies on to clean up temp relations left behind by a +# crashed backend. +$psql1->quit; + +$node->poll_query_until( + 'postgres', + "SELECT NOT EXISTS (SELECT 1 FROM pg_proc WHERE proname = 'cross_session_func')" +) or die "cross_session_func was not cleaned up after owner session exit"; + +ok(1, 'cross_session_func cleaned up when owner session ends'); + +done_testing(); -- 2.43.0 ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-07 08:04 Alexander Korotkov <[email protected]> parent: Daniil Davydov <[email protected]> 0 siblings, 3 replies; 52+ messages in thread From: Alexander Korotkov @ 2026-05-07 08:04 UTC (permalink / raw) To: Daniil Davydov <[email protected]>; +Cc: Jim Jones <[email protected]>; Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Mon, May 4, 2026 at 12:31 PM Daniil Davydov <[email protected]> wrote: > > On Sun, May 3, 2026 at 7:49 PM Jim Jones <[email protected]> wrote: > > > > On 03/05/2026 10:53, Daniil Davydov wrote: > > > Please, see the attached patch that ensures that cross-session LOCK TABLE works > > > properly. > > > > I guess you should either add it to the 0001 sent by Alexander, or > > create a 0003. Right now the cfbot is complaining, as > > 013_temp_obj_multisession.pl still does not exist :) > > > > OK, I'll attach this patch as a third one, just for review purposes. After > agreement on its content, it should be included into the 0001 and 0002 patches. Thank you. I've integrated your check into 0001, and added an explicit check that the table gets removed once the lock by other session is removed. Let me do a quick summary: * Our buffer manager is not capable for reading temp tables of other sessions. * This was covered by explicit checks, but broken since b7b0f3f27241 introduced alternative code path for reading tables. * This doesn't apply to DROP TABLE. DROP TABLE is a conscious exclusion and the only operation we can do correctly for other session' temp tables. There is an explicit exclusion in the code to skip the attempt to cleanup buffers of other session' temp tables. * This patchset consists of tests (0001) for various operations with other session's temp tables including buggy behavior, and the fix (0002) including changes for tests. Thus, I don't see the reason why this shouldn't be committed and backpatched to PG17 (first release containing b7b0f3f27241). Opinions? Michael? ------ Regards, Alexander Korotkov Supabase Attachments: [application/octet-stream] v22-0002-Prevent-access-to-other-sessions-temp-tables.patch (8.8K, 2-v22-0002-Prevent-access-to-other-sessions-temp-tables.patch) download | inline diff: From 990c7a826eda2865f2e6c2c081a502cb04dba973 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Sat, 2 May 2026 15:23:42 +0300 Subject: [PATCH v22 2/2] Prevent access to other sessions' temp tables Commit b7b0f3f2724 ("Use streaming I/O in sequential scans") routed sequential scans through read_stream_next_buffer(), bypassing the RELATION_IS_OTHER_TEMP() check in ReadBufferExtended(). As a result, a superuser can attempt to read or modify temp tables of other sessions through the read-stream path. When no index is present so the planner cannot pick an index scan, SELECT/UPDATE/DELETE/MERGE silently see no rows / report zero affected rows, and COPY produces an empty output -- because the buffer manager has no visibility into the owning session's local buffers and silently returns nothing. INSERT and any path that goes through nbtree (i.e. an index scan) still error out via the existing check in ReadBufferExtended(), which is reached from hio.c and nbtree respectively, but this is incidental. Fix by enforcing RELATION_IS_OTHER_TEMP() at three additional buffer-manager entry points: - read_stream_begin_impl() rejects the read at stream setup time, covering sequential and bitmap scans that go through the read-stream path. - ReadBuffer_common() becomes the canonical place for the check, consolidating the existing one previously kept in ReadBufferExtended(). All ReadBufferExtended() callers go through ReadBuffer_common(), so the consolidation is behaviour-preserving. - StartReadBuffersImpl() catches direct callers of StartReadBuffers() that bypass both of the above. This is currently defense-in-depth but documents the contract for future code. The companion test in src/test/modules/test_misc was added in the preceding commit; this commit updates the assertions for SELECT, UPDATE, DELETE, MERGE and COPY (which previously documented the bug as silent success) to expect the new error. Author: Jim Jones <[email protected]> Author: Daniil Davydov <[email protected]> Co-authored-by: Alexander Korotkov <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Soumya S Murali <[email protected]> Reviewed-by: Tom Lane <[email protected]> Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com Backpatch-through: 17 --- src/backend/storage/aio/read_stream.c | 10 ++++++ src/backend/storage/buffer/bufmgr.c | 33 ++++++++++++------- .../test_misc/t/013_temp_obj_multisession.pl | 27 +++++++-------- 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 2374b4cd507..a318539e56c 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -776,6 +776,16 @@ read_stream_begin_impl(int flags, uint32 max_possible_buffer_limit; Oid tablespace_id; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Decide how many I/Os we will allow to run at the same time. This * number also affects how far we look ahead for opportunities to start diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 1878efb4aa9..cc398db124d 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -791,7 +791,7 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) if (RelationUsesLocalBuffers(reln)) { - /* see comments in ReadBufferExtended */ + /* see comments in ReadBuffer_common */ if (RELATION_IS_OTHER_TEMP(reln)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -928,19 +928,10 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, { Buffer buf; - /* - * Reject attempts to read non-local temporary relations; we would be - * likely to get wrong data since we have no visibility into the owning - * session's local buffers. - */ - if (RELATION_IS_OTHER_TEMP(reln)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot access temporary tables of other sessions"))); - /* * Read the buffer, and update pgstat counters to reflect a cache hit or - * miss. + * miss. The other-session temp-relation check is enforced by + * ReadBuffer_common(). */ buf = ReadBuffer_common(reln, RelationGetSmgr(reln), 0, forkNum, blockNum, mode, strategy); @@ -1292,6 +1283,18 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, int flags; char persistence; + /* + * Reject attempts to read non-local temporary relations; we would be + * likely to get wrong data since we have no visibility into the owning + * session's local buffers. This is the canonical place for the check, + * covering the ReadBufferExtended() entry point and any other caller that + * supplies a Relation. + */ + if (rel && RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + /* * Backward compatibility path, most code should use ExtendBufferedRel() * instead, as acquiring the extension lock inside ExtendBufferedRel() @@ -1382,6 +1385,12 @@ StartReadBuffersImpl(ReadBuffersOperation *operation, Assert(*nblocks > 0); Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + /* see comments in ReadBuffer_common */ + if (operation->rel && RELATION_IS_OTHER_TEMP(operation->rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot access temporary tables of other sessions"))); + if (operation->persistence == RELPERSISTENCE_TEMP) { io_context = IOCONTEXT_NORMAL; diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl index bfcba42acd0..5f3cc7d2fc5 100644 --- a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -55,25 +55,20 @@ my ($stdout, $stderr); # DML and SELECT have to read the table's data and therefore go through # the buffer manager. With no index on the table, the planner cannot # use index access, so SELECT/UPDATE/DELETE/MERGE/COPY all run through -# the read-stream path. -# -# XXX: in current code, the read-stream path bypasses the -# RELATION_IS_OTHER_TEMP() check, so these commands silently see no -# rows / report zero affected rows -- the visible symptom of the bug -# this test suite documents. A follow-up patch will route the check -# through read_stream_begin_impl() and these assertions will be -# updated to expect "cannot access temporary tables of other sessions". +# the read-stream path and are caught by read_stream_begin_impl(). $node->psql( 'postgres', "SELECT val FROM $tempschema.foo;", - stdout => \$stdout, stderr => \$stderr); -is($stderr, '', 'SELECT (currently no error -- bug to be fixed)'); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'SELECT (seqscan via read_stream)'); # INSERT goes through hio.c which calls ReadBufferExtended() to find a -# page with free space; that hits the existing check before any data is -# written. This case currently errors as expected. +# page with free space; that hits the existing check before any data +# is written. $node->psql( 'postgres', "INSERT INTO $tempschema.foo VALUES (73);", @@ -87,21 +82,21 @@ $node->psql( 'postgres', "UPDATE $tempschema.foo SET val = NULL;", stderr => \$stderr); -is($stderr, '', 'UPDATE (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'UPDATE'); $node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); -is($stderr, '', 'DELETE (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'DELETE'); $node->psql( 'postgres', "MERGE INTO $tempschema.foo USING (VALUES (42)) AS s(val) " . "ON foo.val = s.val WHEN MATCHED THEN DELETE;", stderr => \$stderr); -is($stderr, '', 'MERGE (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'MERGE'); $node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", stderr => \$stderr); -is($stderr, '', 'COPY (currently no error -- bug to be fixed)'); +like($stderr, qr/cannot access temporary tables of other sessions/, 'COPY'); # DDL and maintenance commands have their own command-specific checks # (older than the buffer-manager check above), so they fail with -- 2.39.5 (Apple Git-154) [application/octet-stream] v22-0001-Add-tests-for-cross-session-temp-table-access.patch (15.4K, 3-v22-0001-Add-tests-for-cross-session-temp-table-access.patch) download | inline diff: From 77ae14ba31396e25b7417b9357edb7982611035d Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Sat, 2 May 2026 15:22:26 +0300 Subject: [PATCH v22 1/2] Add tests for cross-session temp table access Add a TAP test in src/test/modules/test_misc that documents what happens when one session attempts to read or modify another session's temporary table. This commit only adds tests; it does not change backend behaviour, so the assertions reflect current behaviour: - SELECT, UPDATE, DELETE, MERGE, COPY on a table without an index silently succeed with no error and zero rows / zero affected rows. These commands run through the read-stream path, which currently bypasses the RELATION_IS_OTHER_TEMP() check. This is the underlying bug to be fixed in a follow-up. - INSERT errors with "cannot access temporary tables of other sessions" because hio.c calls ReadBufferExtended() to find a page with free space and is caught by the existing check there. - Index scan errors via the same existing check, reached through nbtree -> ReadBuffer -> ReadBufferExtended. - TRUNCATE / ALTER TABLE / ALTER INDEX / CLUSTER fail with their command-specific error messages. - VACUUM is silently skipped to avoid noise during database-wide VACUUM (vacuum_rel() returns without warning). - DROP TABLE is intentionally allowed: DROP does not touch the table's contents, and autovacuum relies on this to clean up temp relations orphaned by a crashed backend. - ALTER FUNCTION / DROP FUNCTION on an owner-created function over its own temp row type work as catalog operations -- they don't read the underlying data. - CREATE FUNCTION from a separate session, using another session's temp row type as an argument, is allowed but emits a NOTICE: the function is moved into the creator's pg_temp namespace with an auto-dependency on the borrowed type, so it disappears together with the session that created it. - A bare DROP TABLE on a temp table that has a cross-session dependent function fails with a catalog-level dependency error. - LOCK TABLE in ACCESS SHARE mode on another session's temp table succeeds and properly blocks the owner's session-exit cleanup (which acquires AccessExclusiveLock via findDependentObjects). This exercises the same LockRelationOid path used by autovacuum when cleaning up orphaned temp relations. - When the owner session ends, the normal session-exit cleanup cascades through DEPENDENCY_NORMAL and removes both the temp objects and any cross-session functions that depended on them. Also document the contract for RELATION_IS_OTHER_TEMP() so that future buffer-access entry points enforce the same rule. Author: Jim Jones <[email protected]> Author: Daniil Davydov <[email protected]> Co-authored-by: Alexander Korotkov <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Soumya S Murali <[email protected]> Reviewed-by: Tom Lane <[email protected]> Discussion: https://postgr.es/m/CAJDiXghdFcZ8%3Dnh4G69te7iRr3Q0uFyXxb3ZdG09_GTNZXwH0g%40mail.gmail.com --- src/include/utils/rel.h | 9 + src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/013_temp_obj_multisession.pl | 267 ++++++++++++++++++ 3 files changed, 277 insertions(+) create mode 100644 src/test/modules/test_misc/t/013_temp_obj_multisession.pl diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index cd1e92f2302..ad50e43b801 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -664,6 +664,15 @@ RelationCloseSmgr(Relation relation) * RELATION_IS_OTHER_TEMP * Test for a temporary relation that belongs to some other session. * + * Any code path that reads a relation's data must reject such relations: + * the owning session keeps the data in its private local buffer pool, + * which we cannot inspect. Existing buffer-manager entry points + * (ReadBufferExtended(), ReadBuffer_common(), StartReadBuffersImpl(), + * read_stream_begin_impl(), PrefetchBuffer()) already enforce this; any + * new buffer-access entry point must do the same. Command-level code + * (TRUNCATE, ALTER TABLE, VACUUM, CLUSTER, REINDEX, ...) additionally + * uses this macro for command-specific error messages. + * * Beware of multiple eval of argument */ #define RELATION_IS_OTHER_TEMP(relation) \ diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 356d8454b39..969e90b396d 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -21,6 +21,7 @@ tests += { 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', 't/012_ddlutils.pl', + 't/013_temp_obj_multisession.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/013_temp_obj_multisession.pl b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl new file mode 100644 index 00000000000..bfcba42acd0 --- /dev/null +++ b/src/test/modules/test_misc/t/013_temp_obj_multisession.pl @@ -0,0 +1,267 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Tests that one session cannot read or modify data in another session's +# temporary table. Each session keeps its temp data in its own local +# buffer pool, and a different backend has no visibility into those +# buffers, so any command that needs to look at the data must be +# rejected. +# +# DROP TABLE is intentionally allowed: it does not touch the table's +# contents, and autovacuum relies on this to clean up orphaned temp +# relations left behind by a crashed backend. +# +# A regression caught here typically means a new buffer-access entry +# point bypasses the RELATION_IS_OTHER_TEMP() check. See +# ReadBuffer_common(), StartReadBuffersImpl(), and read_stream_begin_impl() +# for the existing checks. When adding a new command or buffer-access +# path, also add a corresponding case below. + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::BackgroundPsql; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('temp_lock'); +$node->init; +$node->start; + +# Owner session. Created via background_psql so it stays alive while +# the second session probes its temp objects. +my $psql1 = $node->background_psql('postgres'); + +# Initially create the table without an index, so read paths go straight +# through the read-stream / buffer-manager entry points without being +# masked by an index scan that would hit ReadBuffer_common from nbtree. +$psql1->query_safe(q(CREATE TEMP TABLE foo AS SELECT 42 AS val;)); + +# Resolve the owner's temp schema so the probing session can refer to +# the table by a fully-qualified name. +my $tempschema = $node->safe_psql( + 'postgres', + q{ + SELECT n.nspname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE relname = 'foo' AND relpersistence = 't'; + } +); +chomp $tempschema; +ok($tempschema =~ /^pg_temp_\d+$/, "got temp schema: $tempschema"); + +my ($stdout, $stderr); + +# DML and SELECT have to read the table's data and therefore go through +# the buffer manager. With no index on the table, the planner cannot +# use index access, so SELECT/UPDATE/DELETE/MERGE/COPY all run through +# the read-stream path. +# +# XXX: in current code, the read-stream path bypasses the +# RELATION_IS_OTHER_TEMP() check, so these commands silently see no +# rows / report zero affected rows -- the visible symptom of the bug +# this test suite documents. A follow-up patch will route the check +# through read_stream_begin_impl() and these assertions will be +# updated to expect "cannot access temporary tables of other sessions". + +$node->psql( + 'postgres', + "SELECT val FROM $tempschema.foo;", + stdout => \$stdout, + stderr => \$stderr); +is($stderr, '', 'SELECT (currently no error -- bug to be fixed)'); + +# INSERT goes through hio.c which calls ReadBufferExtended() to find a +# page with free space; that hits the existing check before any data is +# written. This case currently errors as expected. +$node->psql( + 'postgres', + "INSERT INTO $tempschema.foo VALUES (73);", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'INSERT (caught via hio.c)'); + +$node->psql( + 'postgres', + "UPDATE $tempschema.foo SET val = NULL;", + stderr => \$stderr); +is($stderr, '', 'UPDATE (currently no error -- bug to be fixed)'); + +$node->psql('postgres', "DELETE FROM $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'DELETE (currently no error -- bug to be fixed)'); + +$node->psql( + 'postgres', + "MERGE INTO $tempschema.foo USING (VALUES (42)) AS s(val) " + . "ON foo.val = s.val WHEN MATCHED THEN DELETE;", + stderr => \$stderr); +is($stderr, '', 'MERGE (currently no error -- bug to be fixed)'); + +$node->psql('postgres', "COPY $tempschema.foo TO STDOUT;", + stderr => \$stderr); +is($stderr, '', 'COPY (currently no error -- bug to be fixed)'); + +# DDL and maintenance commands have their own command-specific checks +# (older than the buffer-manager check above), so they fail with +# command-specific error messages. Verifying them here documents the +# expected behaviour and guards against accidental removal of those +# checks. + +$node->psql('postgres', "TRUNCATE TABLE $tempschema.foo;", + stderr => \$stderr); +like($stderr, qr/cannot truncate temporary tables of other sessions/, + 'TRUNCATE'); + +$node->psql( + 'postgres', + "ALTER TABLE $tempschema.foo ALTER COLUMN val TYPE bigint;", + stderr => \$stderr); +like($stderr, qr/cannot alter temporary tables of other sessions/, + 'ALTER TABLE'); + +# VACUUM silently skips other sessions' temp tables (vacuum_rel() returns +# without warning to avoid noise during database-wide VACUUM). Verify +# that no error is reported, and that no buffer-access path is hit. +$node->psql('postgres', "VACUUM $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'VACUUM is silently skipped'); + +$node->psql('postgres', "CLUSTER $tempschema.foo;", stderr => \$stderr); +like($stderr, + qr/cannot execute CLUSTER on temporary tables of other sessions/, + 'CLUSTER'); + +# Now create an index to exercise the index-scan path. nbtree calls +# ReadBuffer (which is ReadBufferExtended -> ReadBuffer_common), so +# this exercises a different chain of buffer-manager entry points. +$psql1->query_safe(q(CREATE INDEX ON foo(val);)); + +$node->psql( + 'postgres', + "SET enable_seqscan = off; SELECT val FROM $tempschema.foo WHERE val = 42;", + stderr => \$stderr); +like( + $stderr, + qr/cannot access temporary tables of other sessions/, + 'index scan (ReadBuffer_common via nbtree)'); + +# ALTER INDEX goes through the same CheckAlterTableIsSafe() path as +# ALTER TABLE, so it produces the same error. +$node->psql( + 'postgres', + "ALTER INDEX $tempschema.foo_val_idx SET (fillfactor = 50);", + stderr => \$stderr); +like($stderr, qr/cannot alter temporary tables of other sessions/, + 'ALTER INDEX'); + +# A function created by the owner in its own pg_temp using its own +# row type can be observed via the catalog by a separate session. +# ALTER FUNCTION and DROP FUNCTION on it must work as catalog +# operations -- they don't read the underlying table -- which +# documents the boundary between catalog and data access for temp +# objects. +$psql1->query_safe( + q[CREATE FUNCTION pg_temp.foo_id(r foo) RETURNS int LANGUAGE SQL ] + . q[AS 'SELECT r.val';]); + +$node->psql( + 'postgres', + "ALTER FUNCTION $tempschema.foo_id($tempschema.foo) " + . "SET search_path = pg_catalog;", + stderr => \$stderr); +is($stderr, '', 'ALTER FUNCTION on function over other session\'s row type'); + +$node->psql( + 'postgres', + "DROP FUNCTION $tempschema.foo_id($tempschema.foo);", + stderr => \$stderr); +is($stderr, '', 'DROP FUNCTION on function over other session\'s row type'); + +# DROP TABLE on another session's temp table is intentionally permitted. +# DROP doesn't touch the table's contents, and autovacuum relies on this +# to remove temp relations orphaned by a crashed backend. Verify that +# the bare DROP succeeds without error. +$node->psql('postgres', "DROP TABLE $tempschema.foo;", stderr => \$stderr); +is($stderr, '', 'DROP TABLE is allowed'); + +# Cross-session CREATE FUNCTION scenario. The owner creates a fresh +# temp table foo2 in its pg_temp namespace, and a separate session +# then creates a function whose argument type is that row type. +# PostgreSQL allows this and emits a NOTICE: the function is moved +# into the creator's pg_temp namespace with an auto-dependency on +# the borrowed type, so it disappears together with the session that +# created it. +$psql1->query_safe(q(CREATE TEMP TABLE foo2 AS SELECT 42 AS val;)); + +$node->psql( + 'postgres', + "CREATE FUNCTION public.cross_session_func(r $tempschema.foo2) " + . "RETURNS int LANGUAGE SQL AS 'SELECT 1';", + stderr => \$stderr); +like( + $stderr, + qr/function "cross_session_func" will be effectively temporary/, + 'CREATE FUNCTION using other session\'s row type is effectively temporary' +); + +# A bare DROP TABLE on foo2 now fails because cross_session_func +# depends on its row type. This is normal SQL dependency behaviour +# and documents that DROP itself is not blocked by buffer-manager +# checks -- we get a catalog-level error instead. +$node->psql('postgres', "DROP TABLE $tempschema.foo2;", stderr => \$stderr); +like( + $stderr, + qr/cannot drop table .*\.foo2 because other objects depend on it/, + 'DROP TABLE blocked by cross-session dependency'); + +my $foo2_oid = $node->safe_psql('postgres', + "SELECT oid FROM pg_class WHERE relname='foo2';"); + +# Cross-session LOCK TABLE scenario. Ensure that LockRelationOid is working +# properly for other temp tables since this mechanism is also used by +# autovacuum during orphaned tables cleanup. +my $psql2 = $node->background_psql('postgres'); +$psql2->query_safe( + qq{ + BEGIN; + LOCK TABLE $tempschema.foo2 IN ACCESS SHARE MODE; +}); + +# When the owner session ends, its temp objects are dropped via the +# normal session-exit cleanup, which cascades through +# DEPENDENCY_NORMAL and also removes the cross-session function that +# depended on the temp row type. This is the same mechanism +# autovacuum relies on to clean up temp relations left behind by a +# crashed backend. +# Access share lock on the foo2 will block session-exit cleanup, because an +# owner will try to acquire deletion lock all its temp objects via +# findDependentObjects. +my $log_offset = -s $node->logfile; +$psql1->quit; + +# Check whether session-exit cleanup is blocked. +$node->wait_for_log(qr/waiting for AccessExclusiveLock on relation $foo2_oid/, + $log_offset); + +# Release lock on foo2 and allow session-exit cleanup to finish. +$psql2->query_safe(q(COMMIT;)); +$psql2->quit; + +# After releasing the lock, the owner can finally acquire +# AccessExclusiveLock on foo2 and finish session-exit cleanup. Verify +# directly that both foo2 (the locked temp table) and cross_session_func +# (which depended on its row type) have been dropped. Both being gone +# confirms the owner's cleanup got past the blocked findDependentObjects() +# call and completed normally. +$node->poll_query_until('postgres', + "SELECT NOT EXISTS (SELECT 1 FROM pg_class WHERE oid = $foo2_oid)") + or die "foo2 was not cleaned up after owner session exit"; + +is( $node->safe_psql( + 'postgres', + "SELECT count(*) FROM pg_proc WHERE proname = 'cross_session_func'"), + '0', + 'cross_session_func cleaned up by cascade from foo2'); + +done_testing(); -- 2.39.5 (Apple Git-154) ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-07 08:43 Jim Jones <[email protected]> parent: Alexander Korotkov <[email protected]> 2 siblings, 1 reply; 52+ messages in thread From: Jim Jones @ 2026-05-07 08:43 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; Daniil Davydov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On 07/05/2026 10:04, Alexander Korotkov wrote: > Thus, I don't see the reason why this shouldn't be committed and > backpatched to PG17 (first release containing b7b0f3f27241). Quick question: should we work on a separate patch for PG14-PG16? In these versions, the error message is raised depending on the temporary table's tuple count, as demonstrated in [1]: == session 1 == psql (14.20 (Debian 14.20-1.pgdg13+1)) Type "help" for help. postgres=# CREATE TEMPORARY TABLE t (id int); CREATE TABLE postgres=# \d pg_temp*.* Table "pg_temp_3.t" Column | Type | Collation | Nullable | Default --------+---------+-----------+----------+--------- id | integer | | | == session 2 == psql (14.20 (Debian 14.20-1.pgdg13+1)) Type "help" for help. postgres=# SELECT * FROM pg_temp_3.t; id ---- (0 rows) == session 1 == postgres=# INSERT INTO t VALUES (42); INSERT 0 1 == session 2 == postgres=# SELECT * FROM pg_temp_3.t; ERROR: cannot access temporary tables of other sessions Thanks! Best, Jim 1 - https://www.postgresql.org/message-id/800c75af-9bd0-48ac-b4bf-54cadf2bc723%40uni-muenster.de ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-07 10:43 Alexander Korotkov <[email protected]> parent: Jim Jones <[email protected]> 0 siblings, 0 replies; 52+ messages in thread From: Alexander Korotkov @ 2026-05-07 10:43 UTC (permalink / raw) To: Jim Jones <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Thu, May 7, 2026 at 11:43 AM Jim Jones <[email protected]> wrote: > On 07/05/2026 10:04, Alexander Korotkov wrote: > > Thus, I don't see the reason why this shouldn't be committed and > > backpatched to PG17 (first release containing b7b0f3f27241). > > Quick question: should we work on a separate patch for PG14-PG16? In > these versions, the error message is raised depending on the temporary > table's tuple count, as demonstrated in [1]: > > == session 1 == > > psql (14.20 (Debian 14.20-1.pgdg13+1)) > Type "help" for help. > > postgres=# CREATE TEMPORARY TABLE t (id int); > CREATE TABLE > postgres=# \d pg_temp*.* > Table "pg_temp_3.t" > Column | Type | Collation | Nullable | Default > --------+---------+-----------+----------+--------- > id | integer | | | > > == session 2 == > > psql (14.20 (Debian 14.20-1.pgdg13+1)) > Type "help" for help. > > postgres=# SELECT * FROM pg_temp_3.t; > id > ---- > (0 rows) > > == session 1 == > > postgres=# INSERT INTO t VALUES (42); > INSERT 0 1 > > == session 2 == > > postgres=# SELECT * FROM pg_temp_3.t; > ERROR: cannot access temporary tables of other sessions Thank you for your question. Yes, PostgreSQL 14-16 first does RelationGetNumberOfBlocks(), then scans the relation through the buffer manager. If RelationGetNumberOfBlocks() returns 0, then no buffers accessed and no error thrown. So, it could scan empty tables because it doesn't requires accessing local buffers of other sessions. I would investigate this further. If this doesn't have negative side effects (like execution of other commands that couldn't be performed correctly), I would avoid backpatching to these versions. ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-07 14:22 Tom Lane <[email protected]> parent: Alexander Korotkov <[email protected]> 2 siblings, 1 reply; 52+ messages in thread From: Tom Lane @ 2026-05-07 14:22 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Jim Jones <[email protected]>; Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Alexander Korotkov <[email protected]> writes: > Thus, I don't see the reason why this shouldn't be committed and > backpatched to PG17 (first release containing b7b0f3f27241). We are less than 48 hours from code freeze for this month's back branch releases. I think it's already too late for any inessential changes, especially if you're less than 100.00% sure of them. Please hold off until after the release cycle. regards, tom lane ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-07 16:13 Alexander Korotkov <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 0 replies; 52+ messages in thread From: Alexander Korotkov @ 2026-05-07 16:13 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Jim Jones <[email protected]>; Michael Paquier <[email protected]>; Soumya S Murali <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, Tom! On Thu, May 7, 2026 at 5:22 PM Tom Lane <[email protected]> wrote: > Alexander Korotkov <[email protected]> writes: > > Thus, I don't see the reason why this shouldn't be committed and > > backpatched to PG17 (first release containing b7b0f3f27241). > > We are less than 48 hours from code freeze for this month's back > branch releases. I think it's already too late for any inessential > changes, especially if you're less than 100.00% sure of them. > Please hold off until after the release cycle. Thank you for noticing. Yes, I'm not 100% sure new tap tests wouldn't break buildfarm. It would be pity to break it so close to the release freeze. I'll wait. ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-08 06:19 Michael Paquier <[email protected]> parent: Alexander Korotkov <[email protected]> 2 siblings, 2 replies; 52+ messages in thread From: Michael Paquier @ 2026-05-08 06:19 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Thu, May 07, 2026 at 11:04:16AM +0300, Alexander Korotkov wrote: > Let me do a quick summary: > * Our buffer manager is not capable for reading temp tables of other sessions. > * This was covered by explicit checks, but broken since b7b0f3f27241 > introduced alternative code path for reading tables. > * This doesn't apply to DROP TABLE. DROP TABLE is a conscious > exclusion and the only operation we can do correctly for other > session' temp tables. There is an explicit exclusion in the code to > skip the attempt to cleanup buffers of other session' temp tables. > * This patchset consists of tests (0001) for various operations with > other session's temp tables including buggy behavior, and the fix > (0002) including changes for tests. > > Thus, I don't see the reason why this shouldn't be committed and > backpatched to PG17 (first release containing b7b0f3f27241). > Opinions? Michael? Hmm. I don't have any counter-arguments against a backpatch based on your argument related to b7b0f3f27241. Thanks for reorganizing the patch set so as the tests happen first, and the changes in the code become second. If you wish me to look at this patch set in details, I may be able to do so around the beginning of next week. I'm not sure that there is a strong urgency in tackling this issue for this minor release, this could wait a bit more.. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-08 06:39 Alexander Korotkov <[email protected]> parent: Michael Paquier <[email protected]> 1 sibling, 0 replies; 52+ messages in thread From: Alexander Korotkov @ 2026-05-08 06:39 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Fri, May 8, 2026 at 9:19 AM Michael Paquier <[email protected]> wrote: > On Thu, May 07, 2026 at 11:04:16AM +0300, Alexander Korotkov wrote: > > Let me do a quick summary: > > * Our buffer manager is not capable for reading temp tables of other sessions. > > * This was covered by explicit checks, but broken since b7b0f3f27241 > > introduced alternative code path for reading tables. > > * This doesn't apply to DROP TABLE. DROP TABLE is a conscious > > exclusion and the only operation we can do correctly for other > > session' temp tables. There is an explicit exclusion in the code to > > skip the attempt to cleanup buffers of other session' temp tables. > > * This patchset consists of tests (0001) for various operations with > > other session's temp tables including buggy behavior, and the fix > > (0002) including changes for tests. > > > > Thus, I don't see the reason why this shouldn't be committed and > > backpatched to PG17 (first release containing b7b0f3f27241). > > Opinions? Michael? > > Hmm. I don't have any counter-arguments against a backpatch based on > your argument related to b7b0f3f27241. Thanks for reorganizing the > patch set so as the tests happen first, and the changes in the code > become second. > > If you wish me to look at this patch set in details, I may be able to > do so around the beginning of next week. I'm not sure that there is a > strong urgency in tackling this issue for this minor release, this > could wait a bit more.. Absolutely, no urgency to include it into this minor release as I already agreed [1]. You're very welcome if you could take a look in the beginning of the next week. Links. 1. https://www.postgresql.org/message-id/CAPpHfdsEr6RF-SkzVGD6PzFusENbpNPxKnRY7iQQ%3DcZQYJia6w%40mail.g... ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-14 06:12 Alexander Korotkov <[email protected]> parent: Michael Paquier <[email protected]> 1 sibling, 1 reply; 52+ messages in thread From: Alexander Korotkov @ 2026-05-14 06:12 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, Michael! On Fri, May 8, 2026 at 9:19 AM Michael Paquier <[email protected]> wrote: > > On Thu, May 07, 2026 at 11:04:16AM +0300, Alexander Korotkov wrote: > > Let me do a quick summary: > > * Our buffer manager is not capable for reading temp tables of other sessions. > > * This was covered by explicit checks, but broken since b7b0f3f27241 > > introduced alternative code path for reading tables. > > * This doesn't apply to DROP TABLE. DROP TABLE is a conscious > > exclusion and the only operation we can do correctly for other > > session' temp tables. There is an explicit exclusion in the code to > > skip the attempt to cleanup buffers of other session' temp tables. > > * This patchset consists of tests (0001) for various operations with > > other session's temp tables including buggy behavior, and the fix > > (0002) including changes for tests. > > > > Thus, I don't see the reason why this shouldn't be committed and > > backpatched to PG17 (first release containing b7b0f3f27241). > > Opinions? Michael? > > Hmm. I don't have any counter-arguments against a backpatch based on > your argument related to b7b0f3f27241. Thanks for reorganizing the > patch set so as the tests happen first, and the changes in the code > become second. > > If you wish me to look at this patch set in details, I may be able to > do so around the beginning of next week. I'm not sure that there is a > strong urgency in tackling this issue for this minor release, this > could wait a bit more.. Any news from your side? ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-14 06:58 Michael Paquier <[email protected]> parent: Alexander Korotkov <[email protected]> 0 siblings, 1 reply; 52+ messages in thread From: Michael Paquier @ 2026-05-14 06:58 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Thu, May 14, 2026 at 09:12:58AM +0300, Alexander Korotkov wrote: > On Fri, May 8, 2026 at 9:19 AM Michael Paquier <[email protected]> wrote: >> If you wish me to look at this patch set in details, I may be able to >> do so around the beginning of next week. I'm not sure that there is a >> strong urgency in tackling this issue for this minor release, this >> could wait a bit more.. > > Any news from your side? (Forgot -hackers and other folks in CC, sorry about that.) Unfortunately I have not been able to get back to it this week, and next week is moot. Perhaps it is better to not wait for me here, so feel free to go ahead as you feel. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-14 13:39 Alexander Korotkov <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 2 replies; 52+ messages in thread From: Alexander Korotkov @ 2026-05-14 13:39 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> On Thu, May 14, 2026 at 9:58 AM Michael Paquier <[email protected]> wrote: > On Thu, May 14, 2026 at 09:12:58AM +0300, Alexander Korotkov wrote: > > On Fri, May 8, 2026 at 9:19 AM Michael Paquier <[email protected]> wrote: > >> If you wish me to look at this patch set in details, I may be able to > >> do so around the beginning of next week. I'm not sure that there is a > >> strong urgency in tackling this issue for this minor release, this > >> could wait a bit more.. > > > > Any news from your side? > > (Forgot -hackers and other folks in CC, sorry about that.) > > Unfortunately I have not been able to get back to it this week, and > next week is moot. Perhaps it is better to not wait for me here, so > feel free to go ahead as you feel. Thank you for noticing. I've pushed this today. I have to slightly revise the tests to run on 18 and 17 (different log messages, and default value of log_lock_waits). ------ Regards, Alexander Korotkov Supabase ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-14 16:48 Jim Jones <[email protected]> parent: Alexander Korotkov <[email protected]> 1 sibling, 0 replies; 52+ messages in thread From: Jim Jones @ 2026-05-14 16:48 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; Michael Paquier <[email protected]>; +Cc: Daniil Davydov <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi Alexander On 14/05/2026 15:39, Alexander Korotkov wrote: > Thank you for noticing. I've pushed this today. I have to slightly > revise the tests to run on 18 and 17 (different log messages, and > default value of log_lock_waits). Awesome. Thanks for taking care of it! Best, Jim ^ permalink raw reply [nested|flat] 52+ messages in thread
* Re: Fix bug with accessing to temporary tables of other sessions @ 2026-05-14 16:48 Daniil Davydov <[email protected]> parent: Alexander Korotkov <[email protected]> 1 sibling, 0 replies; 52+ messages in thread From: Daniil Davydov @ 2026-05-14 16:48 UTC (permalink / raw) To: Alexander Korotkov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Jim Jones <[email protected]>; Soumya S Murali <[email protected]>; Tom Lane <[email protected]>; Stepan Neretin <[email protected]>; pgsql-hackers; Mohamed Ali <[email protected]>; Nazneen Jafri <[email protected]>; Shawn McCoy <[email protected]> Hi, On Thu, May 14, 2026 at 8:39 PM Alexander Korotkov <[email protected]> wrote: > > On Thu, May 14, 2026 at 9:58 AM Michael Paquier <[email protected]> wrote: > > On Thu, May 14, 2026 at 09:12:58AM +0300, Alexander Korotkov wrote: > > > On Fri, May 8, 2026 at 9:19 AM Michael Paquier <[email protected]> wrote: > > >> If you wish me to look at this patch set in details, I may be able to > > >> do so around the beginning of next week. I'm not sure that there is a > > >> strong urgency in tackling this issue for this minor release, this > > >> could wait a bit more.. > > > > > > Any news from your side? > > > > (Forgot -hackers and other folks in CC, sorry about that.) > > > > Unfortunately I have not been able to get back to it this week, and > > next week is moot. Perhaps it is better to not wait for me here, so > > feel free to go ahead as you feel. > > Thank you for noticing. I've pushed this today. I have to slightly > revise the tests to run on 18 and 17 (different log messages, and > default value of log_lock_waits). > Thank you very much for your help!) BTW, we still have another problem with temp tables. Tom wrote [1] about it within this thread: > Reality is that we cannot know whether an > unqualified-name RangeVar references a temp table until we do a > catalog lookup, so IMO we should not have a relpersistence field there > at all. At best it means something quite different from what it means > elsewhere, and that's a recipe for confusion. But changing that would > not be a bug fix (AFAIK) but refactoring to reduce the probability of > future bugs. I'll try to implement that next month. But if someone gets ahead of me, please attach me in CC of the new thread. [1] https://www.postgresql.org/message-id/4075754.1774378690%40sss.pgh.pa.us -- Best regards, Daniil Davydov ^ permalink raw reply [nested|flat] 52+ messages in thread
end of thread, other threads:[~2026-05-14 16:48 UTC | newest] Thread overview: 52+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-02-27 14:02 Re: Fix bug with accessing to temporary tables of other sessions Daniil Davydov <[email protected]> 2026-03-23 10:22 ` Soumya S Murali <[email protected]> 2026-03-24 17:26 ` Daniil Davydov <[email protected]> 2026-03-25 07:07 ` Daniil Davydov <[email protected]> 2026-04-08 09:17 ` Soumya S Murali <[email protected]> 2026-04-08 10:41 ` Jim Jones <[email protected]> 2026-04-08 14:04 ` Jim Jones <[email protected]> 2026-04-09 11:46 ` Daniil Davydov <[email protected]> 2026-04-09 14:35 ` Jim Jones <[email protected]> 2026-04-09 15:29 ` Daniil Davydov <[email protected]> 2026-04-09 17:46 ` Jim Jones <[email protected]> 2026-04-10 07:10 ` Daniil Davydov <[email protected]> 2026-04-10 10:28 ` Jim Jones <[email protected]> 2026-04-10 15:28 ` Daniil Davydov <[email protected]> 2026-04-16 05:56 ` Soumya S Murali <[email protected]> 2026-04-13 12:40 ` Soumya S Murali <[email protected]> 2026-04-13 13:56 ` Jim Jones <[email protected]> 2026-04-16 06:41 ` Soumya S Murali <[email protected]> 2026-04-20 20:07 ` Alexander Korotkov <[email protected]> 2026-04-21 05:45 ` Soumya S Murali <[email protected]> 2026-04-21 10:54 ` Alexander Korotkov <[email protected]> 2026-04-21 12:52 ` Jim Jones <[email protected]> 2026-04-21 23:41 ` Michael Paquier <[email protected]> 2026-04-24 22:09 ` Jim Jones <[email protected]> 2026-04-24 22:30 ` David G. Johnston <[email protected]> 2026-04-24 23:01 ` Jim Jones <[email protected]> 2026-04-25 13:53 ` Jim Jones <[email protected]> 2026-04-25 08:34 ` Daniil Davydov <[email protected]> 2026-05-01 19:16 ` Jim Jones <[email protected]> 2026-05-02 14:16 ` Alexander Korotkov <[email protected]> 2026-05-02 15:37 ` Daniil Davydov <[email protected]> 2026-05-02 16:34 ` Alexander Korotkov <[email protected]> 2026-05-02 17:32 ` Jim Jones <[email protected]> 2026-05-03 08:53 ` Daniil Davydov <[email protected]> 2026-05-03 12:49 ` Jim Jones <[email protected]> 2026-05-04 09:31 ` Daniil Davydov <[email protected]> 2026-05-07 08:04 ` Alexander Korotkov <[email protected]> 2026-05-07 08:43 ` Jim Jones <[email protected]> 2026-05-07 10:43 ` Alexander Korotkov <[email protected]> 2026-05-07 14:22 ` Tom Lane <[email protected]> 2026-05-07 16:13 ` Alexander Korotkov <[email protected]> 2026-05-08 06:19 ` Michael Paquier <[email protected]> 2026-05-08 06:39 ` Alexander Korotkov <[email protected]> 2026-05-14 06:12 ` Alexander Korotkov <[email protected]> 2026-05-14 06:58 ` Michael Paquier <[email protected]> 2026-05-14 13:39 ` Alexander Korotkov <[email protected]> 2026-05-14 16:48 ` Jim Jones <[email protected]> 2026-05-14 16:48 ` Daniil Davydov <[email protected]> 2026-04-21 13:17 ` Daniil Davydov <[email protected]> 2026-04-27 00:50 ` Michael Paquier <[email protected]> 2026-04-13 14:18 ` Daniil Davydov <[email protected]> 2026-04-13 12:36 ` Soumya S Murali <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox