From b5542eb5747cf05c599b4d08e0e06d737f54d861 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 7 Mar 2021 00:11:38 -0600 Subject: [PATCH 2/2] Allow specifying access method of partitioned tables.. ..to be inherited by partitions See also: ca4103025dfe26eaaf6a500dec9170fbb176eebc 8586bf7ed8889f39a59dd99b292014b73be85342 ebfe2dbd6b624e2a428e14b7ee9322cc096f63f7 - prevent DROP AM --- src/backend/catalog/heap.c | 3 +- src/backend/commands/tablecmds.c | 76 ++++++++++++++++++------- src/backend/utils/cache/lsyscache.c | 22 +++++++ src/backend/utils/cache/relcache.c | 4 +- src/include/utils/lsyscache.h | 1 + src/test/regress/expected/create_am.out | 21 +++---- src/test/regress/sql/create_am.sql | 6 +- 7 files changed, 98 insertions(+), 35 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 83746d3fd9..57b6146b4f 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1464,12 +1464,13 @@ heap_create_with_catalog(const char *relname, /* * Make a dependency link to force the relation to be deleted if its - * access method is. Do this only for relation and materialized views. + * access method is. Do this only for relevant types. * * No need to add an explicit dependency for the toast table, as the * main table depends on it. */ if (relkind == RELKIND_RELATION || + relkind == RELKIND_PARTITIONED_TABLE || relkind == RELKIND_MATVIEW) { ObjectAddressSet(referenced, AccessMethodRelationId, accessmtd); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index ca30399501..ed058b8b99 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -540,6 +540,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); @@ -646,7 +647,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, Oid ofTypeId; ObjectAddress address; LOCKMODE parentLockmode; - const char *accessMethod = NULL; Oid accessMethodId = InvalidOid; /* @@ -908,23 +908,23 @@ 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 == RELKIND_RELATION) { - 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 partitioned tables, when no access method is specified, we + * default to the parent table's AM. + */ + 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 == RELKIND_RELATION || relkind == RELKIND_TOASTVALUE || + relkind == RELKIND_PARTITIONED_TABLE || relkind == RELKIND_MATVIEW) - 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); + accessMethodId = get_table_am_oid(default_table_access_method, false); /* * Create the relation. Inherited defaults and constraints are passed in @@ -4628,12 +4628,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, @@ -5020,6 +5014,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_SetAccessMethod: /* SET ACCESS METHOD */ /* handled specially in Phase 3 */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod); break; case AT_SetTableSpace: /* SET TABLESPACE */ @@ -13993,6 +13989,48 @@ 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; + 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); + + ((Form_pg_class) GETSTRUCT(tuple))->relam = newAccessMethod; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + + 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 6bba5f8ec4..703d8d06be 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -2062,6 +2062,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 13d9994af3..35c72fa74d 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1187,9 +1187,11 @@ RelationBuildDesc(Oid targetRelId, bool insertIt) case RELKIND_VIEW: case RELKIND_COMPOSITE_TYPE: case RELKIND_FOREIGN_TABLE: - case RELKIND_PARTITIONED_TABLE: Assert(relation->rd_rel->relam == InvalidOid); break; + case RELKIND_PARTITIONED_TABLE: + /* Do nothing: it's a catalog settings for partitions to inherit */ + break; } /* extract reloptions if any */ diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index 77871aaefc..10145570fd 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 22d6a06910..19e4c19bfb 100644 --- a/src/test/regress/expected/create_am.out +++ b/src/test/regress/expected/create_am.out @@ -176,11 +176,9 @@ SELECT f1 FROM tableam_tblmv_heap2 ORDER BY f1; 1 (1 row) --- CREATE TABLE .. PARTITION BY doesn't not support USING -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); +-- CREATE TABLE .. PARTITION BY supports USING -- new partitions will inherit from the current default, rather the partition root +CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; SET default_table_access_method = 'heap'; CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a'); SET default_table_access_method = 'heap2'; @@ -205,14 +203,17 @@ WHERE pa.oid = pc.relam ORDER BY 3, 1, 2; relkind | amname | relname ---------+--------+---------------------------------- + r | heap2 | tableam_parted_a_heap2 r | heap2 | tableam_parted_b_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) +(10 rows) -- Show dependencies onto AM - there shouldn't be any for toast SELECT pg_describe_object(classid,objid,objsubid) AS obj @@ -226,9 +227,11 @@ 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_b_heap2 table tableam_parted_d_heap2 -(5 rows) +(7 rows) -- ALTER TABLE SET ACCESS METHOD CREATE TABLE heaptable USING heap AS @@ -262,7 +265,6 @@ CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x); -- negative test ALTER TABLE am_partitioned SET ACCESS METHOD heap2; -ERROR: cannot change access method of a partitioned table DROP TABLE am_partitioned; -- Second, create objects in the new AM by changing the default AM BEGIN; @@ -300,7 +302,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 @@ -329,7 +331,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 d0d39d71ec..28984cab1f 100644 --- a/src/test/regress/sql/create_am.sql +++ b/src/test/regress/sql/create_am.sql @@ -124,11 +124,9 @@ 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 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); +-- CREATE TABLE .. PARTITION BY supports USING -- new partitions will inherit from the current default, rather the partition root +CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2; SET default_table_access_method = 'heap'; CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a'); SET default_table_access_method = 'heap2'; -- 2.17.0