From a9fc146192430aa45224fb1e64bd95e0bb64fc00 Mon Sep 17 00:00:00 2001 From: Soumyadeep Chakraborty Date: Thu, 9 Jun 2022 10:57:35 -0700 Subject: [PATCH v3 1/2] Allow ATSETAM on partition roots Setting the access method on partition roots was disallowed. This commit relaxes that restriction. Summary of changes over original patch: * Rebased * Minor comment updates * Add more test coverage Authors: Justin Pryzby , Soumyadeep Chakraborty --- src/backend/catalog/heap.c | 3 +- src/backend/commands/tablecmds.c | 103 +++++++++++++++++++----- src/backend/utils/cache/relcache.c | 4 + src/test/regress/expected/create_am.out | 50 +++++++----- src/test/regress/sql/create_am.sql | 23 +++--- 5 files changed, 129 insertions(+), 54 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 1803194db94..4561f3b8c98 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1449,7 +1449,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 2de0ebacec3..9a5eabcac49 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -570,6 +570,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); @@ -676,7 +677,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, Oid ofTypeId; ObjectAddress address; LOCKMODE parentLockmode; - const char *accessMethod = NULL; Oid accessMethodId = InvalidOid; /* @@ -938,20 +938,32 @@ 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; + HeapTuple tup; + Oid relid; - if (partitioned) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("specifying a table access method is not supported on a partitioned table"))); - } - else if (RELKIND_HAS_TABLE_AM(relkind)) - accessMethod = default_table_access_method; + /* + * For partitioned tables, when no access method is specified, we + * default to the parent table's AM. + */ + Assert(list_length(inheritOids) == 1); + /* XXX: should implement get_rel_relam? */ + relid = linitial_oid(inheritOids); + tup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for relation %u", relid); - /* look up the access method, verify it is for a table */ - if (accessMethod != NULL) - accessMethodId = get_table_am_oid(accessMethod, false); + accessMethodId = ((Form_pg_class) GETSTRUCT(tup))->relam; + + ReleaseSysCache(tup); + + if (!OidIsValid(accessMethodId)) + accessMethodId = get_table_am_oid(default_table_access_method, 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 @@ -4693,13 +4705,6 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, break; 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, @@ -5085,7 +5090,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, /* nothing to do here, oid columns don't exist anymore */ 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. Other relation types which have storage are + * handled by Phase 3. + */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod); break; case AT_SetTableSpace: /* SET TABLESPACE */ @@ -14421,6 +14432,58 @@ 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 relations have no storage the access method can be updated with a + * simple metadata 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); + + /* + * Record dependency on AM. This is only required for relations + * that have no physical storage. + */ + changeDependencyFor(RelationRelationId, RelationGetRelid(rel), + 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/relcache.c b/src/backend/utils/cache/relcache.c index 60e72f9e8bf..7e955c44173 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1206,6 +1206,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: the am is a catalog setting for partitions to inherit */ + } else Assert(relation->rd_rel->relam == InvalidOid); diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out index e9a9283d7ab..a48199df9a2 100644 --- a/src/test/regress/expected/create_am.out +++ b/src/test/regress/expected/create_am.out @@ -176,19 +176,12 @@ 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 from the partition root 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'); -SET default_table_access_method = 'heap2'; -CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b'); -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; -CREATE TABLE tableam_parted_d_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('d') USING heap2; +CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b') USING heap; -- List all objects in AM SELECT pc.relkind, @@ -205,14 +198,13 @@ WHERE pa.oid = pc.relam ORDER BY 3, 1, 2; relkind | amname | relname ---------+--------+---------------------------------- - r | heap2 | tableam_parted_b_heap2 - r | heap2 | tableam_parted_d_heap2 + r | heap2 | tableam_parted_a_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_b_heap2 - t | heap2 | toast for tableam_parted_d_heap2 -(7 rows) + t | heap2 | toast for tableam_parted_a_heap2 +(6 rows) -- Show dependencies onto AM - there shouldn't be any for toast SELECT pg_describe_object(classid,objid,objsubid) AS obj @@ -226,8 +218,8 @@ ORDER BY classid, objid, objsubid; table tableam_tbl_heap2 table tableam_tblas_heap2 materialized view tableam_tblmv_heap2 - table tableam_parted_b_heap2 - table tableam_parted_d_heap2 + table tableam_parted_heap2 + table tableam_parted_a_heap2 (5 rows) -- ALTER TABLE SET ACCESS METHOD @@ -284,11 +276,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 +333,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 +362,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..8f3db03bba2 100644 --- a/src/test/regress/sql/create_am.sql +++ b/src/test/regress/sql/create_am.sql @@ -124,19 +124,12 @@ 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 from the partition root 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'); -SET default_table_access_method = 'heap2'; -CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b'); -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; -CREATE TABLE tableam_parted_d_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('d') USING heap2; +CREATE TABLE tableam_parted_b_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('b') USING heap; -- List all objects in AM SELECT @@ -183,10 +176,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.25.1