Hello, This is a fresh thread to continue the discussion on ALTER TABLE SET ACCESS METHOD when applied to partition roots, as requested.
Current behavior (HEAD): CREATE TABLE am_partitioned(x INT, y INT) PARTITION BY hash (x); ALTER TABLE am_partitioned SET ACCESS METHOD heap2; ERROR: cannot change access method of a partitioned table Potential behavior options: (A) Don't recurse to existing children and ensure that the new am gets inherited by any new children. (ALTER TABLE SET TABLESPACE behavior) (B) Recurse to existing children and modify their am. Also, ensure that any new children inherit the new am. A patch [1] was introduced earlier by Justin to implement (A). v1-0001-Allow-ATSETAM-on-partition-roots.patch contains a rebase of that patch against latest HEAD, with minor updates on comments and some additional test coverage. I think that (B) is necessary for partition hierarchies with a high number of partitions. One typical use case in Greenplum, for instance, is to convert heap tables containing cold data to append-only storage at the root or subroot level of partition hierarchies consisting of thousands of partitions. Asking users to ALTER individual partitions is cumbersome and error-prone. Furthermore, I believe that (B) should be the default and (A) can be chosen by using the ONLY clause. This would give us the best of both worlds and would make the use of ONLY consistent. The patch v1-0002-Make-ATSETAM-recurse-by-default.patch achieves that. Thoughts? Regards, Soumyadeep (VMware) [1] https://www.postgresql.org/message-id/20210308010707.GA29832%40telsasoft.com
From 440517ff8a5912d4c657a464b2c1de9c8b3a4f70 Mon Sep 17 00:00:00 2001 From: Soumyadeep Chakraborty <soumyadeep2...@gmail.com> Date: Tue, 17 May 2022 15:22:48 -0700 Subject: [PATCH v1 2/2] Make ATSETAM recurse by default ATSETAM now recurses to partition children by default. To prevent recursion, and have the new am apply to future children only, users must specify the ONLY clause. --- src/backend/commands/tablecmds.c | 2 ++ src/test/regress/expected/create_am.out | 13 ++++++++++++- src/test/regress/sql/create_am.sql | 5 ++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b3f411dd71f..58251c22ffe 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4711,6 +4711,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot have multiple SET ACCESS METHOD subcommands"))); + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); ATPrepSetAccessMethod(tab, rel, cmd->name); pass = AT_PASS_MISC; /* does not matter; no work in Phase 2 */ break; diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out index a48199df9a2..e99de1ac912 100644 --- a/src/test/regress/expected/create_am.out +++ b/src/test/regress/expected/create_am.out @@ -284,7 +284,7 @@ CREATE TABLE am_partitioned(x INT, y INT) 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; +ALTER TABLE ONLY 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; @@ -296,6 +296,17 @@ SELECT relname, amname FROM pg_class c, pg_am am am_partitioned_3 | heap2 (4 rows) +ALTER TABLE am_partitioned SET ACCESS METHOD heap; +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 | heap + am_partitioned_1 | heap + am_partitioned_2 | heap + am_partitioned_3 | heap +(4 rows) + DROP TABLE am_partitioned; -- Second, create objects in the new AM by changing the default AM BEGIN; diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql index 8f3db03bba2..f4e22684782 100644 --- a/src/test/regress/sql/create_am.sql +++ b/src/test/regress/sql/create_am.sql @@ -184,10 +184,13 @@ CREATE TABLE am_partitioned(x INT, y INT) 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; +ALTER TABLE ONLY 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; +ALTER TABLE am_partitioned SET ACCESS METHOD heap; +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
From e3847f79691509c05196f87134acd89d1a116d68 Mon Sep 17 00:00:00 2001 From: Soumyadeep Chakraborty <soumyadeep2...@gmail.com> Date: Mon, 16 May 2022 14:46:20 -0700 Subject: [PATCH v1 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 <pryz...@telsasoft.com>, Soumyadeep Chakraborty <soumyadeep2...@gmail.com> --- 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..b3f411dd71f 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)); + accessMethodId = ((Form_pg_class) GETSTRUCT(tup))->relam; - /* look up the access method, verify it is for a table */ - if (accessMethod != NULL) - accessMethodId = get_table_am_oid(accessMethod, false); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for relation %u", relid); + + 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 have no storage the tablespace can be updated with a simple + * metadata only operation to update the tablespace. + */ +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