From a9f357d98b90df9ee0ec5f9f9f1ac083d653daaa Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 7 Mar 2021 00:11:38 -0600
Subject: [PATCH] Allow specifying access method of partitioned tables..

..to be inherited by partitions

See also:
ca4103025dfe26eaaf6a500dec9170fbb176eebc
8586bf7ed8889f39a59dd99b292014b73be85342

ebfe2dbd6b624e2a428e14b7ee9322cc096f63f7 - prevent DROP AM

Authors: Justin Pryzby, Soumyadeep Chakraborty
---
 doc/src/sgml/ref/create_table.sgml      |  9 ++-
 src/backend/catalog/heap.c              |  3 +-
 src/backend/commands/tablecmds.c        | 88 +++++++++++++++++++------
 src/backend/utils/cache/lsyscache.c     | 22 +++++++
 src/backend/utils/cache/relcache.c      |  4 ++
 src/bin/pg_dump/pg_dump.c               |  3 +-
 src/bin/pg_dump/t/002_pg_dump.pl        | 16 ++++-
 src/include/utils/lsyscache.h           |  1 +
 src/test/regress/expected/create_am.out | 45 +++++++++----
 src/test/regress/sql/create_am.sql      | 18 +++--
 10 files changed, 167 insertions(+), 42 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 10ef699fab9..b20d272b151 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1295,9 +1295,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       This optional clause specifies the table access method to use to store
       the contents for the new table; the method needs be an access method of
       type <literal>TABLE</literal>. See <xref linkend="tableam"/> for more
-      information.  If this option is not specified, the default table access
-      method is chosen for the new table. See <xref
-      linkend="guc-default-table-access-method"/> for more information.
+      information.  If this option is not specified, a default table access
+      method is chosen for the new table.
+      When creating a partition, the default table access method is the
+      access method of its parent.
+      For other tables, the default is determined by
+      <xref linkend="guc-default-table-access-method"/>.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 2a0d82aedd7..bbf8e08618b 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1452,7 +1452,8 @@ heap_create_with_catalog(const char *relname,
 		 * No need to add an explicit dependency for the toast table, as the
 		 * main table depends on it.
 		 */
-		if (RELKIND_HAS_TABLE_AM(relkind) && relkind != RELKIND_TOASTVALUE)
+		if ((RELKIND_HAS_TABLE_AM(relkind) && relkind != RELKIND_TOASTVALUE) ||
+				relkind == RELKIND_PARTITIONED_TABLE)
 		{
 			ObjectAddressSet(referenced, AccessMethodRelationId, accessmtd);
 			add_exact_object_address(&referenced, addrs);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3147dddf286..71b8b0b2317 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -571,6 +571,7 @@ static ObjectAddress ATExecClusterOn(Relation rel, const char *indexName,
 									 LOCKMODE lockmode);
 static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
 static void ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname);
+static void ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod);
 static bool ATPrepChangePersistence(Relation rel, bool toLogged);
 static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
 								const char *tablespacename, LOCKMODE lockmode);
@@ -680,7 +681,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	Oid			ofTypeId;
 	ObjectAddress address;
 	LOCKMODE	parentLockmode;
-	const char *accessMethod = NULL;
 	Oid			accessMethodId = InvalidOid;
 
 	/*
@@ -945,20 +945,21 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	 * a type of relation that needs one, use the default.
 	 */
 	if (stmt->accessMethod != NULL)
+		accessMethodId = get_table_am_oid(stmt->accessMethod, false);
+	else if (stmt->partbound &&
+			(RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE))
 	{
-		accessMethod = stmt->accessMethod;
-
-		if (partitioned)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("specifying a table access method is not supported on a partitioned table")));
+		/*
+		 * For partitions, if no access method is specified, default to the AM
+		 * of the parent table.
+		 */
+		Assert(list_length(inheritOids) == 1);
+		accessMethodId = get_rel_relam(linitial_oid(inheritOids));
+		if (!OidIsValid(accessMethodId))
+			accessMethodId = get_table_am_oid(default_table_access_method, false);
 	}
-	else if (RELKIND_HAS_TABLE_AM(relkind))
-		accessMethod = default_table_access_method;
-
-	/* look up the access method, verify it is for a table */
-	if (accessMethod != NULL)
-		accessMethodId = get_table_am_oid(accessMethod, false);
+	else if (RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE)
+		accessMethodId = get_table_am_oid(default_table_access_method, false);
 
 	/*
 	 * Create the relation.  Inherited defaults and constraints are passed in
@@ -4787,12 +4788,6 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 		case AT_SetAccessMethod:	/* SET ACCESS METHOD */
 			ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW);
 
