Hi,
On Mon, Mar 17, 2025 at 10:09 PM David G. Johnston
<david.g.johns...@gmail.com> wrote:
>
> On Monday, March 17, 2025, Daniil Davydov <3daniss...@gmail.com> wrote:
>>
>>
>> >
>> > 2."you have not any temporary relations" --> "you have no any temporary
>> > relations"
>> I am not an English speaker, but it seems that "have not" would be
>> more correct. Someone has to judge us :)
>>
>
> Both are not good.
>
> “pg_temp was specified but it contains no relations” [1]
That sounds reasonable. I'll fix it. Thanks!
> But why are we promoting this situation to an error? It should be a relation
> not found error just like any other and not its own special case.
> The fact we create pg_temp only as it is needed is an implementation detail
> that should not be visible to the user.
1)
Error message has no mention of a non-existent schema. "Schema has no
relations" => "Given relation not found in empty schema". It seems to
me that these are equivalent statements.
2)
Is this really the implementation detail that we want to hide from the
user? User can just run "select pg_my_temp_schema();" and see that
there is no temp schema in the current session.
Don't get me wrong - I can agree with that, but for now it seems odd to me...
Steven Niu also mentioned this issue, but IMO we must give the most
accurate description of the problem - tell "relation not found" only
if we have temp namespace, but not specified relation in it.
Please see v4 patch (only comment change).
--
Best regards,
Daniil Davydov
From 33e2b77607c26bb82a807711ecdec5174be64874 Mon Sep 17 00:00:00 2001
From: Daniil Davidov <d.davy...@postgrespro.ru>
Date: Mon, 17 Mar 2025 15:43:15 +0700
Subject: [PATCH v4] Fix accessing other sessions temp tables
---
src/backend/catalog/namespace.c | 71 +++++++++++++++++++--------------
src/backend/nodes/makefuncs.c | 6 ++-
src/backend/parser/gram.y | 11 ++++-
3 files changed, 57 insertions(+), 31 deletions(-)
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index d97d632a7ef..9e96189920d 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -499,28 +499,40 @@ 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 (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 (consider adding GUC for to allow to DROP such
+ * tables?).
*/
- if (namespaceId != myTempNamespace)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("temporary tables cannot specify a schema name")));
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("could 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)
{
@@ -3387,17 +3399,18 @@ LookupExplicitNamespace(const char *nspname, bool missing_ok)
Oid namespaceId;
AclResult aclresult;
- /* check for pg_temp alias */
+ /*
+ * If namespace specified in 'pg_temp' format (without owner backend id
+ * specification), we assume that this is our temporary namespace.
+ */
if (strcmp(nspname, "pg_temp") == 0)
{
- if (OidIsValid(myTempNamespace))
- return myTempNamespace;
+ if (!OidIsValid(myTempNamespace))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_SCHEMA_NAME),
+ errmsg("pg_temp was specified but it contains no relations")));
- /*
- * Since this is used only for looking up existing objects, there is
- * no point in trying to initialize the temp namespace here; and doing
- * so might create problems for some callers --- just fall through.
- */
+ return myTempNamespace;
}
namespaceId = get_namespace_oid(nspname, missing_ok);
@@ -3553,21 +3566,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,
@@ -3774,6 +3785,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/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 271ae26cbaf..f68948d475c 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -19424,7 +19424,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;
@@ -19464,6 +19468,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;
}
--
2.43.0
