Thread: ALTER DEFAULT PRIVILEGES is buggy, and so is its testing

ALTER DEFAULT PRIVILEGES is buggy, and so is its testing

From
Tom Lane
Date:
Buildfarm members crake and xenodermus recently fell over with
very similar symptoms in the pg_upgrade test:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2018-11-07%2021%3A47%3A29

pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 6350; 0 0 ACL SCHEMA "regress_rls_schema" andrew
pg_restore: [archiver (db)] could not execute query: ERROR:  role "33757" does not exist
    Command was: GRANT ALL ON SCHEMA "regress_rls_schema" TO "33757";

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=xenodermus&dt=2018-11-08%2023%3A00%3A01

pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 5357; 0 0 ACL SCHEMA "mvtest_mvschema" bf
pg_restore: [archiver (db)] could not execute query: ERROR:  role "33894" does not exist
    Command was: GRANT ALL ON SCHEMA "mvtest_mvschema" TO "33894";


The bogus numeric user "names" are presumably there because aclitemout
will just fall back to printing a referenced role's OID if it fails to
look up the role's name; hence, what we're looking at here is grants
made to since-dropped roles, which somehow didn't get cleaned up.

Now, looking at the regression test scripts that create these particular
schemas, that's rather mind-boggling: they never do any grant or revoke
at all on those schemas, so how'd there come to be any ACL entries?

I think that the answer is that these particular scripts run in parallel
with the "privileges" script, in which we find this:

ALTER DEFAULT PRIVILEGES GRANT ALL ON SCHEMAS TO regress_priv_user2;

What seems to be happening is

1. Given the right concurrent timing, these schemas absorb a GRANT
to regress_priv_user2 during the window in which the above is active.

2. The GRANT apparently doesn't get entered into pg_shdepend, because
when privileges.sql drops the role, there is no complaint.  (It's easy
to reproduce this bug without any parallelism.)

3. The regression tests proper don't notice the dangling ACL, but
dump/restore sure does.

The reason we are just seeing this now, no doubt, is Andres having
changed the pg_upgrade test to run the core regression tests in
parallel.  I'm sure it's happened a whole lot before without anyone
noticing.

So it seems like testing ALTER DEFAULT PRIVILEGES in a script that
runs in parallel with anything else is a damfool idea.  But there
is also a live bug of failure to correctly record privileges granted
this way.  Without that bug, we'd have had our noses rubbed in the
parallel race conditions long ago.

            regards, tom lane


Re: ALTER DEFAULT PRIVILEGES is buggy, and so is its testing

From
Tom Lane
Date:
I wrote:
> So it seems like testing ALTER DEFAULT PRIVILEGES in a script that
> runs in parallel with anything else is a damfool idea.  But there
> is also a live bug of failure to correctly record privileges granted
> this way.  Without that bug, we'd have had our noses rubbed in the
> parallel race conditions long ago.

It turns out the bug only applies to schemas and types; the other
callers of get_user_default_acl() already had ad-hoc code to deal
with the problem.  I made that a bit less ad-hoc by creating a
subroutine to do the work and documenting the need to call it.

To fix the tests' race condition, it seems to be sufficient to
wrap a transaction around the section of privileges.sql that
grants and then revokes global default privileges.  The other
uses of ALTER DEFAULT PRIVILEGES in the tests are targeted narrowly
enough to ensure that concurrent tests shouldn't pick them up.