-			/* partitioned tables don't have an access method */
-			if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("cannot change access method of a partitioned table")));
-
 			/* check if another access method change was already requested */
 			if (OidIsValid(tab->newAccessMethod))
 				ereport(ERROR,
@@ -5146,6 +5141,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
 			break;
 		case AT_SetAccessMethod:	/* SET ACCESS METHOD */
 			/* handled specially in Phase 3 */
+
+			/*
+			 * Only do this for partitioned tables, for which this is just a
+			 * catalog change.  Tables with storage are handled by Phase 3.
+			 */
+			if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+				ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod);
 			break;
 		case AT_SetTableSpace:	/* SET TABLESPACE */
 
@@ -14541,6 +14543,54 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
 	list_free(reltoastidxids);
 }
 
+/*
+ * Special handling of ALTER TABLE SET ACCESS METHOD for relations with no
+ * storage that have an interest in preserving AM.
+ *
+ * Since these have no storage, setting the access method is a catalog only
+ * operation.
+ */
+static void
+ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod)
+{
+	Relation	pg_class;
+	Oid			relid;
+	Oid			oldrelam;
+	HeapTuple	tuple;
+
+	/*
+	 * Shouldn't be called on relations having storage; these are processed in
+	 * phase 3.
+	 */
+	Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind));
+
+	relid = RelationGetRelid(rel);
+
+	/* Pull the record for this relation and update it */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
+
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", relid);
+
+	oldrelam = ((Form_pg_class) GETSTRUCT(tuple))->relam;
+	((Form_pg_class) GETSTRUCT(tuple))->relam = newAccessMethod;
+	CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+	/* Update dependency on new AM */
+	changeDependencyFor(RelationRelationId, relid, AccessMethodRelationId,
+						oldrelam, newAccessMethod);
+
+	InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0);
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	/* Make sure the relam change is visible */
+	CommandCounterIncrement();
+}
+
 /*
  * Special handling of ALTER TABLE SET TABLESPACE for relations with no
  * storage that have an interest in preserving tablespace.
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index c7607895cdd..39a97cf5d4c 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -2073,6 +2073,28 @@ get_rel_persistence(Oid relid)
 	return result;
 }
 
+/*
+ * get_rel_relam
+ *
+ *		Returns the relam associated with a given relation.
+ */
+Oid
+get_rel_relam(Oid relid)
+{
+	HeapTuple	tp;
+	Form_pg_class reltup;
+	Oid			result;
+
+	tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+	if (!HeapTupleIsValid(tp))
+		elog(ERROR, "cache lookup failed for relation %u", relid);
+	reltup = (Form_pg_class) GETSTRUCT(tp);
+	result = reltup->relam;
+	ReleaseSysCache(tp);
+
+	return result;
+}
+
 
 /*				---------- TRANSFORM CACHE ----------						 */
 
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 40140de9589..08d5f315c87 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1207,6 +1207,10 @@ retry:
 	else if (RELKIND_HAS_TABLE_AM(relation->rd_rel->relkind) ||
 			 relation->rd_rel->relkind == RELKIND_SEQUENCE)
 		RelationInitTableAccessMethod(relation);
+	else if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	{
+		/* Do nothing: it's a catalog settings for partitions to inherit */
+	}
 	else
 		Assert(relation->rd_rel->relam == InvalidOid);
 
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d62780a0880..c0dd029e36d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -16111,7 +16111,8 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 		if (RELKIND_HAS_TABLESPACE(tbinfo->relkind))
 			tablespace = tbinfo->reltablespace;
 
