From 80de05d23f964365312c1704dc69e15725e7c26d Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 7 Mar 2021 00:11:38 -0600 Subject: [PATCH v2 2/3] Allow specifying acccess 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 | 86 +++++++++++++++++++++---- src/backend/utils/cache/relcache.c | 4 +- src/test/regress/expected/create_am.out | 20 +++--- src/test/regress/sql/create_am.sql | 6 +- 5 files changed, 93 insertions(+), 26 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 9abc4a1f55..693a94107d 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1462,12 +1462,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 4f62f062fa..73cd6e311b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -510,6 +510,7 @@ static bool ATPrepChangePersistence(Relation rel, bool toLogged); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacename, LOCKMODE lockmode); static void ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname, LOCKMODE lockmode); +static void ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod); static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode); static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace); static void ATExecSetRelOptions(Relation rel, List *defList, @@ -604,7 +605,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, Oid ofTypeId; ObjectAddress address; LOCKMODE parentLockmode; - const char *accessMethod = NULL; Oid accessMethodId = InvalidOid; /* @@ -862,23 +862,33 @@ 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"))); + /* + * 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, ObjectIdDatumGet(relid)); + accessMethodId = ((Form_pg_class) GETSTRUCT(tup))->relam; + 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 == 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 @@ -4751,6 +4761,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, /* nothing to do here, oid columns don't exist anymore */ break; case AT_SetAccessMethod: /* SET ACCESS METHOD */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ATExecSetAccessMethodNoStorage(rel, tab->newAccessMethod); /* Handled specially in Phase 3 */ break; case AT_SetTableSpace: /* SET TABLESPACE */ @@ -13407,6 +13419,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 7ef510cd01..70aa2d93bd 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1212,9 +1212,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/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out index 7ebdd5ded6..3d520c3149 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 SELECT a, repeat(a::text,9999) FROM generate_series(1,9) AS a; @@ -282,7 +285,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 @@ -311,7 +314,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 677a851cd3..4eaa058164 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