In the attached proposed patch for HEAD, since I had to adjust
the API of GenerateTypeDependencies anyway, I changed it to pass
the new pg_type row and get most of the old arguments out of that,
as the argument list was getting pretty unwieldy.  I suppose we
could make a back-branch patch that doesn't change that API and
instead makes the callers responsible for recording ACL dependencies,
but ugh.  Does anyone think it's likely that third-party code is
calling GenerateTypeDependencies?

            regards, tom lane

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 578e4c6..bd14775 100644
*** a/src/backend/catalog/aclchk.c
--- b/src/backend/catalog/aclchk.c
*************** get_default_acl_internal(Oid roleId, Oid
*** 5445,5451 ****
  /*
   * Get default permissions for newly created object within given schema
   *
!  * Returns NULL if built-in system defaults should be used
   */
  Acl *
  get_user_default_acl(ObjectType objtype, Oid ownerId, Oid nsp_oid)
--- 5445,5454 ----
  /*
   * Get default permissions for newly created object within given schema
   *
!  * Returns NULL if built-in system defaults should be used.
!  *
!  * If the result is not NULL, caller must call recordDependencyOnNewAcl
!  * once the OID of the new object is known.
   */
  Acl *
  get_user_default_acl(ObjectType objtype, Oid ownerId, Oid nsp_oid)
*************** get_user_default_acl(ObjectType objtype,
*** 5521,5526 ****
--- 5524,5553 ----
  }

  /*
+  * Record dependencies on roles mentioned in a new object's ACL.
+  */
+ void
+ recordDependencyOnNewAcl(Oid classId, Oid objectId, int32 objsubId,
+                          Oid ownerId, Acl *acl)
+ {
+     int            nmembers;
+     Oid           *members;
+
+     /* Nothing to do if ACL is defaulted */
+     if (acl == NULL)
+         return;
+
+     /* Extract roles mentioned in ACL */
+     nmembers = aclmembers(acl, &members);
+
+     /* Update the shared dependency ACL info */
+     updateAclDependencies(classId, objectId, objsubId,
+                           ownerId,
+                           0, NULL,
+                           nmembers, members);
+ }
+
+ /*
   * Record initial privileges for the top-level object passed in.
   *
   * For the object passed in, this will record its ACL (if any) and the ACLs of
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 7203c86..bd4c439 100644
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
*************** heap_create_with_catalog(const char *rel
*** 1375,1380 ****
--- 1375,1381 ----
          myself.classId = RelationRelationId;
          myself.objectId = relid;
          myself.objectSubId = 0;
+
          referenced.classId = NamespaceRelationId;
          referenced.objectId = relnamespace;
          referenced.objectSubId = 0;
*************** heap_create_with_catalog(const char *rel
*** 1382,1387 ****
--- 1383,1390 ----

          recordDependencyOnOwner(RelationRelationId, relid, ownerid);

+         recordDependencyOnNewAcl(RelationRelationId, relid, 0, ownerid, relacl);
+
          recordDependencyOnCurrentExtension(&myself, false);

          if (reloftypeid)
*************** heap_create_with_catalog(const char *rel
*** 1391,1408 ****
              referenced.objectSubId = 0;
              recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
          }
-
-         if (relacl != NULL)
-         {
-             int            nnewmembers;
-             Oid           *newmembers;
-
-             nnewmembers = aclmembers(relacl, &newmembers);
-             updateAclDependencies(RelationRelationId, relid, 0,
-                                   ownerid,
-                                   0, NULL,
-                                   nnewmembers, newmembers);
-         }
      }

      /* Post creation hook for new relation */
--- 1394,1399 ----
diff --git a/src/backend/catalog/pg_namespace.c b/src/backend/catalog/pg_namespace.c
index 2cf52be..0538e31 100644
*** a/src/backend/catalog/pg_namespace.c
--- b/src/backend/catalog/pg_namespace.c
*************** NamespaceCreate(const char *nspName, Oid
*** 100,105 ****
--- 100,108 ----
      /* dependency on owner */
      recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);

+     /* dependences on roles mentioned in default ACL */
+     recordDependencyOnNewAcl(NamespaceRelationId, nspoid, 0, ownerId, nspacl);
+
      /* dependency on extension ... but not for magic temp schemas */
      if (!isTemp)
          recordDependencyOnCurrentExtension(&myself, false);
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 0c81704..e367da7 100644
*** a/src/backend/catalog/pg_proc.c
--- b/src/backend/catalog/pg_proc.c
*************** ProcedureCreate(const char *procedureNam
*** 654,670 ****
          recordDependencyOnOwner(ProcedureRelationId, retval, proowner);

      /* dependency on any roles mentioned in ACL */
!     if (!is_update && proacl != NULL)
!     {
!         int            nnewmembers;
!         Oid           *newmembers;
!
!         nnewmembers = aclmembers(proacl, &newmembers);
!         updateAclDependencies(ProcedureRelationId, retval, 0,
!                               proowner,
!                               0, NULL,
!                               nnewmembers, newmembers);
!     }

      /* dependency on extension */
      recordDependencyOnCurrentExtension(&myself, is_update);
--- 654,662 ----
          recordDependencyOnOwner(ProcedureRelationId, retval, proowner);

      /* dependency on any roles mentioned in ACL */
!     if (!is_update)
!         recordDependencyOnNewAcl(ProcedureRelationId, retval, 0,
!                                  proowner, proacl);

      /* dependency on extension */
      recordDependencyOnCurrentExtension(&myself, is_update);
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 2ddd46d..0b82b18 100644
*** a/src/backend/catalog/pg_type.c
--- b/src/backend/catalog/pg_type.c
*************** TypeShellMake(const char *typeName, Oid
*** 147,169 ****
       * Create dependencies.  We can/must skip this in bootstrap mode.
       */
      if (!IsBootstrapProcessingMode())
!         GenerateTypeDependencies(typeNamespace,
!                                  typoid,
!                                  InvalidOid,
                                   0,
-                                  ownerId,
-                                  F_SHELL_IN,
-                                  F_SHELL_OUT,
-                                  InvalidOid,
-                                  InvalidOid,
-                                  InvalidOid,
-                                  InvalidOid,
-                                  InvalidOid,
-                                  InvalidOid,
                                   false,
-                                  InvalidOid,
-                                  InvalidOid,
-                                  NULL,
                                   false);

      /* Post creation hook for new shell type */
--- 147,158 ----
       * Create dependencies.  We can/must skip this in bootstrap mode.
       */
      if (!IsBootstrapProcessingMode())
!         GenerateTypeDependencies(typoid,
!                                  (Form_pg_type) GETSTRUCT(tup),
!                                  NULL,
!                                  NULL,
                                   0,
                                   false,
                                   false);

      /* Post creation hook for new shell type */
*************** TypeCreate(Oid newTypeOid,
*** 379,384 ****
--- 368,376 ----
      else
          nulls[Anum_pg_type_typdefault - 1] = true;

+     /*
+      * initialize the type's ACL, too.
+      */
      typacl = get_user_default_acl(OBJECT_TYPE, ownerId,
                                    typeNamespace);
      if (typacl != NULL)
*************** TypeCreate(Oid newTypeOid,
*** 462,486 ****
       * Create dependencies.  We can/must skip this in bootstrap mode.
       */
      if (!IsBootstrapProcessingMode())
!         GenerateTypeDependencies(typeNamespace,
!                                  typeObjectId,
!                                  relationOid,
!                                  relationKind,
!                                  ownerId,
!                                  inputProcedure,
!                                  outputProcedure,
!                                  receiveProcedure,
!                                  sendProcedure,
!                                  typmodinProcedure,
!                                  typmodoutProcedure,
!                                  analyzeProcedure,
!                                  elementType,
!                                  isImplicitArray,
!                                  baseType,
!                                  typeCollation,
                                   (defaultTypeBin ?
                                    stringToNode(defaultTypeBin) :
                                    NULL),
                                   rebuildDeps);

      /* Post creation hook for new type */
--- 454,467 ----
       * Create dependencies.  We can/must skip this in bootstrap mode.
       */
      if (!IsBootstrapProcessingMode())
!         GenerateTypeDependencies(typeObjectId,
!                                  (Form_pg_type) GETSTRUCT(tup),
                                   (defaultTypeBin ?
                                    stringToNode(defaultTypeBin) :
                                    NULL),
+                                  typacl,
+                                  relationKind,
+                                  isImplicitArray,
                                   rebuildDeps);

      /* Post creation hook for new type */
*************** TypeCreate(Oid newTypeOid,
*** 499,504 ****
--- 480,493 ----
  /*
   * GenerateTypeDependencies: build the dependencies needed for a type
   *
+  * Most of what this function needs to know about the type is passed as the
+  * new pg_type row, typeForm.  But we can't get at the varlena fields through
+  * that, so defaultExpr and typacl are passed separately.  (typacl is really
+  * "Acl *", but we declare it "void *" to avoid including acl.h in pg_type.h.)
+  *
+  * relationKind and isImplicitArray aren't visible in the pg_type row either,
+  * so they're also passed separately.
+  *
   * If rebuild is true, we remove existing dependencies and rebuild them
   * from scratch.  This is needed for ALTER TYPE, and also when replacing
   * a shell type.  We don't remove an existing extension dependency, though.
*************** TypeCreate(Oid newTypeOid,
*** 508,530 ****
   * that type will become a member of the extension.)
   */
  void
! GenerateTypeDependencies(Oid typeNamespace,
!                          Oid typeObjectId,
!                          Oid relationOid,    /* only for relation rowtypes */
!                          char relationKind, /* ditto */
!                          Oid owner,
!                          Oid inputProcedure,
!                          Oid outputProcedure,
!                          Oid receiveProcedure,
!                          Oid sendProcedure,
!                          Oid typmodinProcedure,
!                          Oid typmodoutProcedure,
!                          Oid analyzeProcedure,
!                          Oid elementType,
!                          bool isImplicitArray,
!                          Oid baseType,
!                          Oid typeCollation,
                           Node *defaultExpr,
                           bool rebuild)
  {
      ObjectAddress myself,
--- 497,508 ----
   * that type will become a member of the extension.)
   */
  void
! GenerateTypeDependencies(Oid typeObjectId,
!                          Form_pg_type typeForm,
                           Node *defaultExpr,
+                          void *typacl,
+                          char relationKind, /* only for relation rowtypes */
+                          bool isImplicitArray,
                           bool rebuild)
  {
      ObjectAddress myself,
*************** GenerateTypeDependencies(Oid typeNamespa
*** 542,620 ****
      myself.objectSubId = 0;

      /*
!      * Make dependencies on namespace, owner, extension.
       *
       * For a relation rowtype (that's not a composite type), we should skip
       * these because we'll depend on them indirectly through the pg_class
       * entry.  Likewise, skip for implicit arrays since we'll depend on them
       * through the element type.
       */
!     if ((!OidIsValid(relationOid) || relationKind == RELKIND_COMPOSITE_TYPE) &&
          !isImplicitArray)
      {
          referenced.classId = NamespaceRelationId;
!         referenced.objectId = typeNamespace;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);

!         recordDependencyOnOwner(TypeRelationId, typeObjectId, owner);

          recordDependencyOnCurrentExtension(&myself, rebuild);
      }

      /* Normal dependencies on the I/O functions */
!     if (OidIsValid(inputProcedure))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = inputProcedure;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(outputProcedure))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = outputProcedure;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(receiveProcedure))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = receiveProcedure;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(sendProcedure))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = sendProcedure;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typmodinProcedure))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typmodinProcedure;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typmodoutProcedure))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typmodoutProcedure;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(analyzeProcedure))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = analyzeProcedure;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }
--- 520,603 ----
      myself.objectSubId = 0;

      /*
!      * Make dependencies on namespace, owner, ACL, extension.
       *
       * For a relation rowtype (that's not a composite type), we should skip
       * these because we'll depend on them indirectly through the pg_class
       * entry.  Likewise, skip for implicit arrays since we'll depend on them
       * through the element type.
       */