-		if (RELKIND_HAS_TABLE_AM(tbinfo->relkind))
+		if (RELKIND_HAS_TABLE_AM(tbinfo->relkind) ||
+			tbinfo->relkind == RELKIND_PARTITIONED_TABLE)
 			tableam = tbinfo->amname;
 
 		ArchiveEntry(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 42215f82f7a..f1efff96d23 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4380,13 +4380,25 @@ my %tests = (
 		create_sql   => '
 			CREATE TABLE dump_test.regress_pg_dump_table_am_0() USING heap;
 			CREATE TABLE dump_test.regress_pg_dump_table_am_1 (col1 int) USING regress_table_am;
-			CREATE TABLE dump_test.regress_pg_dump_table_am_2() USING heap;',
+			CREATE TABLE dump_test.regress_pg_dump_table_am_2() USING heap;
+			CREATE TABLE dump_test.regress_pg_dump_table_am_parent (id int) PARTITION BY RANGE (id) USING regress_table_am;
+			CREATE TABLE dump_test.regress_pg_dump_table_am_child PARTITION OF dump_test.regress_pg_dump_table_am_parent DEFAULT',
 		regexp => qr/^
 			\QSET default_table_access_method = regress_table_am;\E
 			(\n(?!SET[^;]+;)[^\n]*)*
 			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_1 (\E
 			\n\s+\Qcol1 integer\E
-			\n\);/xm,
+			\n\);
+			(.*\n)*
+			\QSET default_table_access_method = heap;\E
+			(\n(?!SET[^;]+;)[^\n]*)*
+			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_2 (\E
+			(.*\n)*
+			\QSET default_table_access_method = regress_table_am;\E
+			(\n(?!SET[^;]+;)[^\n]*)*
+			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_parent (\E
+			(\n(?!SET[^;]+;)[^\n]*)*
+			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_child (\E/xm,
 		like => {
 			%full_runs, %dump_test_schema_runs, section_pre_data => 1,
 		},
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 4f5418b9728..9f26357d373 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -139,6 +139,7 @@ extern char get_rel_relkind(Oid relid);
 extern bool get_rel_relispartition(Oid relid);
 extern Oid	get_rel_tablespace(Oid relid);
 extern char get_rel_persistence(Oid relid);
+extern Oid	get_rel_relam(Oid relid);
 extern Oid	get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
 extern Oid	get_transform_tosql(Oid typid, Oid langid, List *trftypes);
 extern bool get_typisdefined(Oid typid);
diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out
index e9a9283d7ab..34d86eacf65 100644
--- a/src/test/regress/expected/create_am.out
+++ b/src/test/regress/expected/create_am.out
@@ -176,15 +176,15 @@ SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1;
   1
 (1 row)
 
--- CREATE TABLE ..  PARTITION BY doesn't not support USING
+-- CREATE TABLE ..  PARTITION BY supports USING
+-- new partitions will inherit the AM from their parent table
 CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2;
-ERROR:  specifying a table access method is not supported on a partitioned table
-CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a);
--- new partitions will inherit from the current default, rather the partition root
 SET default_table_access_method = 'heap';
 CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a');
+CREATE TABLE tableam_parted_aa_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('aa') PARTITION BY LIST (a);
 SET default_table_access_method = 'heap2';
 CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b');
+CREATE TABLE tableam_parted_bb_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('bb') PARTITION BY LIST (a);
 RESET default_table_access_method;
 -- but the method can be explicitly specified
 CREATE TABLE tableam_parted_c_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('c') USING heap;
@@ -205,14 +205,19 @@ WHERE pa.oid = pc.relam
 ORDER BY 3, 1, 2;
  relkind | amname |             relname              
 ---------+--------+----------------------------------
+ r       | heap2  | tableam_parted_a_heap2
+ p       | heap2  | tableam_parted_aa_heap2
  r       | heap2  | tableam_parted_b_heap2
+ p       | heap2  | tableam_parted_bb_heap2
  r       | heap2  | tableam_parted_d_heap2
+ p       | heap2  | tableam_parted_heap2
  r       | heap2  | tableam_tbl_heap2
  r       | heap2  | tableam_tblas_heap2
  m       | heap2  | tableam_tblmv_heap2
+ t       | heap2  | toast for tableam_parted_a_heap2
  t       | heap2  | toast for tableam_parted_b_heap2
  t       | heap2  | toast for tableam_parted_d_heap2
-(7 rows)
+(12 rows)
 
 -- Show dependencies onto AM - there shouldn't be any for toast
 SELECT pg_describe_object(classid,objid,objsubid) AS obj
@@ -226,9 +231,13 @@ ORDER BY classid, objid, objsubid;
  table tableam_tbl_heap2
  table tableam_tblas_heap2
  materialized view tableam_tblmv_heap2
+ table tableam_parted_heap2
+ table tableam_parted_a_heap2
+ table tableam_parted_aa_heap2
  table tableam_parted_b_heap2
+ table tableam_parted_bb_heap2
  table tableam_parted_d_heap2
-(5 rows)
+(9 rows)
 
 -- ALTER TABLE SET ACCESS METHOD
 CREATE TABLE heaptable USING heap AS
@@ -284,11 +293,26 @@ ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
 ERROR:  cannot have multiple SET ACCESS METHOD subcommands
 DROP MATERIALIZED VIEW heapmv;
 DROP TABLE heaptable;
--- No support for partitioned tables.
+-- partition hierarchies
+-- upon ALTER, new children will inherit the new am, whereas the existing
+-- children will remain untouched
 CREATE TABLE am_partitioned(x INT, y INT)
   PARTITION BY hash (x);
+CREATE TABLE am_partitioned_1 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 0);
+CREATE TABLE am_partitioned_2 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 1);
+ALTER TABLE am_partitioned_1 SET ACCESS METHOD heap2;
 ALTER TABLE am_partitioned SET ACCESS METHOD heap2;
-ERROR:  cannot change access method of a partitioned table
+CREATE TABLE am_partitioned_3 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 2);
+SELECT relname, amname FROM pg_class c, pg_am am
+  WHERE c.relam = am.oid AND c.relname LIKE 'am_partitioned%' ORDER BY 1;
+     relname      | amname 
+------------------+--------
+ am_partitioned   | heap2
+ am_partitioned_1 | heap2
+ am_partitioned_2 | heap
+ am_partitioned_3 | heap2
+(4 rows)
+
 DROP TABLE am_partitioned;
 -- Second, create objects in the new AM by changing the default AM
 BEGIN;
