pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed

From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: allow_system_table_mods stuff
Date: Fri, 28 Jun 2019 11:51:01 +0200
Message-ID: <a7455db0-2fc0-4ddc-4d8f-e4c860957a34@2ndquadrant.com> (raw)
In-Reply-To: <8b00ea5e-28a7-88ba-e848-21528b632354@2ndquadrant.com>
References: <8b00ea5e-28a7-88ba-e848-21528b632354@2ndquadrant.com>

Here is a new patch after the discussion.

- Rename allow_system_table_mods to allow_system_table_ddl.

(This makes room for a new allow_system_table_dml, but it's not
implemented here.)

- Make allow_system_table_ddl SUSET.

- Add regression test.

- Remove the behavior that allow_system_table_mods allowed
non-superusers to do DML on catalog tables without further access checking.

I think there was general agreement on all these points.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

v2-0001-New-and-improved-allow_system_table_ddl-setting.patch (text/plain, 43.3K) download
From b44c7c7b7177a87e777b3b2a7fe10ea739bfaa72 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 28 Jun 2019 11:36:04 +0200
Subject: [PATCH v2] New and improved allow_system_table_ddl setting

Rename allow_system_table_mods to allow_system_table_ddl to clarify
its meaning.  There is also some discussion about introducing a
parallel allow_system_table_dml setting, so this makes room for that.

Make allow_system_table_ddl settable at run time by superusers.  It
was previously postmaster start only.  We don't necessarily want to
make system catalog DDL wide-open, but there are occasionally useful
things to do like setting reloptions or statistics on a busy system
table, and blocking those doesn't help anyone.  Also, this enables
writing a test suite.

Add a regression test file that exercises the kinds of commands that
allow_system_table_ddl allows.

Previously, allow_system_table_mods allowed a non-superuser to do DML
on a system table without further permission checks.  This has been
removed, as it was quite inconsistent with the rest of the meaning of
this setting.  (Since allow_system_table_mods was previously only
accessible with a server restart, it is unlikely that anyone was using
this possibility.)

Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com
---
 doc/src/sgml/config.sgml                      |  16 +-
 doc/src/sgml/ref/show.sgml                    |  10 +-
 src/backend/catalog/aclchk.c                  |   5 +-
 src/backend/catalog/catalog.c                 |   2 +-
 src/backend/catalog/heap.c                    |  16 +-
 src/backend/catalog/index.c                   |  16 +-
 src/backend/commands/indexcmds.c              |   2 +-
 src/backend/commands/policy.c                 |   6 +-
 src/backend/commands/schemacmds.c             |   4 +-
 src/backend/commands/tablecmds.c              |  20 +-
 src/backend/commands/tablespace.c             |   4 +-
 src/backend/commands/trigger.c                |   6 +-
 src/backend/postmaster/postmaster.c           |   2 +-
 src/backend/rewrite/rewriteDefine.c           |   4 +-
 src/backend/tcop/postgres.c                   |   2 +-
 src/backend/utils/cache/relmapper.c           |   2 +-
 src/backend/utils/init/globals.c              |   2 +-
 src/backend/utils/misc/guc.c                  |   6 +-
 src/include/catalog/heap.h                    |   4 +-
 src/include/catalog/index.h                   |   4 +-
 src/include/miscadmin.h                       |   2 +-
 .../regress/expected/alter_system_table.out   | 170 ++++++++++++++++
 src/test/regress/expected/alter_table.out     |  10 +-
 src/test/regress/parallel_schedule            |   2 +-
 src/test/regress/serial_schedule              |   1 +
 src/test/regress/sql/alter_system_table.sql   | 187 ++++++++++++++++++
 src/test/regress/sql/alter_table.sql          |   6 +-
 27 files changed, 431 insertions(+), 80 deletions(-)
 create mode 100644 src/test/regress/expected/alter_system_table.out
 create mode 100644 src/test/regress/sql/alter_system_table.sql

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 84341a30e5..7a4e141a2d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9288,17 +9288,19 @@ <title>Developer Options</title>
     </para>
 
     <variablelist>
-     <varlistentry id="guc-allow-system-table-mods" xreflabel="allow_system_table_mods">
-      <term><varname>allow_system_table_mods</varname> (<type>boolean</type>)
+     <varlistentry id="guc-allow-system-table-ddl" xreflabel="allow_system_table_ddl">
+      <term><varname>allow_system_table_ddl</varname> (<type>boolean</type>)
       <indexterm>
-        <primary><varname>allow_system_table_mods</varname> configuration parameter</primary>
+        <primary><varname>allow_system_table_ddl</varname> configuration parameter</primary>
       </indexterm>
       </term>
       <listitem>
        <para>
-        Allows modification of the structure of system tables.
-        This is used by <command>initdb</command>.
-        This parameter can only be set at server start.
+        Allows DDL commands on system tables.  This is otherwise not allowed
+        even for superusers.  This is used by <command>initdb</command>.
+        Inconsiderate use of this setting can cause irretrievable data loss or
+        seriously corrupt the database system.  Only superusers can change
+        this setting.
        </para>
       </listitem>
      </varlistentry>
@@ -9831,7 +9833,7 @@ <title>Short Option Key</title>
        </row>
        <row>
         <entry><option>-O</option></entry>
-        <entry><literal>allow_system_table_mods = on</literal></entry>
+        <entry><literal>allow_system_table_ddl = on</literal></entry>
        </row>
        <row>
         <entry><option>-p <replaceable>x</replaceable></option></entry>
