Re: Extension tracking temp table and causing update failure - Mailing list pgsql-bugs

I wrote:
> Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:
>> Could we force temp tables created in an extension script to be ON
>> COMMIT DROP so that CurrentExtensionObject is still set and your patch
>> kicks in, preventing the DROP cascading?

> Huh, yeah, that might work.  It's ugly but at least the ugliness is
> localized; and there's no obvious reason why such a temp table ought to
> survive past the end of the script.

Actually, after I got done hacking the temp-schema case, I realized that
preventing temp tables from becoming extension members isn't so ugly as
I first thought; in fact, it's pretty much a one-liner, and much cleaner
than hacking ON COMMIT DROP.  PFA a patch that fixes both of the
temp-table issues.

            regards, tom lane

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index a8653cd49562d95748c8fe38517aa1d0785c9aff..d1d458d7fa485606f1c0e877fdd558b42f6ea273 100644
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
*************** AddNewRelationType(const char *typeName,
*** 957,966 ****
--- 957,968 ----
   *    reltablespace: OID of tablespace it goes in
   *    relid: OID to assign to new rel, or InvalidOid to select a new OID
   *    reltypeid: OID to assign to rel's rowtype, or InvalidOid to select one
+  *    reloftypeid: if a typed table, OID of underlying type; else InvalidOid
   *    ownerid: OID of new rel's owner
   *    tupdesc: tuple descriptor (source of column definitions)
   *    cooked_constraints: list of precooked check constraints and defaults
   *    relkind: relkind for new rel
+  *    relpersistence: rel's persistence status (permanent, temp, or unlogged)
   *    shared_relation: TRUE if it's to be a shared relation
   *    mapped_relation: TRUE if the relation will use the relfilenode map
   *    oidislocal: TRUE if oid column (if any) should be marked attislocal
*************** heap_create_with_catalog(const char *rel
*** 1235,1240 ****
--- 1237,1246 ----
       * should they have any ACL entries.  The same applies for extension
       * dependencies.
       *
+      * If it's a temp table, we do not make it an extension member; this
+      * prevents the unintuitive result that deletion of the temp table at
+      * session end would make the whole extension go away.
+      *
       * Also, skip this in bootstrap mode, since we don't make dependencies
       * while bootstrapping.
       */
*************** heap_create_with_catalog(const char *rel
*** 1255,1261 ****

          recordDependencyOnOwner(RelationRelationId, relid, ownerid);

!         recordDependencyOnCurrentExtension(&myself, false);

          if (reloftypeid)
          {
--- 1261,1268 ----

          recordDependencyOnOwner(RelationRelationId, relid, ownerid);

!         if (relpersistence != RELPERSISTENCE_TEMP)
!             recordDependencyOnCurrentExtension(&myself, false);

          if (reloftypeid)
          {
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 80d6fc7d0c2c721b43751cfe0d1d7233796f7d31..dc8f8eaf3f3f60f51fd8b59aa78ccfc36e1b23f9 100644
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
*************** InitTempTableNamespace(void)
*** 3558,3564 ****
           * temp tables.  This works because the places that access the temp
           * namespace for my own backend skip permissions checks on it.
           */
!         namespaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID);
          /* Advance command counter to make namespace visible */
          CommandCounterIncrement();
      }
--- 3558,3565 ----
           * temp tables.  This works because the places that access the temp
           * namespace for my own backend skip permissions checks on it.
           */
!         namespaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID,
!                                       true);
          /* Advance command counter to make namespace visible */
          CommandCounterIncrement();
      }
*************** InitTempTableNamespace(void)
*** 3582,3588 ****
      toastspaceId = get_namespace_oid(namespaceName, true);
      if (!OidIsValid(toastspaceId))
      {
!         toastspaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID);
          /* Advance command counter to make namespace visible */
          CommandCounterIncrement();
      }
--- 3583,3590 ----
      toastspaceId = get_namespace_oid(namespaceName, true);
      if (!OidIsValid(toastspaceId))
      {
!         toastspaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID,
!                                        true);
          /* Advance command counter to make namespace visible */
          CommandCounterIncrement();
      }
diff --git a/src/backend/catalog/pg_namespace.c b/src/backend/catalog/pg_namespace.c
index d5ab48a59dc76eb1eff7e6ad4229e87693ba00c2..be812a246c09dbc92f762fb50f1a6fac0e074f73 100644
*** a/src/backend/catalog/pg_namespace.c
--- b/src/backend/catalog/pg_namespace.c
***************
*** 26,35 ****

  /* ----------------
   * NamespaceCreate
   * ---------------
   */
  Oid
! NamespaceCreate(const char *nspName, Oid ownerId)
  {
      Relation    nspdesc;
      HeapTuple    tup;
--- 26,43 ----

  /* ----------------
   * NamespaceCreate
+  *
+  * Create a namespace (schema) with the given name and owner OID.
+  *
+  * If isTemp is true, this schema is a per-backend schema for holding
+  * temporary tables.  Currently, the only effect of that is to prevent it
+  * from being linked as a member of any active extension.  (If someone
+  * does CREATE TEMP TABLE in an extension script, we don't want the temp
+  * schema to become part of the extension.)
   * ---------------
   */
  Oid
! NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp)
  {
      Relation    nspdesc;
      HeapTuple    tup;
*************** NamespaceCreate(const char *nspName, Oid
*** 82,89 ****
      /* dependency on owner */
      recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);

!     /* dependency on extension */
!     recordDependencyOnCurrentExtension(&myself, false);

      /* Post creation hook for new schema */
      InvokeObjectAccessHook(OAT_POST_CREATE, NamespaceRelationId, nspoid, 0);
--- 90,98 ----
      /* dependency on owner */
      recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);

!     /* dependency on extension ... but not for magic temp schemas */
!     if (!isTemp)
!         recordDependencyOnCurrentExtension(&myself, false);

      /* Post creation hook for new schema */
      InvokeObjectAccessHook(OAT_POST_CREATE, NamespaceRelationId, nspoid, 0);
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 916328529a071d94b668f7b789a15cb179e631e4..6745af501d4680ad08d5945cb289a1af3716afb3 100644
*** a/src/backend/commands/schemacmds.c
--- b/src/backend/commands/schemacmds.c
*************** CreateSchemaCommand(CreateSchemaStmt *st
*** 94,100 ****
                              save_sec_context | SECURITY_LOCAL_USERID_CHANGE);

      /* Create the schema's namespace */
!     namespaceId = NamespaceCreate(schemaName, owner_uid);

      /* Advance cmd counter to make the namespace visible */
      CommandCounterIncrement();
--- 94,100 ----
                              save_sec_context | SECURITY_LOCAL_USERID_CHANGE);

      /* Create the schema's namespace */
!     namespaceId = NamespaceCreate(schemaName, owner_uid, false);

      /* Advance cmd counter to make the namespace visible */
      CommandCounterIncrement();
diff --git a/src/include/catalog/pg_namespace.h b/src/include/catalog/pg_namespace.h
index aad76a1452aae126c468553bec752182a980ff12..1daba477b409a453bfbe19885fd57455c56245cc 100644
*** a/src/include/catalog/pg_namespace.h
--- b/src/include/catalog/pg_namespace.h
*************** DESCR("standard public schema");
*** 79,84 ****
  /*
   * prototypes for functions in pg_namespace.c
   */
! extern Oid    NamespaceCreate(const char *nspName, Oid ownerId);

  #endif   /* PG_NAMESPACE_H */
--- 79,84 ----
  /*
   * prototypes for functions in pg_namespace.c
   */
! extern Oid    NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp);

  #endif   /* PG_NAMESPACE_H */