@@ -326,7 +350,7 @@ ORDER BY 3, 1, 2;
  f       |        | tableam_fdw_heapx
  r       | heap2  | tableam_parted_1_heapx
  r       | heap   | tableam_parted_2_heapx
- p       |        | tableam_parted_heapx
+ p       | heap2  | tableam_parted_heapx
  S       |        | tableam_seq_heapx
  r       | heap2  | tableam_tbl_heapx
  r       | heap2  | tableam_tblas_heapx
@@ -355,7 +379,6 @@ ERROR:  cannot drop access method heap2 because other objects depend on it
 DETAIL:  table tableam_tbl_heap2 depends on access method heap2
 table tableam_tblas_heap2 depends on access method heap2
 materialized view tableam_tblmv_heap2 depends on access method heap2
-table tableam_parted_b_heap2 depends on access method heap2
-table tableam_parted_d_heap2 depends on access method heap2
+table tableam_parted_heap2 depends on access method heap2
 HINT:  Use DROP ... CASCADE to drop the dependent objects too.
 -- we intentionally leave the objects created above alive, to verify pg_dump support
diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql
index 256884c9592..73cef0da4c8 100644
--- a/src/test/regress/sql/create_am.sql
+++ b/src/test/regress/sql/create_am.sql
@@ -124,15 +124,15 @@ CREATE SEQUENCE tableam_seq_heap2 USING heap2;
 CREATE MATERIALIZED VIEW tableam_tblmv_heap2 USING heap2 AS SELECT * FROM tableam_tbl_heap2;
 SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1;
 
--- CREATE TABLE ..  PARTITION BY doesn't not support USING
+-- CREATE TABLE ..  PARTITION BY supports USING
+-- new partitions will inherit the AM from their parent table
 CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2;
-
-CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a);
--- new partitions will inherit from the current default, rather the partition root
 SET default_table_access_method = 'heap';
 CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a');
+CREATE TABLE tableam_parted_aa_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('aa') PARTITION BY LIST (a);
 SET default_table_access_method = 'heap2';
 CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b');
+CREATE TABLE tableam_parted_bb_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('bb') PARTITION BY LIST (a);
 RESET default_table_access_method;
 -- but the method can be explicitly specified
 CREATE TABLE tableam_parted_c_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('c') USING heap;
@@ -183,10 +183,18 @@ ALTER TABLE heaptable SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
 ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
 DROP MATERIALIZED VIEW heapmv;
 DROP TABLE heaptable;
--- No support for partitioned tables.
+-- partition hierarchies
+-- upon ALTER, new children will inherit the new am, whereas the existing
+-- children will remain untouched
 CREATE TABLE am_partitioned(x INT, y INT)
   PARTITION BY hash (x);
+CREATE TABLE am_partitioned_1 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 0);
+CREATE TABLE am_partitioned_2 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 1);
+ALTER TABLE am_partitioned_1 SET ACCESS METHOD heap2;
 ALTER TABLE am_partitioned SET ACCESS METHOD heap2;
+CREATE TABLE am_partitioned_3 PARTITION OF am_partitioned FOR VALUES WITH (MODULUS 3,REMAINDER 2);
+SELECT relname, amname FROM pg_class c, pg_am am
+  WHERE c.relam = am.oid AND c.relname LIKE 'am_partitioned%' ORDER BY 1;
 DROP TABLE am_partitioned;
 
 -- Second, create objects in the new AM by changing the default AM
-- 
2.34.1