!     if ((!OidIsValid(typeForm->typrelid) ||
!          relationKind == RELKIND_COMPOSITE_TYPE) &&
          !isImplicitArray)
      {
          referenced.classId = NamespaceRelationId;
!         referenced.objectId = typeForm->typnamespace;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);

!         recordDependencyOnOwner(TypeRelationId, typeObjectId,
!                                 typeForm->typowner);
!
!         recordDependencyOnNewAcl(TypeRelationId, typeObjectId, 0,
!                                  typeForm->typowner, typacl);

          recordDependencyOnCurrentExtension(&myself, rebuild);
      }

      /* Normal dependencies on the I/O functions */
!     if (OidIsValid(typeForm->typinput))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typeForm->typinput;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typeForm->typoutput))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typeForm->typoutput;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typeForm->typreceive))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typeForm->typreceive;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typeForm->typsend))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typeForm->typsend;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typeForm->typmodin))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typeForm->typmodin;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typeForm->typmodout))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typeForm->typmodout;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

!     if (OidIsValid(typeForm->typanalyze))
      {
          referenced.classId = ProcedureRelationId;
!         referenced.objectId = typeForm->typanalyze;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }
*************** GenerateTypeDependencies(Oid typeNamespa
*** 628,637 ****
       * relation is, and not otherwise. And in the latter, of course we get the
       * opposite effect.
       */
!     if (OidIsValid(relationOid))
      {
          referenced.classId = RelationRelationId;
!         referenced.objectId = relationOid;
          referenced.objectSubId = 0;

          if (relationKind != RELKIND_COMPOSITE_TYPE)
--- 611,620 ----
       * relation is, and not otherwise. And in the latter, of course we get the
       * opposite effect.
       */
!     if (OidIsValid(typeForm->typrelid))
      {
          referenced.classId = RelationRelationId;
!         referenced.objectId = typeForm->typrelid;
          referenced.objectSubId = 0;

          if (relationKind != RELKIND_COMPOSITE_TYPE)
*************** GenerateTypeDependencies(Oid typeNamespa
*** 645,674 ****
       * dependent on the element type.  Otherwise, if it has an element type,
       * the dependency is a normal one.
       */
!     if (OidIsValid(elementType))
      {
          referenced.classId = TypeRelationId;
!         referenced.objectId = elementType;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced,
                             isImplicitArray ? DEPENDENCY_INTERNAL : DEPENDENCY_NORMAL);
      }

      /* Normal dependency from a domain to its base type. */
!     if (OidIsValid(baseType))
      {
          referenced.classId = TypeRelationId;
!         referenced.objectId = baseType;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

      /* Normal dependency from a domain to its collation. */
      /* We know the default collation is pinned, so don't bother recording it */
!     if (OidIsValid(typeCollation) && typeCollation != DEFAULT_COLLATION_OID)
      {
          referenced.classId = CollationRelationId;
!         referenced.objectId = typeCollation;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }
--- 628,658 ----
       * dependent on the element type.  Otherwise, if it has an element type,
       * the dependency is a normal one.
       */
!     if (OidIsValid(typeForm->typelem))
      {
          referenced.classId = TypeRelationId;
!         referenced.objectId = typeForm->typelem;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced,
                             isImplicitArray ? DEPENDENCY_INTERNAL : DEPENDENCY_NORMAL);
      }

      /* Normal dependency from a domain to its base type. */
!     if (OidIsValid(typeForm->typbasetype))
      {
          referenced.classId = TypeRelationId;
!         referenced.objectId = typeForm->typbasetype;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }

      /* Normal dependency from a domain to its collation. */
      /* We know the default collation is pinned, so don't bother recording it */
!     if (OidIsValid(typeForm->typcollation) &&
!         typeForm->typcollation != DEFAULT_COLLATION_OID)
      {
          referenced.classId = CollationRelationId;
!         referenced.objectId = typeForm->typcollation;
          referenced.objectSubId = 0;
          recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
      }
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 1238800..74dd534 100644
*** a/src/backend/commands/typecmds.c
--- b/src/backend/commands/typecmds.c
*************** AlterDomainDefault(List *names, Node *de
*** 2179,2184 ****
--- 2179,2187 ----
      Relation    rel;
      char       *defaultValue;
      Node       *defaultExpr = NULL; /* NULL if no default specified */
+     Acl           *typacl;
+     Datum        aclDatum;
+     bool        isNull;
      Datum        new_record[Natts_pg_type];
      bool        new_record_nulls[Natts_pg_type];
      bool        new_record_repl[Natts_pg_type];
*************** AlterDomainDefault(List *names, Node *de
*** 2270,2293 ****

      CatalogTupleUpdate(rel, &tup->t_self, newtuple);

      /* Rebuild dependencies */
!     GenerateTypeDependencies(typTup->typnamespace,
!                              domainoid,
!                              InvalidOid,    /* typrelid is n/a */
                               0, /* relation kind is n/a */
-                              typTup->typowner,
-                              typTup->typinput,
-                              typTup->typoutput,
-                              typTup->typreceive,
-                              typTup->typsend,
-                              typTup->typmodin,
-                              typTup->typmodout,
-                              typTup->typanalyze,
-                              InvalidOid,
                               false, /* a domain isn't an implicit array */
-                              typTup->typbasetype,
-                              typTup->typcollation,
-                              defaultExpr,
                               true); /* Rebuild is true */

      InvokeObjectPostAlterHook(TypeRelationId, domainoid, 0);
--- 2273,2293 ----

      CatalogTupleUpdate(rel, &tup->t_self, newtuple);

+     /* Must extract ACL for use of GenerateTypeDependencies */
+     aclDatum = heap_getattr(newtuple, Anum_pg_type_typacl,
+                             RelationGetDescr(rel), &isNull);
+     if (isNull)
+         typacl = NULL;
+     else
+         typacl = DatumGetAclPCopy(aclDatum);
+
      /* Rebuild dependencies */
!     GenerateTypeDependencies(domainoid,
!                              (Form_pg_type) GETSTRUCT(newtuple),
!                              defaultExpr,
!                              typacl,
                               0, /* relation kind is n/a */
                               false, /* a domain isn't an implicit array */
                               true); /* Rebuild is true */

      InvokeObjectPostAlterHook(TypeRelationId, domainoid, 0);
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index 76f3297..57595a6 100644
*** a/src/include/catalog/pg_type.h
--- b/src/include/catalog/pg_type.h
*************** extern ObjectAddress TypeCreate(Oid newT
*** 323,345 ****
             bool typeNotNull,
             Oid typeCollation);

! extern void GenerateTypeDependencies(Oid typeNamespace,
!                          Oid typeObjectId,
!                          Oid relationOid,
                           char relationKind,
-                          Oid owner,
-                          Oid inputProcedure,
-                          Oid outputProcedure,
-                          Oid receiveProcedure,
-                          Oid sendProcedure,
-                          Oid typmodinProcedure,
-                          Oid typmodoutProcedure,
-                          Oid analyzeProcedure,
-                          Oid elementType,
                           bool isImplicitArray,
-                          Oid baseType,
-                          Oid typeCollation,
-                          Node *defaultExpr,
                           bool rebuild);

  extern void RenameTypeInternal(Oid typeOid, const char *newTypeName,
--- 323,334 ----
             bool typeNotNull,
             Oid typeCollation);

! extern void GenerateTypeDependencies(Oid typeObjectId,
!                          Form_pg_type typeForm,
!                          Node *defaultExpr,
!                          void *typacl,
                           char relationKind,
                           bool isImplicitArray,
                           bool rebuild);

  extern void RenameTypeInternal(Oid typeOid, const char *newTypeName,
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index f4d4be8..72936ee 100644
*** a/src/include/utils/acl.h
--- b/src/include/utils/acl.h
*************** typedef enum
*** 189,194 ****
--- 189,196 ----
  extern Acl *acldefault(ObjectType objtype, Oid ownerId);
  extern Acl *get_user_default_acl(ObjectType objtype, Oid ownerId,
                       Oid nsp_oid);
+ extern void recordDependencyOnNewAcl(Oid classId, Oid objectId, int32 objsubId,
+                          Oid ownerId, Acl *acl);

  extern Acl *aclupdate(const Acl *old_acl, const AclItem *mod_aip,
            int modechg, Oid ownerId, DropBehavior behavior);
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index d3925e7..3af92ed 100644
*** a/src/test/regress/expected/privileges.out
--- b/src/test/regress/expected/privileges.out
*************** SELECT has_table_privilege('regress_priv
*** 1571,1576 ****
--- 1571,1582 ----
  ALTER DEFAULT PRIVILEGES FOR ROLE regress_priv_user1 REVOKE EXECUTE ON FUNCTIONS FROM public;
  ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_priv_user2; -- error
  ERROR:  cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS
+ --
+ -- Testing blanket default grants is very hazardous since it might change
+ -- the privileges attached to objects created by concurrent regression tests.
+ -- To avoid that, be sure to revoke the privileges again before committing.
+ --
+ BEGIN;
  ALTER DEFAULT PRIVILEGES GRANT USAGE ON SCHEMAS TO regress_priv_user2;
  CREATE SCHEMA testns2;
  SELECT has_schema_privilege('regress_priv_user2', 'testns2', 'USAGE'); -- yes
*************** SELECT has_schema_privilege('regress_pri
*** 1614,1619 ****
--- 1620,1626 ----
  (1 row)

  ALTER DEFAULT PRIVILEGES REVOKE ALL ON SCHEMAS FROM regress_priv_user2;
+ COMMIT;
  CREATE SCHEMA testns5;
  SELECT has_schema_privilege('regress_priv_user2', 'testns5', 'USAGE'); -- no
   has_schema_privilege
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 0d5c2ca..e3e6930 100644
*** a/src/test/regress/sql/privileges.sql
--- b/src/test/regress/sql/privileges.sql
*************** ALTER DEFAULT PRIVILEGES FOR ROLE regres
*** 935,940 ****
--- 935,947 ----

  ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_priv_user2; -- error

+ --
+ -- Testing blanket default grants is very hazardous since it might change
+ -- the privileges attached to objects created by concurrent regression tests.
+ -- To avoid that, be sure to revoke the privileges again before committing.
+ --
+ BEGIN;
+
  ALTER DEFAULT PRIVILEGES GRANT USAGE ON SCHEMAS TO regress_priv_user2;

  CREATE SCHEMA testns2;
*************** SELECT has_schema_privilege('regress_pri
*** 958,963 ****
--- 965,972 ----

  ALTER DEFAULT PRIVILEGES REVOKE ALL ON SCHEMAS FROM regress_priv_user2;

+ COMMIT;
+
  CREATE SCHEMA testns5;

  SELECT has_schema_privilege('regress_priv_user2', 'testns5', 'USAGE'); -- no