On Wed, Jul 1, 2026 at 9:51 PM Ashutosh Bapat
<[email protected]> wrote:
>
> I wondered whether we are missing special handling for PROPGRAPH at
> other places. I looked at other places where we handle OBJECT_SEQUENCE
> separately in acl related files. I discovered following missing cases
>
> 1. ExecGrant_Relation: I think we should clip the extra privileges
> with a warning when GRANT ... TABLE syntax is used to grant privileges
> on a property graph, just like sequences. To me it looks like we
> should prohibit GRANT ... TABLE on property graph altogether. But
> haven't done so to keep it in sync with sequences. The backward
> compatibility comment,  "For backward compatibility, just ... " should
> not be applicable in case of property graph since we can introduce
> whatever behaviour we expect from GRANT ... TABLE right from the first
> release which introduced property graph. But I am not sure if that's
> the only backward compatibility we are talking about here. Those
> commits go more than a few decades back and commit message itself
> doesn't help me much. Maybe someone with a better historical
> perspective may help. I have also added a test scenario for a
> non-property graph privilege to be added using GRANT ... TABLE syntax.
>

Since property graphs share the namespace with regular tables, I think
GRANT ... TABLE should be supported on property graphs, but restrict
it to only the privileges applicable to property graphs. Done that way
in the attached patch.

> The second change in this function seems necessary but without it, I
> couldn't find a visible bug. Mostly it's masked because the privileges
> available on a table are a superset of privileges available on a
> property graph.
>

This change is needed so that we can provide a correct error message.

Here's a revised patch set.
0010 is your patch without any changes
0011 is my changes described above.



-- 
Best Wishes,
Ashutosh Bapat
From 43a27e7a278d7b8a23bb7967c60aec5a226a3ebb Mon Sep 17 00:00:00 2001
From: Noah Misch <[email protected]>
Date: Wed, 1 Jul 2026 11:59:33 +0530
Subject: [PATCH v20260703 10/13] 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
---
 src/backend/catalog/information_schema.sql |  2 +-
 src/backend/utils/adt/acl.c                |  3 ++
 src/bin/pg_dump/pg_dump.c                  | 37 ++++++++++++++++++++--
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
index 4f0e2492937..624d538a5c0 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 01caa12eca7..e2547d719ed 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 f67daf85911..4d660d14b4c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7231,8 +7231,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, "
@@ -7418,7 +7427,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));
@@ -7470,6 +7479,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 */
 
 		/*
-- 
2.34.1

From b21ed425500c8bd492ed7acd615715c9d32ae3a7 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Wed, 1 Jul 2026 21:34:48 +0530
Subject: [PATCH v20260703 11/13] Fix GRANT ... TABLE on a property graph

A property graph only supports the SELECT privileges. Privileges can be
granted using either GRANT ... PROPERTY GRAPH or GRANT ... TABLE.  When
the GRANT ... TABLE form is used, ignore any privileges other than SELECT
and emit a warning, as we already do for sequences.

While here, add the missing RELKIND_PROPGRAPH cases in pg_class_aclmask_ext()
and in the object-type switch in ExecGrant_Relation() so that the default ACL
and the objtype passed to restrict_and_check_grant() are correct.

Author: Ashutosh Bapat <[email protected]>
Discussion: https://www.postgresql.org/message-id/[email protected]
---
 src/backend/catalog/aclchk.c                  | 31 ++++++++++++++++++-
 .../expected/create_property_graph.out        |  2 ++
 .../regress/sql/create_property_graph.sql     |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 140cd1302f5..54b59892c43 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -1784,7 +1784,7 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid, const char *relname,
 }
 
 /*
- *	This processes both sequences and non-sequences.
+ * This processes all pg_class entries including sequences and property graphs.
  */
 static void
 ExecGrant_Relation(InternalGrant *istmt)