diff --git a/doc/src/sgml/ref/show.sgml b/doc/src/sgml/ref/show.sgml
index 945b0491b1..c98dc02444 100644
--- a/doc/src/sgml/ref/show.sgml
+++ b/doc/src/sgml/ref/show.sgml
@@ -167,14 +167,14 @@ <title>Examples</title>
    Show all settings:
 <programlisting>
 SHOW ALL;
-            name         | setting |                description                                                          
--------------------------+---------+-------------------------------------------------
- allow_system_table_mods | off     | Allows modifications of the structure of ...
+            name        | setting |                description                                                          
+------------------------+---------+-------------------------------------------------
+ allow_system_table_ddl | off     | Allows DDL on system tables.
     .
     .
     .
- xmloption               | content | Sets whether XML data in implicit parsing ...
- zero_damaged_pages      | off     | Continues processing past damaged page headers.
+ xmloption              | content | Sets whether XML data in implicit parsing ...
+ zero_damaged_pages     | off     | Continues processing past damaged page headers.
 (196 rows)
 </programlisting></para>
  </refsect1>
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 2797af35c3..945f799c55 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3852,7 +3852,7 @@ pg_class_aclmask(Oid table_oid, Oid roleid,
 
 	/*
 	 * Deny anyone permission to update a system catalog unless
-	 * pg_authid.rolsuper is set.  Also allow it if allowSystemTableMods.
+	 * pg_authid.rolsuper is set.
 	 *
 	 * As of 7.4 we have some updatable system views; those shouldn't be
 	 * protected in this way.  Assume the view rules can take care of
@@ -3861,8 +3861,7 @@ pg_class_aclmask(Oid table_oid, Oid roleid,
 	if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE | ACL_USAGE)) &&
 		IsSystemClass(table_oid, classForm) &&
 		classForm->relkind != RELKIND_VIEW &&
-		!superuser_arg(roleid) &&
-		!allowSystemTableMods)
+		!superuser_arg(roleid))
 	{
 #ifdef ACLDEBUG
 		elog(DEBUG2, "permission denied for system catalog update");
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index a065419cdb..77d8d54526 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -59,7 +59,7 @@
  *		We treat toast tables of user relations as "system relations" for
  *		protection purposes, e.g. you can't change their schemas without
  *		special permissions.  Therefore, most uses of this function are
- *		checking whether allow_system_table_mods restrictions apply.
+ *		checking whether allow_system_table_ddl restrictions apply.
  *		For other purposes, consider whether you shouldn't be using
  *		IsCatalogRelation instead.
  *
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 86820eecfc..e6960e543d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -304,7 +304,7 @@ heap_create(const char *relname,
 			char relpersistence,
 			bool shared_relation,
 			bool mapped_relation,
-			bool allow_system_table_mods,
+			bool allow_system_table_ddl,
 			TransactionId *relfrozenxid,
 			MultiXactId *relminmxid)
 {
@@ -320,10 +320,10 @@ heap_create(const char *relname,
 	 * paths including pg_catalog are too confusing for now.
 	 *
 	 * But allow creating indexes on relations in pg_catalog even if
-	 * allow_system_table_mods = off, upper layers already guarantee it's on a
+	 * allow_system_table_ddl = off, upper layers already guarantee it's on a
 	 * user defined relation, not a system one.
 	 */
-	if (!allow_system_table_mods &&
+	if (!allow_system_table_ddl &&
 		((IsCatalogNamespace(relnamespace) && relkind != RELKIND_INDEX) ||
 		 IsToastNamespace(relnamespace)) &&
 		IsNormalProcessingMode())
@@ -1053,7 +1053,7 @@ AddNewRelationType(const char *typeName,
  *	reloptions: reloptions in Datum form, or (Datum) 0 if none
  *	use_user_acl: true if should look for user-defined default permissions;
  *		if false, relacl is always set NULL
- *	allow_system_table_mods: true to allow creation in system namespaces
+ *	allow_system_table_ddl: true to allow creation in system namespaces
  *	is_internal: is this a system-generated catalog?
  *
  * Output parameters:
@@ -1080,7 +1080,7 @@ heap_create_with_catalog(const char *relname,
 						 OnCommitAction oncommit,
 						 Datum reloptions,
 						 bool use_user_acl,
-						 bool allow_system_table_mods,
+						 bool allow_system_table_ddl,
 						 bool is_internal,
 						 Oid relrewrite,
 						 ObjectAddress *typaddress)
@@ -1105,11 +1105,11 @@ heap_create_with_catalog(const char *relname,
 
 	/*
 	 * Validate proposed tupdesc for the desired relkind.  If
-	 * allow_system_table_mods is on, allow ANYARRAY to be used; this is a
+	 * allow_system_table_ddl is on, allow ANYARRAY to be used; this is a
 	 * hack to allow creating pg_statistic and cloning it during VACUUM FULL.
 	 */
 	CheckAttributeNamesTypes(tupdesc, relkind,
-							 allow_system_table_mods ? CHKATYPE_ANYARRAY : 0);
+							 allow_system_table_ddl ? CHKATYPE_ANYARRAY : 0);
 
 	/*
 	 * This would fail later on anyway, if the relation already exists.  But
@@ -1226,7 +1226,7 @@ heap_create_with_catalog(const char *relname,
 							   relpersistence,
 							   shared_relation,
 							   mapped_relation,
-							   allow_system_table_mods,
+							   allow_system_table_ddl,
 							   &relfrozenxid,
 							   &relminmxid);
 
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index bb60b23093..b7719c3077 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -682,7 +682,7 @@ UpdateIndexRelation(Oid indexoid,
  *			create a partitioned index (table must be partitioned)
  * constr_flags: flags passed to index_constraint_create
  *		(only if INDEX_CREATE_ADD_CONSTRAINT is set)
- * allow_system_table_mods: allow table to be a system catalog
+ * allow_system_table_ddl: allow table to be a system catalog
  * is_internal: if true, post creation hook for new index
  * constraintId: if not NULL, receives OID of created constraint
  *
@@ -705,7 +705,7 @@ index_create(Relation heapRelation,
 			 Datum reloptions,
 			 bits16 flags,
 			 bits16 constr_flags,
-			 bool allow_system_table_mods,
+			 bool allow_system_table_ddl,
 			 bool is_internal,
 			 Oid *constraintId)
 {
@@ -755,7 +755,7 @@ index_create(Relation heapRelation,
 	if (indexInfo->ii_NumIndexAttrs < 1)
 		elog(ERROR, "must index at least one column");
 
-	if (!allow_system_table_mods &&
+	if (!allow_system_table_ddl &&
 		IsSystemRelation(heapRelation) &&
 		IsNormalProcessingMode())
 		ereport(ERROR,
@@ -886,7 +886,7 @@ index_create(Relation heapRelation,
 								relpersistence,
 								shared_relation,
 								mapped_relation,
-								allow_system_table_mods,
+								allow_system_table_ddl,
 								&relfrozenxid,
 								&relminmxid);
 
@@ -1011,7 +1011,7 @@ index_create(Relation heapRelation,
 												indexRelationName,
 												constraintType,
 												constr_flags,
-												allow_system_table_mods,
+												allow_system_table_ddl,
 												is_internal);
 			if (constraintId)
 				*constraintId = localaddr.objectId;
@@ -1668,7 +1668,7 @@ index_concurrently_set_dead(Oid heapId, Oid indexId)
  *		INDEX_CONSTR_CREATE_UPDATE_INDEX: update the pg_index row
  *		INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS: remove existing dependencies
  *			of index on table's columns
- * allow_system_table_mods: allow table to be a system catalog
+ * allow_system_table_ddl: allow table to be a system catalog
  * is_internal: index is constructed due to internal process
  */
 ObjectAddress
@@ -1679,7 +1679,7 @@ index_constraint_create(Relation heapRelation,
 						const char *constraintName,
 						char constraintType,
 						bits16 constr_flags,
-						bool allow_system_table_mods,
+						bool allow_system_table_ddl,
 						bool is_internal)
 {
 	Oid			namespaceId = RelationGetNamespace(heapRelation);
@@ -1701,7 +1701,7 @@ index_constraint_create(Relation heapRelation,
 	Assert(!IsBootstrapProcessingMode());
 
 	/* enforce system-table restriction */
-	if (!allow_system_table_mods &&
+	if (!allow_system_table_ddl &&
 		IsSystemRelation(heapRelation) &&
 		IsNormalProcessingMode())
 		ereport(ERROR,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 579b998954..b6bc6285be 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1009,7 +1009,7 @@ DefineIndex(Oid relationId,
 					 collationObjectId, classObjectId,
 					 coloptions, reloptions,
 					 flags, constr_flags,
-					 allowSystemTableMods, !check_rights,
+					 allowSystemTableDDL, !check_rights,
 					 &createdConstraintId);
 
 	ObjectAddressSet(address, RelationRelationId, indexRelationId);
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 1df76623ad..f400cbceaf 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -82,7 +82,7 @@ RangeVarCallbackForPolicy(const RangeVar *rv, Oid relid, Oid oldrelid,
 		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relid)), rv->relname);
 
 	/* No system table modifications unless explicitly allowed. */
-	if (!allowSystemTableMods && IsSystemClass(relid, classform))
+	if (!allowSystemTableDDL && IsSystemClass(relid, classform))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -395,7 +395,7 @@ RemovePolicyById(Oid policy_id)
 				 errmsg("\"%s\" is not a table",
 						RelationGetRelationName(rel))));
 
-	if (!allowSystemTableMods && IsSystemRelation(rel))
+	if (!allowSystemTableDDL && IsSystemRelation(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -485,7 +485,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
 				 errmsg("\"%s\" is not a table",
 						RelationGetRelationName(rel))));
 
-	if (!allowSystemTableMods && IsSystemRelation(rel))
+	if (!allowSystemTableDDL && IsSystemRelation(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 6cf94a3140..6e7956706b 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -100,7 +100,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
 	check_is_member_of_role(saved_uid, owner_uid);
 
 	/* Additional check to protect reserved schema names */
-	if (!allowSystemTableMods && IsReservedName(schemaName))
+	if (!allowSystemTableDDL && IsReservedName(schemaName))
 		ereport(ERROR,
 				(errcode(ERRCODE_RESERVED_NAME),
 				 errmsg("unacceptable schema name \"%s\"", schemaName),
@@ -276,7 +276,7 @@ RenameSchema(const char *oldname, const char *newname)
 		aclcheck_error(aclresult, OBJECT_DATABASE,
 					   get_database_name(MyDatabaseId));
 
-	if (!allowSystemTableMods && IsReservedName(newname))
+	if (!allowSystemTableDDL && IsReservedName(newname))
 		ereport(ERROR,
 				(errcode(ERRCODE_RESERVED_NAME),
 				 errmsg("unacceptable schema name \"%s\"", newname),
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7dcd634a1a..d633442834 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -864,7 +864,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 										  stmt->oncommit,
 										  reloptions,
 										  true,
-										  allowSystemTableMods,
+										  allowSystemTableDDL,
 										  false,
 										  InvalidOid,
 										  typaddress);
@@ -1444,7 +1444,7 @@ RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid,
 	}
 
 	/* In the case of an invalid index, it is fine to bypass this check */
-	if (!invalid_system_index && !allowSystemTableMods && IsSystemClass(relOid, classform))
+	if (!invalid_system_index && !allowSystemTableDDL && IsSystemClass(relOid, classform))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -1926,7 +1926,7 @@ truncate_check_rel(Oid relid, Form_pg_class reltuple)
 		aclcheck_error(aclresult, get_relkind_objtype(reltuple->relkind),
 					   relname);
 
-	if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
+	if (!allowSystemTableDDL && IsSystemClass(relid, reltuple))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -2899,7 +2899,7 @@ renameatt_check(Oid myrelid, Form_pg_class classform, bool recursing)
 	if (!pg_class_ownercheck(myrelid, GetUserId()))
 		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(myrelid)),
 					   NameStr(classform->relname));
-	if (!allowSystemTableMods && IsSystemClass(myrelid, classform))
+	if (!allowSystemTableDDL && IsSystemClass(myrelid, classform))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -5146,7 +5146,7 @@ ATSimplePermissions(Relation rel, int allowed_targets)
 		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(rel->rd_rel->relkind),
 					   RelationGetRelationName(rel));
 
-	if (!allowSystemTableMods && IsSystemRelation(rel))
+	if (!allowSystemTableDDL && IsSystemRelation(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -6659,7 +6659,7 @@ ATPrepSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newVa
 	 * We do our own permission checking because (a) we want to allow SET
 	 * STATISTICS on indexes (for expressional index columns), and (b) we want
 	 * to allow SET STATISTICS on system catalogs without requiring
-	 * allowSystemTableMods to be turned on.
+	 * allowSystemTableDDL to be turned on.
 	 */
 	if (rel->rd_rel->relkind != RELKIND_RELATION &&
 		rel->rd_rel->relkind != RELKIND_MATVIEW &&
@@ -7292,7 +7292,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
 									  constraintName,
 									  constraintType,
 									  flags,
-									  allowSystemTableMods,
+									  allowSystemTableDDL,
 									  false);	/* is_internal */
 
 	index_close(indexRel, NoLock);
@@ -7616,7 +7616,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 				 errmsg("referenced relation \"%s\" is not a table",
 						RelationGetRelationName(pkrel))));
 
-	if (!allowSystemTableMods && IsSystemRelation(pkrel))
+	if (!allowSystemTableDDL && IsSystemRelation(pkrel))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -14775,7 +14775,7 @@ RangeVarCallbackOwnsRelation(const RangeVar *relation,
 		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)),
 					   relation->relname);
 
-	if (!allowSystemTableMods &&
+	if (!allowSystemTableDDL &&
 		IsSystemClass(relId, (Form_pg_class) GETSTRUCT(tuple)))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -14811,7 +14811,7 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid,
 		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relid)), rv->relname);
 
 	/* No system table modifications unless explicitly allowed. */
