public inbox for [email protected]
help / color / mirror / Atom feedFrom: Noah Misch <[email protected]>
To: [email protected]
Subject: PROPERTY GRAPH pg_dump ACL minimization
Date: Mon, 29 Jun 2026 19:33:08 -0700
Message-ID: <[email protected]> (raw)
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 */
/*
Attachments:
[text/plain] acldefault-propgraph-v1.patch (5.0K, ../[email protected]/2-acldefault-propgraph-v1.patch)
download | inline diff:
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 */
/*
view thread (11+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: PROPERTY GRAPH pg_dump ACL minimization
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox