From de44be520f8c157dc558f63dd4059d80b6ad00e9 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 2 May 2024 13:45:18 +0900 Subject: [PATCH v2 3/4] Support LOGGED/UNLOGGED for partitioned tables When using ALTER TABLE SET LOGGED/UNLOGGED, indexes and sequences that are owned by the partitioned table changed need to have their relpersistence also changed. This patch does not apply recursion when using ALTER TABLE on a partitioned table: only new partitions inherit the loggedness of the parent if the query does not give a persistence (either LOGGED or UNLOGGED). --- src/backend/commands/tablecmds.c | 118 +++++++++++++++++++++- src/test/regress/expected/alter_table.out | 88 ++++++++++++++++ src/test/regress/sql/alter_table.sql | 37 +++++++ doc/src/sgml/ref/alter_table.sgml | 6 ++ doc/src/sgml/ref/create_table.sgml | 5 + 5 files changed, 249 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 97ba34f0d5..8854e31f14 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -602,6 +602,7 @@ static ObjectAddress ATExecClusterOn(Relation rel, const char *indexName, 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 void ATExecSetPersistenceNoStorage(Relation rel, char newrelpersistence); static void ATPrepSetPersistence(AlteredTableInfo *tab, Relation rel, bool toLogged); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, @@ -811,13 +812,39 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, } /* - * If the grammar did not specify a relpersistence, assume that the - * relation is permanent. Note that this is done before selecting - * the relation's tablespace, as this change may impact the tablespace - * location depending on the persistence set here. + * If the grammar did not specify a relpersistence, determine which one + * to use depending on the relation to create. Note that this is done + * before selecting the relation's tablespace, as this change may impact + * the tablespace location depending on the persistence set here. + * + * If the relation is not partitioned, assume that it is permanent. + * + * A partition inherits the persistence of its partitioned table, it the + * latter is unlogged or logged as the namespace where the relation will + * be created is known. This property cannot be enforced for temporary + * partitioned tables because the namespace of the relation is locked + * before it is possible to know the inheritance tree of this new + * relation, when its RangeVar is locked earlier when transforming the + * CreateStmt query. */ if (stmt->relation->relpersistence == RELPERSISTENCE_INVALID) - stmt->relation->relpersistence = RELPERSISTENCE_PERMANENT; + { + if (stmt->partbound) + { + Oid parentOid = linitial_oid(inheritOids); + char relpersistence = get_rel_persistence(parentOid); + + Assert(list_length(inheritOids) == 1); + /* + * The parent's persistence is logged or unlogged, so rely on + * it when creating the new relation. + */ + if (relpersistence != RELPERSISTENCE_TEMP) + stmt->relation->relpersistence = relpersistence; + } + else + stmt->relation->relpersistence = RELPERSISTENCE_PERMANENT; + } /* * Select tablespace to use: an explicitly indicated one, or (in the case @@ -5426,6 +5453,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_SetLogged: /* SET LOGGED */ case AT_SetUnLogged: /* SET UNLOGGED */ + + /* + * 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 && + tab->chgPersistence) + ATExecSetPersistenceNoStorage(rel, tab->newrelpersistence); break; case AT_DropOids: /* SET WITHOUT OIDS */ /* nothing to do here, oid columns don't exist anymore */ @@ -15542,6 +15577,79 @@ ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId) table_close(pg_class, RowExclusiveLock); } +/* + * Special handling of ALTER TABLE SET LOGGED/UNLOGGED for relations with no + * storage that have an interest in changing their persistence. + * + * Since these have no storage, setting the persistence to permanent or + * unlogged is a catalog-only operation. This needs to switch the + * persistence of all sequences and indexes related to this relation. + */ +static void +ATExecSetPersistenceNoStorage(Relation rel, char newrelpersistence) +{ + Relation pg_class; + HeapTuple tuple; + List *reloids; /* for indexes and sequences */ + ListCell *elt; + Form_pg_class rd_rel; + Oid reloid = RelationGetRelid(rel); + + /* + * Shouldn't be called on relations having storage; these are processed in + * phase 3. + */ + Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)); + + /* Get a modifiable copy of the relation's pg_class row. */ + pg_class = table_open(RelationRelationId, RowExclusiveLock); + + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", reloid); + rd_rel = (Form_pg_class) GETSTRUCT(tuple); + + /* Leave if no update required */ + if (rd_rel->relpersistence == newrelpersistence) + { + heap_freetuple(tuple); + table_close(pg_class, RowExclusiveLock); + return; + } + + /* Update the pg_class row. */ + rd_rel->relpersistence = newrelpersistence; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + + InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0); + + heap_freetuple(tuple); + + /* Update the per-sequence and per-index relpersistence */ + reloids = getOwnedSequences(reloid); + reloids = list_union_oid(reloids, RelationGetIndexList(rel)); + foreach(elt, reloids) + { + Oid classoid = lfirst_oid(elt); + + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(classoid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", classoid); + rd_rel = (Form_pg_class) GETSTRUCT(tuple); + + rd_rel->relpersistence = newrelpersistence; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + InvokeObjectPostAlterHook(RelationRelationId, classoid, 0); + + heap_freetuple(tuple); + } + + /* Make sure the persistence changes are visible */ + CommandCounterIncrement(); + + table_close(pg_class, RowExclusiveLock); +} + /* * ALTER TABLE SET TABLESPACE */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 7666c76238..5071e7d963 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3597,6 +3597,94 @@ ALTER TABLE logged1 SET UNLOGGED; -- silently do nothing DROP TABLE logged3; DROP TABLE logged2; DROP TABLE logged1; +-- SET LOGGED/UNLOGGED with partitioned tables +CREATE TABLE logged_part_1(f1 SERIAL PRIMARY KEY) + PARTITION BY LIST (f1); -- has sequence, index +CREATE TABLE logged_part_2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_1) + PARTITION BY LIST (f1); -- foreign key +CREATE TABLE logged_part_3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_3) + PARTITION BY LIST (f1); -- self-referencing foreign key +-- Check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_1 | p + logged_part_1_f1_seq | p + logged_part_1_pkey | p + logged_part_2 | p + logged_part_2_f1_seq | p + logged_part_2_pkey | p + logged_part_3 | p + logged_part_3_f1_seq | p + logged_part_3_pkey | p +(9 rows) + +ALTER TABLE logged_part_1 SET UNLOGGED; -- fails as a foreign-key exists +ERROR: could not change table "logged_part_1" to unlogged because it references logged table "logged_part_2" +ALTER TABLE logged_part_2 SET UNLOGGED; +ALTER TABLE logged_part_3 SET UNLOGGED; -- skip self-referencing foreign key +-- Re-check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_1 | p + logged_part_1_f1_seq | p + logged_part_1_pkey | p + logged_part_2 | u + logged_part_2_f1_seq | u + logged_part_2_pkey | u + logged_part_3 | u + logged_part_3_f1_seq | u + logged_part_3_pkey | u +(9 rows) + +ALTER TABLE logged_part_1 SET LOGGED; -- no-op +ALTER TABLE logged_part_2 SET LOGGED; +ALTER TABLE logged_part_3 SET LOGGED; -- skip self-referencing foreign key +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_[2|3]' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_2 | p + logged_part_2_f1_seq | p + logged_part_2_pkey | p + logged_part_3 | p + logged_part_3_f1_seq | p + logged_part_3_pkey | p +(6 rows) + +-- Partitions +CREATE TABLE logged_part_2_1 PARTITION OF logged_part_2 + FOR VALUES IN (1); -- permanent, inherited +CREATE UNLOGGED TABLE logged_part_2_2 PARTITION OF logged_part_2 + FOR VALUES IN (2); -- unlogged, not inherited +ALTER TABLE logged_part_2 SET UNLOGGED; +CREATE TABLE logged_part_2_3 PARTITION OF logged_part_2 + FOR VALUES IN (3); -- unlogged, inherited +CREATE LOGGED TABLE logged_part_2_4 PARTITION OF logged_part_2 + FOR VALUES IN (4); -- logged, not inherited +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_2' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_2 | u + logged_part_2_1 | p + logged_part_2_1_pkey | p + logged_part_2_2 | u + logged_part_2_2_pkey | u + logged_part_2_3 | u + logged_part_2_3_pkey | u + logged_part_2_4 | p + logged_part_2_4_pkey | p + logged_part_2_f1_seq | u + logged_part_2_pkey | u +(11 rows) + +DROP TABLE logged_part_3; +DROP TABLE logged_part_2; +DROP TABLE logged_part_1; -- test ADD COLUMN IF NOT EXISTS CREATE TABLE test_add_column(c1 integer); \d test_add_column diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 9df5a63bdf..30aa62d256 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -2259,6 +2259,43 @@ DROP TABLE logged3; DROP TABLE logged2; DROP TABLE logged1; +-- SET LOGGED/UNLOGGED with partitioned tables +CREATE TABLE logged_part_1(f1 SERIAL PRIMARY KEY) + PARTITION BY LIST (f1); -- has sequence, index +CREATE TABLE logged_part_2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_1) + PARTITION BY LIST (f1); -- foreign key +CREATE TABLE logged_part_3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_3) + PARTITION BY LIST (f1); -- self-referencing foreign key +-- Check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; +ALTER TABLE logged_part_1 SET UNLOGGED; -- fails as a foreign-key exists +ALTER TABLE logged_part_2 SET UNLOGGED; +ALTER TABLE logged_part_3 SET UNLOGGED; -- skip self-referencing foreign key +-- Re-check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; +ALTER TABLE logged_part_1 SET LOGGED; -- no-op +ALTER TABLE logged_part_2 SET LOGGED; +ALTER TABLE logged_part_3 SET LOGGED; -- skip self-referencing foreign key +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_[2|3]' + ORDER BY relname; +-- Partitions +CREATE TABLE logged_part_2_1 PARTITION OF logged_part_2 + FOR VALUES IN (1); -- permanent, inherited +CREATE UNLOGGED TABLE logged_part_2_2 PARTITION OF logged_part_2 + FOR VALUES IN (2); -- unlogged, not inherited +ALTER TABLE logged_part_2 SET UNLOGGED; +CREATE TABLE logged_part_2_3 PARTITION OF logged_part_2 + FOR VALUES IN (3); -- unlogged, inherited +CREATE LOGGED TABLE logged_part_2_4 PARTITION OF logged_part_2 + FOR VALUES IN (4); -- logged, not inherited +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_2' + ORDER BY relname; +DROP TABLE logged_part_3; +DROP TABLE logged_part_2; +DROP TABLE logged_part_1; + -- test ADD COLUMN IF NOT EXISTS CREATE TABLE test_add_column(c1 integer); \d test_add_column diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index ebd8c62038..7937194462 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -803,6 +803,12 @@ WITH ( MODULUS numeric_literal, REM (for identity or serial columns). However, it is also possible to change the persistence of such sequences separately. + + + Setting this property for a partitioned table has no direct effect, + because such tables have no storage of their own, but the configured + value will be inherited by newly-created partitions. + diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 29dfd68dc8..f65c6d14c7 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -216,6 +216,11 @@ WITH ( MODULUS numeric_literal, REM If this is specified, any sequences created together with the permanent table (for identity or serial columns) are also created as permanent. + + + When applied to a partitioned table, newly-created partitions and + their objects (sequences and indexes) will inherit this property. + -- 2.43.0