-	if (!allowSystemTableMods && IsSystemClass(relid, classform))
+	if (!allowSystemTableDDL && IsSystemClass(relid, classform))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 5e43867e6f..5c278b9c2a 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -300,7 +300,7 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 	 * Disallow creation of tablespaces named "pg_xxx"; we reserve this
 	 * namespace for system purposes.
 	 */
-	if (!allowSystemTableMods && IsReservedName(stmt->tablespacename))
+	if (!allowSystemTableDDL && IsReservedName(stmt->tablespacename))
 		ereport(ERROR,
 				(errcode(ERRCODE_RESERVED_NAME),
 				 errmsg("unacceptable tablespace name \"%s\"",
@@ -951,7 +951,7 @@ RenameTableSpace(const char *oldname, const char *newname)
 		aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE, oldname);
 
 	/* Validate new name */
-	if (!allowSystemTableMods && IsReservedName(newname))
+	if (!allowSystemTableDDL && IsReservedName(newname))
 		ereport(ERROR,
 				(errcode(ERRCODE_RESERVED_NAME),
 				 errmsg("unacceptable tablespace name \"%s\"", newname),
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 316692b7c2..0c129bb853 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -313,7 +313,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 				 errmsg("\"%s\" is not a table or view",
 						RelationGetRelationName(rel))));
 
-	if (!allowSystemTableMods && IsSystemRelation(rel))
+	if (!allowSystemTableDDL && IsSystemRelation(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -1539,7 +1539,7 @@ RemoveTriggerById(Oid trigOid)
 				 errmsg("\"%s\" is not a table, view, or foreign table",
 						RelationGetRelationName(rel))));
 
-	if (!allowSystemTableMods && IsSystemRelation(rel))
+	if (!allowSystemTableDDL && IsSystemRelation(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -1648,7 +1648,7 @@ RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid,
 	/* you must own the table to rename one of its triggers */
 	if (!pg_class_ownercheck(relid, GetUserId()))
 		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relid)), rv->relname);
-	if (!allowSystemTableMods && IsSystemClass(relid, form))
+	if (!allowSystemTableDDL && IsSystemClass(relid, form))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 688ad439ed..c336f1081b 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -745,7 +745,7 @@ PostmasterMain(int argc, char *argv[])
 				break;
 
 			case 'O':
-				SetConfigOption("allow_system_table_mods", "true", PGC_POSTMASTER, PGC_S_ARGV);
+				SetConfigOption("allow_system_table_ddl", "true", PGC_POSTMASTER, PGC_S_ARGV);
 				break;
 
 			case 'o':
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 7df2b6154c..7a2df2cd4e 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -271,7 +271,7 @@ DefineQueryRewrite(const char *rulename,
 				 errmsg("\"%s\" is not a table or view",
 						RelationGetRelationName(event_relation))));
 