@@ -1891,6 +1891,29 @@ ExecGrant_Relation(InternalGrant *istmt)
 					this_privileges &= (AclMode) ACL_ALL_RIGHTS_SEQUENCE;
 				}
 			}
+			else if (pg_class_tuple->relkind == RELKIND_PROPGRAPH)
+			{
+				/*
+				 * Per the SQL standard, property graphs share the namespace
+				 * with tables, so GRANT ... ON TABLE also works on a property
+				 * graph. A property graph only supports SELECT, so mask off
+				 * any other requested privileges and emit a warning, as we do
+				 * for sequences.
+				 */
+				if (this_privileges & ~((AclMode) ACL_ALL_RIGHTS_PROPGRAPH))
+				{
+					/*
+					 * Mention the object name because the user needs to know
+					 * which operations succeeded.  This is required because
+					 * WARNING allows the command to continue.
+					 */
+					ereport(WARNING,
+							(errcode(ERRCODE_INVALID_GRANT_OPERATION),
+							 errmsg("property graph \"%s\" only supports SELECT privileges",
+									NameStr(pg_class_tuple->relname))));
+					this_privileges &= (AclMode) ACL_ALL_RIGHTS_PROPGRAPH;
+				}
+			}
 			else
 			{
 				if (this_privileges & ~((AclMode) ACL_ALL_RIGHTS_RELATION))
@@ -1996,6 +2019,9 @@ ExecGrant_Relation(InternalGrant *istmt)
 				case RELKIND_SEQUENCE:
 					objtype = OBJECT_SEQUENCE;
 					break;
+				case RELKIND_PROPGRAPH:
+					objtype = OBJECT_PROPGRAPH;
+					break;
 				default:
 					objtype = OBJECT_TABLE;
 					break;
@@ -3389,6 +3415,9 @@ pg_class_aclmask_ext(Oid table_oid, Oid roleid, AclMode mask,
 			case RELKIND_SEQUENCE:
 				acl = acldefault(OBJECT_SEQUENCE, ownerId);
 				break;
+			case RELKIND_PROPGRAPH:
+				acl = acldefault(OBJECT_PROPGRAPH, ownerId);
+				break;
 			default:
 				acl = acldefault(OBJECT_TABLE, ownerId);
 				break;
diff --git a/src/test/regress/expected/create_property_graph.out b/src/test/regress/expected/create_property_graph.out
index e9f91f77ee9..713966e8a9e 100644
--- a/src/test/regress/expected/create_property_graph.out
+++ b/src/test/regress/expected/create_property_graph.out
@@ -251,6 +251,8 @@ SET ROLE regress_graph_user1;
 GRANT SELECT ON PROPERTY GRAPH g1 TO regress_graph_user2;
 GRANT UPDATE ON PROPERTY GRAPH g1 TO regress_graph_user2;  -- fail
 ERROR:  invalid privilege type UPDATE for property graph
+GRANT UPDATE ON TABLE g1 TO regress_graph_user2;  -- warning
+WARNING:  property graph "g1" only supports SELECT privileges
 RESET ROLE;
 -- collation
 CREATE TABLE tc1 (a int, b text);
diff --git a/src/test/regress/sql/create_property_graph.sql b/src/test/regress/sql/create_property_graph.sql
index 08341e13a50..5325d22206e 100644
--- a/src/test/regress/sql/create_property_graph.sql
+++ b/src/test/regress/sql/create_property_graph.sql
@@ -197,6 +197,7 @@ ALTER PROPERTY GRAPH g1 OWNER TO regress_graph_user1;
 SET ROLE regress_graph_user1;
 GRANT SELECT ON PROPERTY GRAPH g1 TO regress_graph_user2;
 GRANT UPDATE ON PROPERTY GRAPH g1 TO regress_graph_user2;  -- fail
+GRANT UPDATE ON TABLE g1 TO regress_graph_user2;  -- warning
 RESET ROLE;
 
 -- collation
-- 
2.34.1

Reply via email to