Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ad05b5d

Browse files
committed
Fix some issues with temp/transient tables in extension scripts.
Phil Sorber reported that a rewriting ALTER TABLE within an extension update script failed, because it creates and then drops a placeholder table; the drop was being disallowed because the table was marked as an extension member. We could hack that specific case but it seems likely that there might be related cases now or in the future, so the most practical solution seems to be to create an exception to the general rule that extension member objects can only be dropped by dropping the owning extension. To wit: if the DROP is issued within the extension's own creation or update scripts, we'll allow it, implicitly performing an "ALTER EXTENSION DROP object" first. This will simplify cases such as extension downgrade scripts anyway. No docs change since we don't seem to have documented the idea that you would need ALTER EXTENSION DROP for such an action to begin with. Also, arrange for explicitly temporary tables to not get linked as extension members in the first place, and the same for the magic pg_temp_nnn schemas that are created to hold them. This prevents assorted unpleasant results if an extension script creates a temp table: the forced drop at session end would either fail or remove the entire extension, and neither of those outcomes is desirable. Note that this doesn't fix the ALTER TABLE scenario, since the placeholder table is not temp (unless the table being rewritten is). Back-patch to 9.1.
1 parent 610fd34 commit ad05b5d

File tree

6 files changed

+51
-14
lines changed

6 files changed

+51
-14
lines changed

src/backend/catalog/dependency.c

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -549,17 +549,21 @@ findDependentObjects(const ObjectAddress *object,
549549
* another object, or is part of the extension that is the
550550
* other object. We have three cases:
551551
*
552-
* 1. At the outermost recursion level, disallow the DROP. (We
553-
* just ereport here, rather than proceeding, since no other
554-
* dependencies are likely to be interesting.) However, if
555-
* the owning object is listed in pendingObjects, just release
556-
* the caller's lock and return; we'll eventually complete the
557-
* DROP when we reach that entry in the pending list.
552+
* 1. At the outermost recursion level, we normally disallow
553+
* the DROP. (We just ereport here, rather than proceeding,
554+
* since no other dependencies are likely to be interesting.)
555+
* However, there are exceptions.
558556
*/
559557
if (stack == NULL)
560558
{
561559
char *otherObjDesc;
562560

561+
/*
562+
* Exception 1a: if the owning object is listed in
563+
* pendingObjects, just release the caller's lock and
564+
* return. We'll eventually complete the DROP when we
565+
* reach that entry in the pending list.
566+
*/
563567
if (pendingObjects &&
564568
object_address_present(&otherObject, pendingObjects))
565569
{
@@ -568,6 +572,21 @@ findDependentObjects(const ObjectAddress *object,
568572
ReleaseDeletionLock(object);
569573
return;
570574
}
575+
576+
/*
577+
* Exception 1b: if the owning object is the extension
578+
* currently being created/altered, it's okay to continue
579+
* with the deletion. This allows dropping of an
580+
* extension's objects within the extension's scripts,
581+
* as well as corner cases such as dropping a transient
582+
* object created within such a script.
583+
*/
584+
if (creating_extension &&
585+
otherObject.classId == ExtensionRelationId &&
586+
otherObject.objectId == CurrentExtensionObject)
587+
break;
588+
589+
/* No exception applies, so throw the error */
571590
otherObjDesc = getObjectDescription(&otherObject);
572591
ereport(ERROR,
573592
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),

src/backend/catalog/heap.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -945,10 +945,12 @@ AddNewRelationType(const char *typeName,
945945
* reltablespace: OID of tablespace it goes in
946946
* relid: OID to assign to new rel, or InvalidOid to select a new OID
947947
* reltypeid: OID to assign to rel's rowtype, or InvalidOid to select one
948+
* reloftypeid: if a typed table, OID of underlying type; else InvalidOid
948949
* ownerid: OID of new rel's owner
949950
* tupdesc: tuple descriptor (source of column definitions)
950951
* cooked_constraints: list of precooked check constraints and defaults
951952
* relkind: relkind for new rel
953+
* relpersistence: rel's persistence status (permanent, temp, or unlogged)
952954
* shared_relation: TRUE if it's to be a shared relation
953955
* mapped_relation: TRUE if the relation will use the relfilenode map
954956
* oidislocal: TRUE if oid column (if any) should be marked attislocal
@@ -1222,6 +1224,10 @@ heap_create_with_catalog(const char *relname,
12221224
* should they have any ACL entries. The same applies for extension
12231225
* dependencies.
12241226
*
1227+
* If it's a temp table, we do not make it an extension member; this
1228+
* prevents the unintuitive result that deletion of the temp table at
1229+
* session end would make the whole extension go away.
1230+
*
12251231
* Also, skip this in bootstrap mode, since we don't make dependencies
12261232
* while bootstrapping.
12271233
*/
@@ -1242,7 +1248,8 @@ heap_create_with_catalog(const char *relname,
12421248

12431249
recordDependencyOnOwner(RelationRelationId, relid, ownerid);
12441250

1245-
recordDependencyOnCurrentExtension(&myself, false);
1251+
if (relpersistence != RELPERSISTENCE_TEMP)
1252+
recordDependencyOnCurrentExtension(&myself, false);
12461253

12471254
if (reloftypeid)
12481255
{

src/backend/catalog/namespace.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3325,7 +3325,8 @@ InitTempTableNamespace(void)
33253325
* temp tables. This works because the places that access the temp
33263326
* namespace for my own backend skip permissions checks on it.
33273327
*/
3328-
namespaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID);
3328+
namespaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID,
3329+
true);
33293330
/* Advance command counter to make namespace visible */
33303331
CommandCounterIncrement();
33313332
}
@@ -3349,7 +3350,8 @@ InitTempTableNamespace(void)
33493350
toastspaceId = get_namespace_oid(namespaceName, true);
33503351
if (!OidIsValid(toastspaceId))
33513352
{
3352-
toastspaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID);
3353+
toastspaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID,
3354+
true);
33533355
/* Advance command counter to make namespace visible */
33543356
CommandCounterIncrement();
33553357
}

src/backend/catalog/pg_namespace.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,18 @@
2626

2727
/* ----------------
2828
* NamespaceCreate
29+
*
30+
* Create a namespace (schema) with the given name and owner OID.
31+
*
32+
* If isTemp is true, this schema is a per-backend schema for holding
33+
* temporary tables. Currently, the only effect of that is to prevent it
34+
* from being linked as a member of any active extension. (If someone
35+
* does CREATE TEMP TABLE in an extension script, we don't want the temp
36+
* schema to become part of the extension.)
2937
* ---------------
3038
*/
3139
Oid
32-
NamespaceCreate(const char *nspName, Oid ownerId)
40+
NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp)
3341
{
3442
Relation nspdesc;
3543
HeapTuple tup;
@@ -82,8 +90,9 @@ NamespaceCreate(const char *nspName, Oid ownerId)
8290
/* dependency on owner */
8391
recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);
8492

85-
/* dependency on extension */
86-
recordDependencyOnCurrentExtension(&myself, false);
93+
/* dependency on extension ... but not for magic temp schemas */
94+
if (!isTemp)
95+
recordDependencyOnCurrentExtension(&myself, false);
8796

8897
/* Post creation hook for new schema */
8998
InvokeObjectAccessHook(OAT_POST_CREATE, NamespaceRelationId, nspoid, 0);

src/backend/commands/schemacmds.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
9595
save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
9696

9797
/* Create the schema's namespace */
98-
namespaceId = NamespaceCreate(schemaName, owner_uid);
98+
namespaceId = NamespaceCreate(schemaName, owner_uid, false);
9999

100100
/* Advance cmd counter to make the namespace visible */
101101
CommandCounterIncrement();

src/include/catalog/pg_namespace.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,6 @@ DESCR("standard public schema");
7777
/*
7878
* prototypes for functions in pg_namespace.c
7979
*/
80-
extern Oid NamespaceCreate(const char *nspName, Oid ownerId);
80+
extern Oid NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp);
8181

8282
#endif /* PG_NAMESPACE_H */

0 commit comments

Comments
 (0)