Hi, On Thu, Sep 25, 2025 at 5:45 PM Jim Jones <jim.jo...@uni-muenster.de> wrote: > > A few days ago I reviewed one patch[1] that has a significant overlap > with this one. Perhaps they should be merged? >
Thanks for looking into it! I don't know what exactly is meant by merging. Maybe we should just apply a current patch that fixes all problems ?.. > Here my first tests and comments: > .... OK, I'll replace "could not" with "cannot" in order to match previous comments. > > DROP TABLE still works, but I guess it is the main motivation of > RVR_OTHER_TEMP_OK :) Yep, motivation of this decision you can find here [1]. I'll attach a v8 patch that fixes error messages (could not -> cannot). -- Best regards, Daniil Davydov [1] https://www.postgresql.org/message-id/Zx7oLCnqis3FjgCK%40paquier.xyz
From 28250951d1d6888945850aee05f128f590d88c6f Mon Sep 17 00:00:00 2001 From: Daniil Davidov <d.davy...@postgrespro.ru> Date: Thu, 25 Sep 2025 20:06:40 +0700 Subject: [PATCH v8] 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 ed9aeee24bc..6b835dc45c8 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -498,28 +498,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 not 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) { @@ -3620,21 +3636,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, @@ -3841,6 +3855,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 fc89352b661..aa4a80f3f29 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1624,7 +1624,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 e2d9e9be41a..62edf24b5c2 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 9fd48acb1f8..36f2f44cabe 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -19406,7 +19406,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; @@ -19446,6 +19450,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 f1423f28c32..b08d55f2c97 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