pg_dump doesn't do its normal ACL minimization for the new PROPERTY GRAPH
feature.  Patch attached.  See log message for details.

Most of the patch bulk (modest as it is) exists to keep support for dumping
from beta1.  I'm not sure whether it was worth bothering.  Breaking dump from
a beta is without precedent known to me, so I just erred on the side of not
breaking it.  If we were to decide pg_dump could drop support for betas, I'd
be fine with that.

This entails a catversion bump on the v19 branch.  If the master branch
already has a post-branch catversion bump by then, the two catversions should
remain distinct.  I'll use yyyymmdd1 for v19 and yyyymmdd2 for master.  That
feels cleanest to me, since it uses the *2 value where it will be
shortest-lived.  There's precedent in 20b6847 (master) / e256312 (v15).
From: Noah Misch <[email protected]>

Fix pg_dump ACL minimization for PROPERTY GRAPH.

Adding a GRANT caused pg_dump to emit a useless REVOKE + GRANT of owner
privileges, as seen in a dump of the regression database:

  REVOKE ALL ON PROPERTY GRAPH graph_rls_schema.cabinet FROM nm;
  GRANT ALL ON PROPERTY GRAPH graph_rls_schema.cabinet TO nm;
  GRANT ALL ON PROPERTY GRAPH graph_rls_schema.cabinet TO PUBLIC;

For normal dumps, this has no functional consequences.  For --no-owner
restores, the extra statements may fail or locate unrelated users of the
destination cluster.

The problem was pg_dump assuming NULL relacl implies acldefault('r'),
the default for TABLE.  Fix by teaching acldefault() to retrieve the
PROPERTY GRAPH default ACL.  So pg_dump can still dump from 19beta1, use
acldefault('g') for v20+ only.  For v19, use a hard-coded snapshot of
the v19 default.

information_schema.pg_property_graph_privileges also misused
acldefault('r'), but its "c.prtype IN ('SELECT')" predicate compensated
for it.  Switch to the new acldefault('g') for clarity.  Bump catversion
since a new view won't work with old binaries.  Back-patch to v19, which
introduced PROPERTY GRAPH.

Reviewed-by: FIXME
Discussion: https://postgr.es/m/FIXME
Backpatch-through: 19

diff --git a/src/backend/catalog/information_schema.sql 
b/src/backend/catalog/information_schema.sql
index 4f0e249..624d538 100644
--- a/src/backend/catalog/information_schema.sql
+++ b/src/backend/catalog/information_schema.sql
@@ -3328,7 +3328,7 @@ CREATE VIEW pg_property_graph_privileges AS
                   THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_grantable
 
     FROM (
-            SELECT oid, relname, relnamespace, relkind, relowner, 
(aclexplode(coalesce(relacl, acldefault('r', relowner)))).* FROM pg_class
+            SELECT oid, relname, relnamespace, relkind, relowner, 
(aclexplode(coalesce(relacl, acldefault('g', relowner)))).* FROM pg_class
          ) AS c (oid, relname, relnamespace, relkind, relowner, grantor, 
grantee, prtype, grantable),
          pg_namespace nc,
          pg_authid u_grantor,
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 01caa12..e2547d7 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -956,6 +956,9 @@ acldefault_sql(PG_FUNCTION_ARGS)
                case 'c':
                        objtype = OBJECT_COLUMN;
                        break;
+               case 'g':
+                       objtype = OBJECT_PROPGRAPH;
+                       break;
                case 'r':
                        objtype = OBJECT_TABLE;
                        break;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c56437d..41b9531 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7358,8 +7358,17 @@ getTables(Archive *fout, int *numTables)
                                                 "c.relhastriggers, 
c.relpersistence, "
                                                 "c.reloftype, "
                                                 "c.relacl, "
-                                                "acldefault(CASE WHEN 
c.relkind = " CppAsString2(RELKIND_SEQUENCE)
-                                                " THEN 's'::\"char\" ELSE 
'r'::\"char\" END, c.relowner) AS acldefault, "
+                                                "acldefault(CASE"
+                                                " WHEN c.relkind = " 
CppAsString2(RELKIND_PROPGRAPH));
+       /* 19beta1 didn't support acldefault('g'), so we'll fix that below */
+       appendPQExpBufferStr(query,
+                                                fout->remoteVersion >= 200000 ?
+                                                " THEN 'g'::\"char\"" :
+                                                " THEN NULL");
+       appendPQExpBufferStr(query,
+                                                " WHEN c.relkind = " 
CppAsString2(RELKIND_SEQUENCE)
+                                                " THEN 's'::\"char\""
+                                                " ELSE 'r'::\"char\" END, 
c.relowner) AS acldefault, "
                                                 "CASE WHEN c.relkind = " 
CppAsString2(RELKIND_FOREIGN_TABLE) " THEN "
                                                 "(SELECT ftserver FROM 
pg_catalog.pg_foreign_table WHERE ftrelid = c.oid) "
                                                 "ELSE 0 END AS foreignserver, "
@@ -7579,7 +7588,7 @@ getTables(Archive *fout, int *numTables)
                tblinfo[i].dobj.namespace =
                        findNamespace(atooid(PQgetvalue(res, i, 
i_relnamespace)));
                tblinfo[i].dacl.acl = pg_strdup(PQgetvalue(res, i, i_relacl));
-               tblinfo[i].dacl.acldefault = pg_strdup(PQgetvalue(res, i, 
i_acldefault));
+               /* acldefault computed below */
                tblinfo[i].dacl.privtype = 0;
                tblinfo[i].dacl.initprivs = NULL;
                tblinfo[i].relkind = *(PQgetvalue(res, i, i_relkind));
@@ -7631,6 +7640,28 @@ getTables(Archive *fout, int *numTables)
                tblinfo[i].is_identity_sequence = (strcmp(PQgetvalue(res, i, 
i_is_identity_sequence), "t") == 0);
                tblinfo[i].ispartition = (strcmp(PQgetvalue(res, i, 
i_ispartition), "t") == 0);
 
+               if (tblinfo[i].relkind == RELKIND_PROPGRAPH &&
+                       !(fout->remoteVersion >= 200000))
+               {
+                       PQExpBuffer aclarray = createPQExpBuffer();
+                       PQExpBuffer aclitem = createPQExpBuffer();
+
+                       /* Standard ACL as of v19 is {owner=r/owner} */
+                       appendPQExpBufferChar(aclarray, '{');
+                       quoteAclUserName(aclitem, tblinfo[i].rolname);
+                       appendPQExpBufferStr(aclitem, "=r/");
+                       quoteAclUserName(aclitem, tblinfo[i].rolname);
+                       appendPGArray(aclarray, aclitem->data);
+                       appendPQExpBufferChar(aclarray, '}');
+
+                       tblinfo[i].dacl.acldefault = pstrdup(aclarray->data);
+
+                       destroyPQExpBuffer(aclarray);
+                       destroyPQExpBuffer(aclitem);
+               }
+               else
+                       tblinfo[i].dacl.acldefault = pg_strdup(PQgetvalue(res, 
i, i_acldefault));
+
                /* other fields were zeroed above */
 
                /*

Reply via email to