-	if (!allowSystemTableMods && IsSystemRelation(event_relation))
+	if (!allowSystemTableDDL && IsSystemRelation(event_relation))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
@@ -927,7 +927,7 @@ RangeVarCallbackForRenameRule(const RangeVar *rv, Oid relid, Oid oldrelid,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a table or view", rv->relname)));
 
-	if (!allowSystemTableMods && IsSystemClass(relid, form))
+	if (!allowSystemTableDDL && IsSystemClass(relid, form))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 44a59e1d4f..d4fa62f227 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3572,7 +3572,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
 				break;
 
 			case 'O':
-				SetConfigOption("allow_system_table_mods", "true", ctx, gucsource);
+				SetConfigOption("allow_system_table_ddl", "true", ctx, gucsource);
 				break;
 
 			case 'o':
diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c
index 3b4f21bc54..baefb94209 100644
--- a/src/backend/utils/cache/relmapper.c
+++ b/src/backend/utils/cache/relmapper.c
@@ -974,7 +974,7 @@ perform_relmap_update(bool shared, const RelMapFile *updates)
 	 * Apply the updates to newmap.  No new mappings should appear, unless
 	 * somebody is adding indexes to system catalogs.
 	 */
-	merge_map_updates(&newmap, updates, allowSystemTableMods);
+	merge_map_updates(&newmap, updates, allowSystemTableDDL);
 
 	/* Write out the updated map and do other necessary tasks */
 	write_relmap_file(shared, &newmap, true, true, true,
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 3bf96de256..50304f070a 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -117,7 +117,7 @@ int			DateOrder = DATEORDER_MDY;
 int			IntervalStyle = INTSTYLE_POSTGRES;
 
 bool		enableFsync = true;
-bool		allowSystemTableMods = false;
+bool		allowSystemTableDDL = false;
 int			work_mem = 1024;
 int			maintenance_work_mem = 16384;
 int			max_parallel_maintenance_workers = 2;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1208eb9a68..941a0e2d24 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1772,12 +1772,12 @@ static struct config_bool ConfigureNamesBool[] =
 	},
 
 	{
-		{"allow_system_table_mods", PGC_POSTMASTER, DEVELOPER_OPTIONS,
-			gettext_noop("Allows modifications of the structure of system tables."),
+		{"allow_system_table_ddl", PGC_SUSET, DEVELOPER_OPTIONS,
+			gettext_noop("Allows DDL on system tables."),
 			NULL,
 			GUC_NOT_IN_SAMPLE
 		},
-		&allowSystemTableMods,
+		&allowSystemTableDDL,
 		false,
 		NULL, NULL, NULL
 	},
diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index eec71c29d5..342337c3fa 100644
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -56,7 +56,7 @@ extern Relation heap_create(const char *relname,
 							char relpersistence,
 							bool shared_relation,
 							bool mapped_relation,
-							bool allow_system_table_mods,
+							bool allow_system_table_ddl,
 							TransactionId *relfrozenxid,
 							MultiXactId *relminmxid);
 
@@ -77,7 +77,7 @@ extern Oid	heap_create_with_catalog(const char *relname,
 									 OnCommitAction oncommit,
 									 Datum reloptions,
 									 bool use_user_acl,
-									 bool allow_system_table_mods,
+									 bool allow_system_table_ddl,
 									 bool is_internal,
 									 Oid relrewrite,
 									 ObjectAddress *typaddress);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 1113d25b2d..0a9280c8f6 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -68,7 +68,7 @@ extern Oid	index_create(Relation heapRelation,
 						 Datum reloptions,
 						 bits16 flags,
 						 bits16 constr_flags,
-						 bool allow_system_table_mods,
+						 bool allow_system_table_ddl,
 						 bool is_internal,
 						 Oid *constraintId);
 
@@ -99,7 +99,7 @@ extern ObjectAddress index_constraint_create(Relation heapRelation,
 											 const char *constraintName,
 											 char constraintType,
 											 bits16 constr_flags,
-											 bool allow_system_table_mods,
+											 bool allow_system_table_ddl,
 											 bool is_internal);
 
 extern void index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode);
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 61a24c2e3c..ce5d27ca33 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -241,7 +241,7 @@ extern PGDLLIMPORT int IntervalStyle;
 #define MAXTZLEN		10		/* max TZ name len, not counting tr. null */
 
 extern bool enableFsync;
-extern PGDLLIMPORT bool allowSystemTableMods;
+extern PGDLLIMPORT bool allowSystemTableDDL;
 extern PGDLLIMPORT int work_mem;
 extern PGDLLIMPORT int maintenance_work_mem;
 extern PGDLLIMPORT int max_parallel_maintenance_workers;
diff --git a/src/test/regress/expected/alter_system_table.out b/src/test/regress/expected/alter_system_table.out
new file mode 100644
index 0000000000..29dc7f05c1
--- /dev/null
+++ b/src/test/regress/expected/alter_system_table.out
@@ -0,0 +1,170 @@
+--
+-- Tests for things affected by allow_system_table_ddl
+--
+-- We run the same set of commands once with allow_system_table_ddl
+-- off and then again with on.
+--
+-- The "on" tests should where possible be wrapped in BEGIN/ROLLBACK
+-- blocks so as to not leave a mess around.  Also, be sure to clean up
+-- to not leave objects that cannot be dumped or restored by the
+-- pg_upgrade test suite.
+CREATE USER regress_user_ast;
+SET allow_system_table_ddl = off;
+-- create new table in pg_catalog
+CREATE TABLE pg_catalog.test (a int);
+ERROR:  permission denied to create "pg_catalog.test"
+DETAIL:  System catalog modifications are currently disallowed.
+-- anyarray column
+CREATE TABLE t1x (a int, b anyarray);
+ERROR:  column "b" has pseudo-type anyarray
+-- index on system catalog
+ALTER TABLE pg_namespace ADD UNIQUE USING INDEX pg_namespace_oid_index;
+ERROR:  permission denied: "pg_namespace" is a system catalog
+-- write to system catalog table as superuser
+-- (allowed even without allow_system_table_ddl)
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 0, 'foo');
+-- write to system catalog table as normal user
+GRANT INSERT ON pg_description TO regress_user_ast;
+SET ROLE regress_user_ast;
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 1, 'foo');
+ERROR:  permission denied for table pg_description
+RESET ROLE;
+-- policy on system catalog
+CREATE POLICY foo ON pg_description FOR SELECT USING (description NOT LIKE 'secret%');
+ERROR:  permission denied: "pg_description" is a system catalog
+-- reserved schema name
+CREATE SCHEMA pg_foo;
+ERROR:  unacceptable schema name "pg_foo"
+DETAIL:  The prefix "pg_" is reserved for system schemas.
+-- drop system table
+DROP TABLE pg_description;
+ERROR:  permission denied: "pg_description" is a system catalog
+-- truncate of system table
+TRUNCATE pg_description;
+ERROR:  permission denied: "pg_description" is a system catalog
+-- rename column of system table
+ALTER TABLE pg_description RENAME COLUMN description TO comment;
+ERROR:  permission denied: "pg_description" is a system catalog
+-- ATSimplePermissions()
+ALTER TABLE pg_description ALTER COLUMN description SET NOT NULL;
+ERROR:  permission denied: "pg_description" is a system catalog
+-- SET STATISTICS
+ALTER TABLE pg_description ALTER COLUMN description SET STATISTICS -1;
+ERROR:  permission denied: "pg_description" is a system catalog
+-- foreign key referencing catalog
+CREATE TABLE foo (a oid, b oid, c int, FOREIGN KEY (a, b, c) REFERENCES pg_description);
+ERROR:  permission denied: "pg_description" is a system catalog
+-- RangeVarCallbackOwnsRelation()
+CREATE INDEX pg_descripton_test_index ON pg_description (description);
+ERROR:  permission denied: "pg_description" is a system catalog
+-- RangeVarCallbackForAlterRelation()
+ALTER TABLE pg_description RENAME TO pg_comment;
+ERROR:  permission denied: "pg_description" is a system catalog
+ALTER TABLE pg_description SET SCHEMA public;
+ERROR:  permission denied: "pg_description" is a system catalog
+-- reserved tablespace name
+CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
+ERROR:  unacceptable tablespace name "pg_foo"
+DETAIL:  The prefix "pg_" is reserved for system tablespaces.
+-- triggers
+CREATE FUNCTION tf1() RETURNS trigger
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  RETURN NULL;
+END $$;
+CREATE TRIGGER t1 BEFORE INSERT ON pg_description EXECUTE FUNCTION tf1();
+ERROR:  permission denied: "pg_description" is a system catalog
+ALTER TRIGGER t1 ON pg_description RENAME TO t2;
+ERROR:  permission denied: "pg_description" is a system catalog
+--DROP TRIGGER t2 ON pg_description;
+-- rules
+CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+ERROR:  permission denied: "pg_description" is a system catalog
+ALTER RULE r1 ON pg_description RENAME TO r2;
+ERROR:  permission denied: "pg_description" is a system catalog
+--DROP RULE r2 ON pg_description;
+SET allow_system_table_ddl = on;
+-- create new table in pg_catalog
+BEGIN;
+CREATE TABLE pg_catalog.test (a int);
+ROLLBACK;
+-- anyarray column
+BEGIN;
+CREATE TABLE t1 (a int, b anyarray);
+ROLLBACK;
+-- index on system catalog
+BEGIN;
+ALTER TABLE pg_namespace ADD UNIQUE USING INDEX pg_namespace_oid_index;
+ROLLBACK;
+-- write to system catalog table as superuser
+BEGIN;
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 2, 'foo');
+ROLLBACK;
+-- write to system catalog table as normal user
+-- (not allowed)
+SET ROLE regress_user_ast;
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 3, 'foo');
+ERROR:  permission denied for table pg_description
+RESET ROLE;
+-- policy on system catalog
+BEGIN;
+CREATE POLICY foo ON pg_description FOR SELECT USING (description NOT LIKE 'secret%');
+ROLLBACK;
+-- reserved schema name
+BEGIN;
+CREATE SCHEMA pg_foo;
+ROLLBACK;
+-- drop system table
+-- (This will fail anyway because it's pinned.)
+BEGIN;
+DROP TABLE pg_description;
+ERROR:  cannot drop table pg_description because it is required by the database system
+ROLLBACK;
+-- truncate of system table
+BEGIN;
+TRUNCATE pg_description;
+ROLLBACK;
+-- rename column of system table
+BEGIN;
+ALTER TABLE pg_description RENAME COLUMN description TO comment;
+ROLLBACK;
+-- ATSimplePermissions()
+BEGIN;
+ALTER TABLE pg_description ALTER COLUMN description SET NOT NULL;
+ROLLBACK;
+-- SET STATISTICS
+BEGIN;
+ALTER TABLE pg_description ALTER COLUMN description SET STATISTICS -1;
+ROLLBACK;
+-- foreign key referencing catalog
+BEGIN;
+ALTER TABLE pg_description ADD PRIMARY KEY USING INDEX pg_description_o_c_o_index;
+CREATE TABLE foo (a oid, b oid, c int, FOREIGN KEY (a, b, c) REFERENCES pg_description);
+ROLLBACK;
+-- RangeVarCallbackOwnsRelation()
+BEGIN;
+CREATE INDEX pg_descripton_test_index ON pg_description (description);
+ROLLBACK;
+-- RangeVarCallbackForAlterRelation()
+BEGIN;
+ALTER TABLE pg_description RENAME TO pg_comment;
+ROLLBACK;
+BEGIN;
+ALTER TABLE pg_description SET SCHEMA public;
+ROLLBACK;
+-- reserved tablespace name
+CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
+ERROR:  directory "/no/such/location" does not exist
+-- triggers
+CREATE TRIGGER t1 BEFORE INSERT ON pg_description EXECUTE FUNCTION tf1();
+ALTER TRIGGER t1 ON pg_description RENAME TO t2;
+DROP TRIGGER t2 ON pg_description;
+-- rules
+CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+ALTER RULE r1 ON pg_description RENAME TO r2;
+DROP RULE r2 ON pg_description;
+-- cleanup
+REVOKE ALL ON pg_description FROM regress_user_ast;
+DROP USER regress_user_ast;
+DROP FUNCTION tf1;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index a639601f66..58892bbb2b 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3221,13 +3221,9 @@ WHERE c.oid IS NOT NULL OR m.mapped_oid IS NOT NULL;
 
 -- Checks on creating and manipulation of user defined relations in
 -- pg_catalog.
---
--- XXX: It would be useful to add checks around trying to manipulate
--- catalog tables, but that might have ugly consequences when run
--- against an existing server with allow_system_table_mods = on.
-SHOW allow_system_table_mods;
- allow_system_table_mods 
--------------------------
+SHOW allow_system_table_ddl;
+ allow_system_table_ddl 
+------------------------
  off
 (1 row)
 
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index f23fe8d870..72c0e515b2 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -111,7 +111,7 @@ test: plancache limit plpgsql copy2 temp domain rangefuncs prepare conversion tr
 # ----------
 # Another group of parallel tests
 # ----------
-test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info
+test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info alter_system_table
 
 # event triggers cannot run concurrently with any test that runs DDL
 test: event_trigger
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index ca200eb599..7cacbffae8 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -191,6 +191,7 @@ test: hash_part
 test: indexing
 test: partition_aggregate
 test: partition_info
+test: alter_system_table
 test: event_trigger
 test: fast_default
 test: stats
diff --git a/src/test/regress/sql/alter_system_table.sql b/src/test/regress/sql/alter_system_table.sql
new file mode 100644
index 0000000000..9753cef11d
--- /dev/null
+++ b/src/test/regress/sql/alter_system_table.sql
@@ -0,0 +1,187 @@
+--
+-- Tests for things affected by allow_system_table_ddl
+--
+-- We run the same set of commands once with allow_system_table_ddl
+-- off and then again with on.
+--
+-- The "on" tests should where possible be wrapped in BEGIN/ROLLBACK
+-- blocks so as to not leave a mess around.  Also, be sure to clean up
+-- to not leave objects that cannot be dumped or restored by the
+-- pg_upgrade test suite.
+
+CREATE USER regress_user_ast;
+
+SET allow_system_table_ddl = off;
+
+-- create new table in pg_catalog
+CREATE TABLE pg_catalog.test (a int);
+
+-- anyarray column
+CREATE TABLE t1x (a int, b anyarray);
+
+-- index on system catalog
+ALTER TABLE pg_namespace ADD UNIQUE USING INDEX pg_namespace_oid_index;
+
+-- write to system catalog table as superuser
+-- (allowed even without allow_system_table_ddl)
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 0, 'foo');
+
+-- write to system catalog table as normal user
+GRANT INSERT ON pg_description TO regress_user_ast;
+SET ROLE regress_user_ast;
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 1, 'foo');
+RESET ROLE;
+
+-- policy on system catalog
+CREATE POLICY foo ON pg_description FOR SELECT USING (description NOT LIKE 'secret%');
+
+-- reserved schema name
+CREATE SCHEMA pg_foo;
+
+-- drop system table
+DROP TABLE pg_description;
+
+-- truncate of system table
+TRUNCATE pg_description;
+
+-- rename column of system table
+ALTER TABLE pg_description RENAME COLUMN description TO comment;
+
+-- ATSimplePermissions()
+ALTER TABLE pg_description ALTER COLUMN description SET NOT NULL;
+
+-- SET STATISTICS
+ALTER TABLE pg_description ALTER COLUMN description SET STATISTICS -1;
+
+-- foreign key referencing catalog
+CREATE TABLE foo (a oid, b oid, c int, FOREIGN KEY (a, b, c) REFERENCES pg_description);
+
+-- RangeVarCallbackOwnsRelation()
+CREATE INDEX pg_descripton_test_index ON pg_description (description);
+
+-- RangeVarCallbackForAlterRelation()
+ALTER TABLE pg_description RENAME TO pg_comment;
+ALTER TABLE pg_description SET SCHEMA public;
+
+-- reserved tablespace name
+CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
+
+-- triggers
+CREATE FUNCTION tf1() RETURNS trigger
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  RETURN NULL;
+END $$;
+
+CREATE TRIGGER t1 BEFORE INSERT ON pg_description EXECUTE FUNCTION tf1();
+ALTER TRIGGER t1 ON pg_description RENAME TO t2;
+--DROP TRIGGER t2 ON pg_description;
+
+-- rules
+CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+ALTER RULE r1 ON pg_description RENAME TO r2;
+--DROP RULE r2 ON pg_description;
+
+
+SET allow_system_table_ddl = on;
+
+-- create new table in pg_catalog
+BEGIN;
+CREATE TABLE pg_catalog.test (a int);
+ROLLBACK;
+
+-- anyarray column
+BEGIN;
+CREATE TABLE t1 (a int, b anyarray);
+ROLLBACK;
+
+-- index on system catalog
+BEGIN;
+ALTER TABLE pg_namespace ADD UNIQUE USING INDEX pg_namespace_oid_index;
+ROLLBACK;
+
+-- write to system catalog table as superuser
+BEGIN;
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 2, 'foo');
+ROLLBACK;
+
+-- write to system catalog table as normal user
+-- (not allowed)
+SET ROLE regress_user_ast;
+INSERT INTO pg_description (objoid, classoid, objsubid, description) VALUES (0, 0, 3, 'foo');
+RESET ROLE;
+
+-- policy on system catalog
+BEGIN;
+CREATE POLICY foo ON pg_description FOR SELECT USING (description NOT LIKE 'secret%');
+ROLLBACK;
+
+-- reserved schema name
+BEGIN;
+CREATE SCHEMA pg_foo;
+ROLLBACK;
+
+-- drop system table
+-- (This will fail anyway because it's pinned.)
+BEGIN;
+DROP TABLE pg_description;
+ROLLBACK;
+
+-- truncate of system table
+BEGIN;
+TRUNCATE pg_description;
+ROLLBACK;
+
+-- rename column of system table
+BEGIN;
+ALTER TABLE pg_description RENAME COLUMN description TO comment;
+ROLLBACK;
+
+-- ATSimplePermissions()
+BEGIN;
+ALTER TABLE pg_description ALTER COLUMN description SET NOT NULL;
+ROLLBACK;
+
+-- SET STATISTICS
+BEGIN;
+ALTER TABLE pg_description ALTER COLUMN description SET STATISTICS -1;
+ROLLBACK;
+
+-- foreign key referencing catalog
+BEGIN;
+ALTER TABLE pg_description ADD PRIMARY KEY USING INDEX pg_description_o_c_o_index;
+CREATE TABLE foo (a oid, b oid, c int, FOREIGN KEY (a, b, c) REFERENCES pg_description);
+ROLLBACK;
+
+-- RangeVarCallbackOwnsRelation()
+BEGIN;
+CREATE INDEX pg_descripton_test_index ON pg_description (description);
+ROLLBACK;
+
+-- RangeVarCallbackForAlterRelation()
+BEGIN;
+ALTER TABLE pg_description RENAME TO pg_comment;
+ROLLBACK;
+BEGIN;
+ALTER TABLE pg_description SET SCHEMA public;
+ROLLBACK;
+
+-- reserved tablespace name
+CREATE TABLESPACE pg_foo LOCATION '/no/such/location';
+
+-- triggers
+CREATE TRIGGER t1 BEFORE INSERT ON pg_description EXECUTE FUNCTION tf1();
+ALTER TRIGGER t1 ON pg_description RENAME TO t2;
+DROP TRIGGER t2 ON pg_description;
+
+-- rules
+CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+ALTER RULE r1 ON pg_description RENAME TO r2;
+DROP RULE r2 ON pg_description;
+
+
+-- cleanup
+REVOKE ALL ON pg_description FROM regress_user_ast;
+DROP USER regress_user_ast;
+DROP FUNCTION tf1;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 0369909b50..0c87a54b8e 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2016,12 +2016,8 @@ CREATE TEMP TABLE filenode_mapping AS
 
 -- Checks on creating and manipulation of user defined relations in
 -- pg_catalog.
---
--- XXX: It would be useful to add checks around trying to manipulate
--- catalog tables, but that might have ugly consequences when run
--- against an existing server with allow_system_table_mods = on.
 
-SHOW allow_system_table_mods;
+SHOW allow_system_table_ddl;
 -- disallowed because of search_path issues with pg_dump
 CREATE TABLE pg_catalog.new_system_table();
 -- instead create in public first, move to catalog

base-commit: 74b7cc8c02137f059703972aa14445b1e073f005
-- 
2.22.0

view thread (25+ messages)  latest in thread

Message-ID: <a7455db0-2fc0-4ddc-4d8f-e4c860957a34@2ndquadrant.com>
Permalink:  ../a7455db0-2fc0-4ddc-4d8f-e4c860957a34@2ndquadrant.com/
Also on:    postgresql.org/message-id/a7455db0-2fc0-4ddc-4d8f-e4c860957a34@2ndquadrant.com

copy link · copy postgr.es · copy git am

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: pgsql-hackers@postgresql.org
  Cc: peter.eisentraut@2ndquadrant.com
  Subject: Re: allow_system_table_mods stuff
  In-Reply-To: <a7455db0-2fc0-4ddc-4d8f-e4c860957a34@2ndquadrant.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox