pg_dump: Fix handling of ALTER DEFAULT PRIVILEGES

In commit 23f34fa, we changed how ACLs were handled to use the new
pg_init_privs catalog and to dump out the ACL commands as REVOKE+GRANT
combinations instead of trying to REVOKE all rights always and then
GRANT back just the ones which were in place.

Unfortunately, the DEFAULT PRIVILEGES system didn't quite get the
correct treatment with this change and ended up (incorrectly) only
including positive GRANTs instead of both the REVOKEs and GRANTs
necessary to preserve the correct privileges.

There are only a couple cases where such REVOKEs are possible because,
generally speaking, there's few rights which exist on objects by
default to be revoked.

Examples of REVOKEs which weren't being correctly preserved are when
privileges are REVOKE'd from the creator/owner, like so:

ALTER DEFAULT PRIVILEGES
  FOR ROLE myrole
  REVOKE SELECT ON TABLES FROM myrole;

or when other default privileges are being revoked, such as EXECUTE
rights granted to public for functions:

ALTER DEFAULT PRIVILEGES
  FOR ROLE myrole
  REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC;

Fix this by correctly working out what the correct REVOKE statements are
(if any) and dump them out, just as we do for everything else.

Noticed while developing additional regression tests for pg_dump, which
will be landing shortly.

Back-patch to 9.6 where the bug was introduced.

diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index cd1e8c4a680adee7be9c4d072f9e6a71bf96e5e0..a062a6b33018f164488107c314595dd7896dd38c 100644 (file)
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -368,11 +368,12 @@ buildACLCommands(const char *name, const char *subname,
  */
 bool
 buildDefaultACLCommands(const char *type, const char *nspname,
-                                               const char *acls, const char *owner,
+                                               const char *acls, const char *racls,
+                                               const char *initacls, const char *initracls,
+                                               const char *owner,
                                                int remoteVersion,
                                                PQExpBuffer sql)
 {
-       bool            result;
        PQExpBuffer prefix;
 
        prefix = createPQExpBuffer();
@@ -388,14 +389,22 @@ buildDefaultACLCommands(const char *type, const char *nspname,
        if (nspname)
                appendPQExpBuffer(prefix, "IN SCHEMA %s ", fmtId(nspname));
 
-       result = buildACLCommands("", NULL,
-                                                         type, acls, "", owner,
-                                                         prefix->data, remoteVersion,
-                                                         sql);
+       if (strlen(initacls) != 0 || strlen(initracls) != 0)
+       {
+               appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
+               if (!buildACLCommands("", NULL, type, initacls, initracls, owner,
+                                                         prefix->data, remoteVersion, sql))
+                       return false;
+               appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
+       }
+
+       if (!buildACLCommands("", NULL, type, acls, racls, owner,
+                                                 prefix->data, remoteVersion, sql))
+               return false;
 
        destroyPQExpBuffer(prefix);
 
-       return result;
+       return true;
 }
 
 /*

diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index b2fd7d37d0cc3490d40dcacbf8304f4b90b52fdf..caa390b3d3e4304b32e29989708ace375708c9ab 100644 (file)
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -41,7 +41,9 @@ extern bool buildACLCommands(const char *name, const char *subname,
                                 const char *owner, const char *prefix, int remoteVersion,
                                 PQExpBuffer sql);
 extern bool buildDefaultACLCommands(const char *type, const char *nspname,
-                                               const char *acls, const char *owner,
+                                               const char *acls, const char *racls,
+                                               const char *initacls, const char *initracls,
+                                               const char *owner,
                                                int remoteVersion,
                                                PQExpBuffer sql);
 extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 38d6a60610ec60197042f6548ac9ec27d7dc6173..8b2c7b913c54af35146808a7d02dc1f537382c18 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -8954,6 +8954,9 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
        int                     i_defaclnamespace;
        int                     i_defaclobjtype;
        int                     i_defaclacl;
+       int                     i_rdefaclacl;
+       int                     i_initdefaclacl;
+       int                     i_initrdefaclacl;
        int                     i,
                                ntups;
 
@@ -8968,13 +8971,50 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
        /* Make sure we are in proper schema */
        selectSourceSchema(fout, "pg_catalog");
 
-       appendPQExpBuffer(query, "SELECT oid, tableoid, "
-                                         "(%s defaclrole) AS defaclrole, "
-                                         "defaclnamespace, "
-                                         "defaclobjtype, "
-                                         "defaclacl "
-                                         "FROM pg_default_acl",
-                                         username_subquery);
+       if (fout->remoteVersion >= 90600)
+       {
+               PQExpBuffer acl_subquery = createPQExpBuffer();
+               PQExpBuffer racl_subquery = createPQExpBuffer();
+               PQExpBuffer initacl_subquery = createPQExpBuffer();
+               PQExpBuffer initracl_subquery = createPQExpBuffer();
+
+               buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
+                                               initracl_subquery, "defaclacl", "defaclrole",
+                                               "CASE WHEN defaclobjtype = 'S' THEN 's' ELSE defaclobjtype END::\"char\"",
+                                               dopt->binary_upgrade);
+
+               appendPQExpBuffer(query, "SELECT d.oid, d.tableoid, "
+                                                 "(%s d.defaclrole) AS defaclrole, "
+                                                 "d.defaclnamespace, "
+                                                 "d.defaclobjtype, "
+                                                 "%s AS defaclacl, "
+                                                 "%s AS rdefaclacl, "
+                                                 "%s AS initdefaclacl, "
+                                                 "%s AS initrdefaclacl "
+                                                 "FROM pg_default_acl d "
+                                                 "LEFT JOIN pg_init_privs pip ON "
+                                                 "(d.oid = pip.objoid "
+                                                 "AND pip.classoid = 'pg_default_acl'::regclass "
+                                                 "AND pip.objsubid = 0) ",
+                                                 username_subquery,
+                                                 acl_subquery->data,
+                                                 racl_subquery->data,
+                                                 initacl_subquery->data,
+                                                 initracl_subquery->data);
+       }
+       else
+       {
+               appendPQExpBuffer(query, "SELECT oid, tableoid, "
+                                                 "(%s defaclrole) AS defaclrole, "
+                                                 "defaclnamespace, "
+                                                 "defaclobjtype, "
+                                                 "defaclacl, "
+                                                 "NULL AS rdefaclacl, "
+                                                 "NULL AS initdefaclacl, "
+                                                 "NULL AS initrdefaclacl "
+                                                 "FROM pg_default_acl",
+                                                 username_subquery);
+       }
 
        res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
 
@@ -8989,6 +9029,9 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
        i_defaclnamespace = PQfnumber(res, "defaclnamespace");
        i_defaclobjtype = PQfnumber(res, "defaclobjtype");
        i_defaclacl = PQfnumber(res, "defaclacl");
+       i_rdefaclacl = PQfnumber(res, "rdefaclacl");
+       i_initdefaclacl = PQfnumber(res, "initdefaclacl");
+       i_initrdefaclacl = PQfnumber(res, "initrdefaclacl");
 
        for (i = 0; i < ntups; i++)
        {
@@ -9010,6 +9053,9 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
                daclinfo[i].defaclrole = pg_strdup(PQgetvalue(res, i, i_defaclrole));
                daclinfo[i].defaclobjtype = *(PQgetvalue(res, i, i_defaclobjtype));
                daclinfo[i].defaclacl = pg_strdup(PQgetvalue(res, i, i_defaclacl));
+               daclinfo[i].rdefaclacl = pg_strdup(PQgetvalue(res, i, i_rdefaclacl));
+               daclinfo[i].initdefaclacl = pg_strdup(PQgetvalue(res, i, i_initdefaclacl));
+               daclinfo[i].initrdefaclacl = pg_strdup(PQgetvalue(res, i, i_initrdefaclacl));
 
                /* Decide whether we want to dump it */
                selectDumpableDefaultACL(&(daclinfo[i]), dopt);
@@ -14615,6 +14661,9 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
                                                                 daclinfo->dobj.namespace != NULL ?
                                                                 daclinfo->dobj.namespace->dobj.name : NULL,
                                                                 daclinfo->defaclacl,
+                                                                daclinfo->rdefaclacl,
+                                                                daclinfo->initdefaclacl,
+                                                                daclinfo->initrdefaclacl,
                                                                 daclinfo->defaclrole,
                                                                 fout->remoteVersion,
                                                                 q))

diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 2bfa2d974203b708bd2be61f7017d8e5450945ce..f05cb3ffe1092852bfded31cb3f3eb5904ab1bcc 100644 (file)
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -524,6 +524,9 @@ typedef struct _defaultACLInfo
        char       *defaclrole;
        char            defaclobjtype;
        char       *defaclacl;
+       char       *rdefaclacl;
+       char       *initdefaclacl;
+       char       *initrdefaclacl;
 } DefaultACLInfo;
 
 typedef struct _blobInfo