Jian, Zsolt, Thank you both for your valuable catches. Attached is v3 addressing the points raised.
On Fri, Aug 7, 2026 at 6:42 AM jian he <[email protected]> wrote: > On Fri, Aug 7, 2026 at 6:59 AM Zsolt Parragi <[email protected]> > wrote: > > > Copying the value of generated column "as is" can produce data that > > > differs from > > > what the generated expression would compute if any merged partition's > > > generation > > > expression differs from the partitioned table's. > > > > I think this would be probably fine, as we can get the same effect by > > replacing a function used by the expression, a preexisting condition > > for many existing cases. But I do agree that requiring the same > > expression is a better approach. > > > > Also, not directly related to this patch, but now that I looked into > > this, I can still use tableoids for check constraints with a text > > cast: > > > > CREATE TABLE t (i int) PARTITION BY RANGE (i); > > CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); > > CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); > > ALTER TABLE t ADD CONSTRAINT cc CHECK (tableoid::regclass::text <> > > 'tp_0_2'); > > INSERT INTO t VALUES (0),(1); > > ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- > > SUCCESS, but should ERROR instead? > > Interesting! > > Before we call MergePartitionsMoveRows, we did RestrictSearchPath(), > which will set GUC search_path > to "pg_catalog, pg_temp" temporally, and text_regclass will consider > search_path when resolve object name. > > On the other hand, if we unconditionally validate all the partitioned > table's inherited CHECK constraints, it may fail > and the resulting message isn't helpful. > The error message below shows what happens when evaluating all CHECK > constraints during MERGE PARTITIONS. > > DROP TABLE IF EXISTS t; > CREATE TABLE t (i int, b text default 't') PARTITION BY RANGE (i); > CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); > CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); > ALTER TABLE t ADD CONSTRAINT cc CHECK (b::regclass::text in ('t', > 'tp_0_1', 'tp_0_2', 'tp_1_2')); > INSERT INTO t VALUES (0); > INSERT INTO t VALUES (0, 'tp_0_1'), (1, 'tp_1_2'), (1, 'public.tp_1_2'); > ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; > ERROR: relation "t" does not exist I see that virtual generated columns also can lead to the problems. The revised 0003 rejects the dependency regardless of the generated column kind, in a new checkPartitionSystemColumnRefs() called before the new partition is created. I confirm that CHECK constraints depending on a system column are also problematic. The revised 0003 rejects CHECK constraints referencing a system column as well, for the same reason as generated columns. Since nothing needs re-verification anymore, the machinery that did it is removed: buildPartitionCheckExprStates(), checkPartitionRowConstraints(), the AlteredTableInfo.constraints population, and the work queue entry and arguments that existed only to carry them. 0002 also refuses to create the new partition in a schema whose FOR TABLES IN SCHEMA publications differ from those of the source partitions, since that would silently add the relocated rows to, or remove them from, such a publication. The check triggers only when a schema publication is actually involved, so a cross-schema MERGE/SPLIT is still allowed otherwise; publications FOR ALL TABLES, or covering the partitioned table itself, keep covering the new partitions and are unaffected. Agreed that the previous wording recommended something that runs into your data-loss scenario. The paragraph now just states the facts: if changes are published for the partitioned table itself, subscribers are unaffected and may keep their own partition layout; otherwise the new partition is not part of the subscription until it is refreshed, and changes made in the meantime are not applied – so refreshing without copying its data would silently lose them. On the rewrite event trigger: MERGE/SPLIT doesn't fire table_rewrite, and I don't think it should. table_rewrite reports a single table that keeps its identity while getting a new relfilenode. MERGE turns N partitions into one new relation and SPLIT one into N, dropping the originals, so there is no single "table being rewritten" to report. The commands are still visible to ddl_command_start/ddl_command_end as an ALTER TABLE. ------ Regards, Alexander Korotkov Supabase
From ffca690258ecdc00f606686f4bc6046bfbd4f49f Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Mon, 3 Aug 2026 00:08:36 +0200 Subject: [PATCH v3 1/3] Don't logically decode MERGE/SPLIT PARTITION row movement ALTER TABLE ... MERGE/SPLIT PARTITION relocates rows between partitions of the same partitioned table by re-inserting them into the freshly created partition(s), using plain heap inserts. Logical decoding emitted those as INSERTs into the new partition with no matching DELETEs for the source rows, which corrupts logical replication subscribers. Pass TABLE_INSERT_NO_LOGICAL to the movers so the relocation is not decoded, just as CLUSTER and VACUUM FULL already do for their rewrites. MERGE/SPLIT PARTITION is a schema change that is not itself replicated, and the moved rows still exist on subscribers, so suppressing the inserts keeps them consistent. Document the behavior in the MERGE PARTITIONS and SPLIT PARTITION commands descriptions, and add a test_decoding regression test. Discussion: https://postgr.es/m/CAN4CZFNCU=t09M=+r2t9hHLJuujdM4oQ8hCK_Sx-GpfiwMAicw@mail.gmail.com --- contrib/test_decoding/Makefile | 3 +- .../expected/partition_merge_split.out | 56 +++++++++++++++++++ contrib/test_decoding/meson.build | 1 + .../sql/partition_merge_split.sql | 34 +++++++++++ doc/src/sgml/ref/alter_table.sgml | 30 ++++++++++ src/backend/commands/tablecmds.c | 20 +++++-- 6 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 contrib/test_decoding/expected/partition_merge_split.out create mode 100644 contrib/test_decoding/sql/partition_merge_split.sql diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index 0111124399a..ab90cd7fec2 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -5,7 +5,8 @@ PGFILEDESC = "test_decoding - example of a logical decoding output plugin" REGRESS = ddl xact rewrite toast permissions decoding_in_xact \ decoding_into_rel binary prepared replorigin time messages \ - repack spill slot truncate stream stats twophase twophase_stream + repack spill slot truncate stream stats twophase twophase_stream \ + partition_merge_split ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \ oldest_xmin snapshot_transfer subxact_without_top concurrent_stream \ twophase_snapshot slot_creation_error catalog_change_snapshot \ diff --git a/contrib/test_decoding/expected/partition_merge_split.out b/contrib/test_decoding/expected/partition_merge_split.out new file mode 100644 index 00000000000..63ec5af98d0 --- /dev/null +++ b/contrib/test_decoding/expected/partition_merge_split.out @@ -0,0 +1,56 @@ +-- Row movement performed by ALTER TABLE ... MERGE/SPLIT PARTITION must not be +-- logically decoded: the relocation is physical (like CLUSTER/VACUUM FULL) and +-- the DDL itself is not replicated, so emitting INSERTs for the moved rows +-- (without matching DELETEs) would corrupt logical subscribers. +SET synchronous_commit = on; +CREATE TABLE part (id int PRIMARY KEY) PARTITION BY RANGE (id); +CREATE TABLE part_1 PARTITION OF part FOR VALUES FROM (0) TO (10); +CREATE TABLE part_2 PARTITION OF part FOR VALUES FROM (10) TO (20); +SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding'); + ?column? +---------- + init +(1 row) + +INSERT INTO part VALUES (1), (11); +-- Drain the two INSERTs. +SELECT count(*) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + count +------- + 4 +(1 row) + +-- MERGE: the relocation of the rows must not be decoded, so nothing (no +-- INSERTs, and with skip-empty-xacts no empty transaction either) is emitted. +ALTER TABLE part MERGE PARTITIONS (part_1, part_2) INTO part_merged; +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + data +------ +(0 rows) + +-- SPLIT: likewise. +ALTER TABLE part SPLIT PARTITION part_merged INTO + (PARTITION part_1 FOR VALUES FROM (0) TO (10), + PARTITION part_2 FOR VALUES FROM (10) TO (20)); +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + data +------ +(0 rows) + +-- A normal INSERT is still decoded afterwards. +INSERT INTO part VALUES (2); +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + data +-------------------------------------------- + BEGIN + table public.part_1: INSERT: id[integer]:2 + COMMIT +(3 rows) + +SELECT 'stop' FROM pg_drop_replication_slot('regression_slot'); + ?column? +---------- + stop +(1 row) + +DROP TABLE part; diff --git a/contrib/test_decoding/meson.build b/contrib/test_decoding/meson.build index ac655853d26..a504bc00794 100644 --- a/contrib/test_decoding/meson.build +++ b/contrib/test_decoding/meson.build @@ -42,6 +42,7 @@ tests += { 'stats', 'twophase', 'twophase_stream', + 'partition_merge_split', ], 'regress_args': [ '--temp-config', files('logical.conf'), diff --git a/contrib/test_decoding/sql/partition_merge_split.sql b/contrib/test_decoding/sql/partition_merge_split.sql new file mode 100644 index 00000000000..efdd6019ebd --- /dev/null +++ b/contrib/test_decoding/sql/partition_merge_split.sql @@ -0,0 +1,34 @@ +-- Row movement performed by ALTER TABLE ... MERGE/SPLIT PARTITION must not be +-- logically decoded: the relocation is physical (like CLUSTER/VACUUM FULL) and +-- the DDL itself is not replicated, so emitting INSERTs for the moved rows +-- (without matching DELETEs) would corrupt logical subscribers. +SET synchronous_commit = on; + +CREATE TABLE part (id int PRIMARY KEY) PARTITION BY RANGE (id); +CREATE TABLE part_1 PARTITION OF part FOR VALUES FROM (0) TO (10); +CREATE TABLE part_2 PARTITION OF part FOR VALUES FROM (10) TO (20); + +SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding'); + +INSERT INTO part VALUES (1), (11); + +-- Drain the two INSERTs. +SELECT count(*) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + +-- MERGE: the relocation of the rows must not be decoded, so nothing (no +-- INSERTs, and with skip-empty-xacts no empty transaction either) is emitted. +ALTER TABLE part MERGE PARTITIONS (part_1, part_2) INTO part_merged; +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + +-- SPLIT: likewise. +ALTER TABLE part SPLIT PARTITION part_merged INTO + (PARTITION part_1 FOR VALUES FROM (0) TO (10), + PARTITION part_2 FOR VALUES FROM (10) TO (20)); +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + +-- A normal INSERT is still decoded afterwards. +INSERT INTO part VALUES (2); +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + +SELECT 'stop' FROM pg_drop_replication_slot('regression_slot'); +DROP TABLE part; diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index ff7071bef5b..b8246a7ee48 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -1281,6 +1281,21 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM dependencies are not silently lost during merge. </para> + <para> + Moving rows into the new partition does not emit logical replication + messages, in the same way that <command>CLUSTER</command> or + <command>VACUUM FULL</command> do not. Note that + <command>ALTER TABLE ... MERGE PARTITIONS</command> is a schema change and + is not itself replicated to logical replication subscribers. If changes + are published for the partitioned table itself (see + <literal>publish_via_partition_root</literal>), subscribers are unaffected + and may keep their own partition layout. Otherwise changes are published + for the individual partitions, and the new partition is not part of the + subscription until the subscription is refreshed; changes made to it in + the meantime are not applied, so refreshing without copying its data would + silently lose them. + </para> + <note> <para> Merging partitions acquires an <literal>ACCESS EXCLUSIVE</literal> lock on @@ -1386,6 +1401,21 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the source partition's indexes. </para> + <para> + Moving rows into the new partitions does not emit logical replication + messages, in the same way that <command>CLUSTER</command> or + <command>VACUUM FULL</command> do not. Note that + <command>ALTER TABLE ... SPLIT PARTITION</command> is a schema change and + is not itself replicated to logical replication subscribers. If changes + are published for the partitioned table itself (see + <literal>publish_via_partition_root</literal>), subscribers are unaffected + and may keep their own partition layout. Otherwise changes are published + for the individual partitions, and the new partitions are not part of the + subscription until the subscription is refreshed; changes made to them in + the meantime are not applied, so refreshing without copying their data + would silently lose them. + </para> + <note> <para> Split partition acquires an <literal>ACCESS EXCLUSIVE</literal> lock on diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6d4c457b820..0eb85c1be17 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -23366,8 +23366,16 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart AlteredTableInfo *tab; ListCell *ltab; - /* The FSM is empty, so don't bother using it. */ - uint32 ti_options = TABLE_INSERT_SKIP_FSM; + /* + * The FSM is empty, so don't bother using it. Also suppress logical + * decoding of these inserts: merging partitions physically relocates rows + * within the same partitioned table, much like CLUSTER or VACUUM FULL. + * The relocation is not a user-level INSERT, and MERGE PARTITIONS is DDL + * that logical replication does not replicate anyway; emitting INSERTs + * for the moved rows (with no matching DELETEs for the source rows) would + * corrupt logical subscribers. + */ + uint32 ti_options = TABLE_INSERT_SKIP_FSM | TABLE_INSERT_NO_LOGICAL; BulkInsertState bistate; /* state of bulk inserts for partition */ TupleTableSlot *dstslot; @@ -24034,8 +24042,12 @@ static void SplitPartitionMoveRows(List **wqueue, Relation rel, Relation splitRel, List *partlist, List *newPartRels) { - /* The FSM is empty, so don't bother using it. */ - uint32 ti_options = TABLE_INSERT_SKIP_FSM; + /* + * The FSM is empty, so don't bother using it. Suppress logical decoding + * of these inserts as well; see the matching comment in + * MergePartitionsMoveRows(). + */ + uint32 ti_options = TABLE_INSERT_SKIP_FSM | TABLE_INSERT_NO_LOGICAL; CommandId mycid; EState *estate; ListCell *listptr, -- 2.55.0
From 029ed90d8398828f8ab6df7d70cce9c84f453411 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Mon, 3 Aug 2026 00:14:57 +0200 Subject: [PATCH v3 2/3] Peserve replica identity and publications in MERGE/SPLIT PARTITION(s) The new partition(s) created by ALTER TABLE ... MERGE/SPLIT PARTITION are built from the partitioned-table template, so they would default to REPLICA IDENTITY DEFAULT and silently drop out of any publication that the source partitions were directly part of, changing replication behavior without a warning. Carry a uniform, simply-representable replica identity (DEFAULT, FULL or NOTHING) from the source partitions to the new partition(s). Raise an error if the sources disagree, or use an index-based identity that cannot be reproduced automatically, and let the user set it explicitly. Also refuse the operation when any source partition is a direct member of a publication: the new partition would otherwise leave it, and faithfully reproducing per-relation column lists and row filters is ambiguous (especially when several sources are merged). Publications that cover the partitioned root continue to include the new partition, so those are unaffected. For the same reason, refuse to create the new partition in a schema whose FOR TABLES IN SCHEMA publications differ from those of the source partitions: such a move would silently add the relocated rows to, or remove them from, such a publication. The check only triggers when a schema publication is actually involved, so a cross-schema MERGE/SPLIT remains allowed otherwise; publications FOR ALL TABLES, or covering the partitioned table itself, keep covering the new partitions and are unaffected. Document this behavior and add a test coverage. Discussion: https://postgr.es/m/CAN4CZFNCU=t09M=+r2t9hHLJuujdM4oQ8hCK_Sx-GpfiwMAicw@mail.gmail.com --- doc/src/sgml/ref/alter_table.sgml | 33 ++++ src/backend/commands/tablecmds.c | 148 ++++++++++++++++++ src/test/regress/expected/partition_merge.out | 62 ++++++++ src/test/regress/expected/partition_split.out | 60 +++++++ src/test/regress/sql/partition_merge.sql | 51 ++++++ src/test/regress/sql/partition_split.sql | 48 ++++++ 6 files changed, 402 insertions(+) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index b8246a7ee48..04ab3d08bbd 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -1281,6 +1281,24 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM dependencies are not silently lost during merge. </para> + <para> + The new partition takes its replica identity from the merged partitions + when they all use the same simple setting + (<literal>DEFAULT</literal>, <literal>FULL</literal> or + <literal>NOTHING</literal>). If they use different settings, or use + <literal>REPLICA IDENTITY USING INDEX</literal>, the error is issued + and the command is aborted. Give the partitions being merged a uniform, + non-index replica identity before merging, and set a different replica + identity on the resulting partition afterwards if desired. Likewise, if + any of the partitions being merged is directly part of a publication, the + command is aborted; publish the partitioned table itself instead of the + individual partitions, or remove the partition from the publication before + merging. For the same reason, the new partition cannot be created in a + schema that is not covered by the same publications defined + <literal>FOR TABLES IN SCHEMA</literal> as the schema of the partitions + being merged. + </para> + <para> Moving rows into the new partition does not emit logical replication messages, in the same way that <command>CLUSTER</command> or @@ -1401,6 +1419,21 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the source partition's indexes. </para> + <para> + The new partitions take their replica identity from the split partition, + unless it uses <literal>REPLICA IDENTITY USING INDEX</literal>, in which + case the error is issued and the command is aborted. Give the partition + being split a non-index replica identity before splitting, and set a + different replica identity on the new partitions afterwards if desired. + Likewise, if the partition being split is directly part of a publication, + the command is rejected; publish the partitioned table itself instead of + the individual partitions, or remove the partition from the publication + before splitting. For the same reason, the new partitions cannot be + created in a schema that is not covered by the same publications defined + <literal>FOR TABLES IN SCHEMA</literal> as the schema of the partition + being split. + </para> + <para> Moving rows into the new partitions does not emit logical replication messages, in the same way that <command>CLUSTER</command> or diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0eb85c1be17..46396117006 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -49,6 +49,7 @@ #include "catalog/pg_opclass.h" #include "catalog/pg_policy.h" #include "catalog/pg_proc.h" +#include "catalog/pg_publication.h" #include "catalog/pg_publication_rel.h" #include "catalog/pg_rewrite.h" #include "catalog/pg_statistic_ext.h" @@ -23353,6 +23354,137 @@ createPartitionTable(List **wqueue, RangeVar *newPartName, return newRel; } +/* + * checkPartitionSchemaPublications: refuse MERGE/SPLIT when the new partition(s) + * would land in a schema whose FOR TABLES IN SCHEMA publications differ from + * those of the source partition(s). + * + * The new partitions are created under the name given in the command, which may + * name a different schema than the source partitions live in. A publication + * defined FOR TABLES IN SCHEMA covers exactly the tables of that schema, so such + * a move would silently add the relocated rows to, or remove them from, that + * publication. Publications FOR ALL TABLES, or covering the partitioned table + * itself, keep covering the new partitions and are therefore not a problem. + * + * 'sourceOids' lists the source partition OIDs, 'newPartRels' the new partition + * Relations. + */ +static void +checkPartitionSchemaPublications(List *sourceOids, List *newPartRels) +{ + foreach_oid(srcOid, sourceOids) + { + Oid srcNsp = get_rel_namespace(srcOid); + List *srcPubs = NIL; + bool srcPubsFetched = false; + + foreach_ptr(RelationData, newrel, newPartRels) + { + Oid newNsp = RelationGetNamespace(newrel); + List *newPubs; + + /* Same schema: publication membership cannot change. */ + if (newNsp == srcNsp) + continue; + + if (!srcPubsFetched) + { + srcPubs = GetSchemaPublications(srcNsp); + srcPubsFetched = true; + } + newPubs = GetSchemaPublications(newNsp); + + /* No schema publication involved, so nothing can change. */ + if (srcPubs == NIL && newPubs == NIL) + continue; + + if (list_length(srcPubs) != list_length(newPubs) || + list_difference_oid(srcPubs, newPubs) != NIL) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move partition \"%s\" to schema \"%s\" with different publications for tables in schema", + get_rel_name(srcOid), + get_namespace_name(newNsp)), + errdetail("Schema \"%s\" and schema \"%s\" are not covered by the same publications defined FOR TABLES IN SCHEMA, so the new partition would silently join or leave a publication.", + get_namespace_name(srcNsp), + get_namespace_name(newNsp)), + errhint("Create the new partition in the same schema, or publish the partitioned table itself.")); + } + } +} + +/* + * transferPartitionReplicaIdentity: propagate the source partitions' replica + * identity to the new partition(s) created by MERGE/SPLIT, and refuse the + * operation for cases we cannot handle without silently changing replication + * behavior. + * + * The new partitions are built from the partitioned-table template and would + * otherwise default to REPLICA IDENTITY DEFAULT and drop out of any publication + * that the source partitions were directly part of. To avoid silent surprises: + * + * - A uniform, simply-representable replica identity (DEFAULT, FULL or + * NOTHING) is carried over to every new partition. If the sources disagree, + * or use an index-based identity (which cannot be reproduced on the new + * partition automatically), we raise an error and ask the user to set it. + * + * - If any source partition is a direct member of a publication, we refuse the + * operation: the new partition would silently leave the publication, and + * faithfully reproducing per-relation column lists and row filters is + * ambiguous (especially when several sources are merged). Publications that + * cover the partitioned root instead continue to include the new partition. + * + * 'sourceOids' lists the source partition OIDs (still present, not yet dropped); + * 'newPartRels' lists the new partition Relations (exclusively locked). + */ +static void +transferPartitionReplicaIdentity(List *sourceOids, List *newPartRels) +{ + char ri_type = '\0'; + bool ri_seen = false; + + foreach_oid(srcOid, sourceOids) + { + Relation src = table_open(srcOid, NoLock); + + if (GetRelationIncludedPublications(srcOid) != NIL) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot merge or split partition \"%s\" that is directly part of a publication", + RelationGetRelationName(src)), + errhint("Publish the partitioned table instead, or add the new partition to the publication after the operation.")); + + if (!ri_seen) + { + ri_type = src->rd_rel->relreplident; + ri_seen = true; + } + else if (ri_type != src->rd_rel->relreplident) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("partitions being merged have different replica identity settings"), + errhint("Set the replica identity of the new partition explicitly after the operation.")); + + table_close(src, NoLock); + } + + /* Nothing to carry over, or the new partitions already match. */ + if (!ri_seen || ri_type == REPLICA_IDENTITY_DEFAULT) + return; + + if (ri_type == REPLICA_IDENTITY_INDEX) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot automatically transfer an index-based replica identity to the new partition"), + errhint("Set the replica identity of the new partition explicitly with ALTER TABLE ... REPLICA IDENTITY USING INDEX.")); + + /* Carry FULL / NOTHING over to each new partition. */ + foreach_ptr(RelationData, newrel, newPartRels) + relation_mark_replica_identity(newrel, ri_type, InvalidOid, true); + + CommandCounterIncrement(); +} + /* * MergePartitionsMoveRows: scan partitions to be merged (mergingPartitions) * of the partitioned table and move rows into the new partition @@ -23903,6 +24035,14 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, Assert(OidIsValid(ownerId)); newPartRel = createPartitionTable(wqueue, cmd->name, rel, ownerId); + /* + * Carry the source partitions' replica identity over to the new + * partition, and reject cases that would silently change replication + * behavior. + */ + transferPartitionReplicaIdentity(mergingPartitions, list_make1(newPartRel)); + checkPartitionSchemaPublications(mergingPartitions, list_make1(newPartRel)); + /* * Switch to the table owner's userid, so that any index functions are run * as that user. Also, lockdown security-restricted operations and @@ -24345,6 +24485,14 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, newPartRels = lappend(newPartRels, newPartRel); } + /* + * Carry the split partition's replica identity over to the new + * partitions, and reject cases that would silently change replication + * behavior. + */ + transferPartitionReplicaIdentity(list_make1_oid(splitRelOid), newPartRels); + checkPartitionSchemaPublications(list_make1_oid(splitRelOid), newPartRels); + /* * Switch to the table owner's userid, so that any index functions are run * as that user. Also, lockdown security-restricted operations and diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out index ccda2b5843b..c00cd5b5599 100644 --- a/src/test/regress/expected/partition_merge.out +++ b/src/test/regress/expected/partition_merge.out @@ -1167,6 +1167,68 @@ SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged'; 0 (1 row) +DROP TABLE t; +-- MERGE PARTITIONS carries over a uniform replica identity ... +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE tp_0_1 REPLICA IDENTITY FULL; +ALTER TABLE tp_1_2 REPLICA IDENTITY FULL; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; +SELECT relreplident FROM pg_class WHERE relname = 'tp_0_2' + AND relnamespace = 'partitions_merge_schema'::regnamespace; + relreplident +-------------- + f +(1 row) + +DROP TABLE t; +-- ... but rejects merging partitions with different replica identities. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE tp_0_1 REPLICA IDENTITY FULL; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +ERROR: partitions being merged have different replica identity settings +HINT: Set the replica identity of the new partition explicitly after the operation. +DROP TABLE t; +-- MERGE PARTITIONS rejects a partition that is directly part of a publication. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +CREATE PUBLICATION pub_merge FOR TABLE tp_0_1; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +ERROR: cannot merge or split partition "tp_0_1" that is directly part of a publication +HINT: Publish the partitioned table instead, or add the new partition to the publication after the operation. +DROP PUBLICATION pub_merge; +DROP TABLE t; +-- Creating the new partition in another schema is only rejected when that +-- actually changes which FOR TABLES IN SCHEMA publications cover it. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +INSERT INTO t VALUES (0), (1); +-- No such publication, so a cross-schema merge is fine. +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO partitions_merge_schema2.tp_0_2; +SELECT count(*) FROM t; + count +------- + 2 +(1 row) + +DROP TABLE t; +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +CREATE PUBLICATION pub_merge FOR TABLES IN SCHEMA partitions_merge_schema; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) + INTO partitions_merge_schema2.tp_0_2; -- fails +ERROR: cannot move partition "tp_0_1" to schema "partitions_merge_schema2" with different publications for tables in schema +DETAIL: Schema "partitions_merge_schema" and schema "partitions_merge_schema2" are not covered by the same publications defined FOR TABLES IN SCHEMA, so the new partition would silently join or leave a publication. +HINT: Create the new partition in the same schema, or publish the partitioned table itself. +-- Staying in the covered schema is fine. +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; +DROP PUBLICATION pub_merge; DROP TABLE t; RESET search_path; -- diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out index 8e245563801..3f7d49b2204 100644 --- a/src/test/regress/expected/partition_split.out +++ b/src/test/regress/expected/partition_split.out @@ -1751,6 +1751,66 @@ SELECT relname, reltablespace FROM pg_class tp_lo | 0 (2 rows) +DROP TABLE t; +-- SPLIT PARTITION carries the split partition's replica identity to the new +-- partitions. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +ALTER TABLE tp_0_2 REPLICA IDENTITY FULL; +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); +SELECT relname, relreplident FROM pg_class + WHERE relname IN ('tp_0_1', 'tp_1_2') + AND relnamespace = 'partition_split_schema'::regnamespace ORDER BY relname; + relname | relreplident +---------+-------------- + tp_0_1 | f + tp_1_2 | f +(2 rows) + +DROP TABLE t; +-- SPLIT PARTITION rejects a partition that is directly part of a publication. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +CREATE PUBLICATION pub_split FOR TABLE tp_0_2; +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +ERROR: cannot merge or split partition "tp_0_2" that is directly part of a publication +HINT: Publish the partitioned table instead, or add the new partition to the publication after the operation. +DROP PUBLICATION pub_split; +DROP TABLE t; +-- Creating the new partitions in another schema is only rejected when that +-- actually changes which FOR TABLES IN SCHEMA publications cover them. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +INSERT INTO t VALUES (0), (1); +-- No such publication, so a cross-schema split is fine. +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION partition_split_schema2.tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION partition_split_schema2.tp_1_2 FOR VALUES FROM (1) TO (2)); +SELECT count(*) FROM t; + count +------- + 2 +(1 row) + +DROP TABLE t; +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +CREATE PUBLICATION pub_split FOR TABLES IN SCHEMA partition_split_schema; +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION partition_split_schema2.tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION partition_split_schema2.tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +ERROR: cannot move partition "tp_0_2" to schema "partition_split_schema2" with different publications for tables in schema +DETAIL: Schema "partition_split_schema" and schema "partition_split_schema2" are not covered by the same publications defined FOR TABLES IN SCHEMA, so the new partition would silently join or leave a publication. +HINT: Create the new partition in the same schema, or publish the partitioned table itself. +-- Staying in the covered schema is fine. +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); +DROP PUBLICATION pub_split; DROP TABLE t; RESET search_path; -- diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql index 80dc365b0ce..63f1ccd0fba 100644 --- a/src/test/regress/sql/partition_merge.sql +++ b/src/test/regress/sql/partition_merge.sql @@ -839,6 +839,57 @@ SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged'; DROP TABLE t; +-- MERGE PARTITIONS carries over a uniform replica identity ... +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE tp_0_1 REPLICA IDENTITY FULL; +ALTER TABLE tp_1_2 REPLICA IDENTITY FULL; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; +SELECT relreplident FROM pg_class WHERE relname = 'tp_0_2' + AND relnamespace = 'partitions_merge_schema'::regnamespace; +DROP TABLE t; + +-- ... but rejects merging partitions with different replica identities. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE tp_0_1 REPLICA IDENTITY FULL; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +DROP TABLE t; + +-- MERGE PARTITIONS rejects a partition that is directly part of a publication. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +CREATE PUBLICATION pub_merge FOR TABLE tp_0_1; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +DROP PUBLICATION pub_merge; +DROP TABLE t; + +-- Creating the new partition in another schema is only rejected when that +-- actually changes which FOR TABLES IN SCHEMA publications cover it. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +INSERT INTO t VALUES (0), (1); +-- No such publication, so a cross-schema merge is fine. +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO partitions_merge_schema2.tp_0_2; +SELECT count(*) FROM t; +DROP TABLE t; + +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +CREATE PUBLICATION pub_merge FOR TABLES IN SCHEMA partitions_merge_schema; +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) + INTO partitions_merge_schema2.tp_0_2; -- fails +-- Staying in the covered schema is fine. +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; +DROP PUBLICATION pub_merge; +DROP TABLE t; + + RESET search_path; -- diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql index ffd15e7f969..c470c42be71 100644 --- a/src/test/regress/sql/partition_split.sql +++ b/src/test/regress/sql/partition_split.sql @@ -1256,6 +1256,54 @@ SELECT relname, reltablespace FROM pg_class WHERE relname IN ('tp_lo', 'tp_hi') ORDER BY relname; DROP TABLE t; +-- SPLIT PARTITION carries the split partition's replica identity to the new +-- partitions. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +ALTER TABLE tp_0_2 REPLICA IDENTITY FULL; +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); +SELECT relname, relreplident FROM pg_class + WHERE relname IN ('tp_0_1', 'tp_1_2') + AND relnamespace = 'partition_split_schema'::regnamespace ORDER BY relname; +DROP TABLE t; + +-- SPLIT PARTITION rejects a partition that is directly part of a publication. +CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +CREATE PUBLICATION pub_split FOR TABLE tp_0_2; +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +DROP PUBLICATION pub_split; +DROP TABLE t; + +-- Creating the new partitions in another schema is only rejected when that +-- actually changes which FOR TABLES IN SCHEMA publications cover them. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +INSERT INTO t VALUES (0), (1); +-- No such publication, so a cross-schema split is fine. +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION partition_split_schema2.tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION partition_split_schema2.tp_1_2 FOR VALUES FROM (1) TO (2)); +SELECT count(*) FROM t; +DROP TABLE t; + +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +CREATE PUBLICATION pub_split FOR TABLES IN SCHEMA partition_split_schema; +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION partition_split_schema2.tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION partition_split_schema2.tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +-- Staying in the covered schema is fine. +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); +DROP PUBLICATION pub_split; +DROP TABLE t; + RESET search_path; -- -- 2.55.0
From b5060c24460e18fcad44b642f1c52555321dcd77 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov <[email protected]> Date: Mon, 3 Aug 2026 00:19:19 +0200 Subject: [PATCH v3 3/3] Don't recalculate generated columns during MERGE/SPLIT PARTITION(S) ALTER TABLE ... MERGE/SPLIT PARTITION unconditionally recomputed every stored generated column of the moved rows using the partitioned table's generation expression. When a leaf partition's generation expression -- or a function it calls -- differed from the partitioned table's, this silently rewrote already-stored values, and could even break constraints. Relocating a row between partitions never changes a user column, so a stored generated column defined over user columns yields the same value; move it as-is instead of recomputing, as every other command preserves generated column values. This alone removes the silent data changes and constraint violations reported for such columns. Moving values as-is is only correct when the source partition's generation expression matches the partitioned table's. A partition can carry a different expression (ATTACH PARTITION requires the generated-column kind to match but does not compare the expressions), in which case the moved-as-is value would not match the new partition's generation expression -- silently storing data inconsistent with the schema, and possibly violating NOT NULL, CHECK, or foreign-key constraints. Reject MERGE/SPLIT in that case, in the new checkPartitionGenExprMatchesParent(). What does legitimately change on the move is tableoid, the only system column allowed in such expressions, so the new checkPartitionSystemColumnRefs() rejects every dependency on it: - A stored generated column would have to be recomputed, but unlike a normal insert the row-movement path does not re-verify NOT NULL, foreign-key, or generated-column-dependent CHECK constraints, so a recomputed value could silently violate them. A virtual generated column is not stored at all, so its value would silently change as soon as the rows live in the new partition, with the same consequences. - A CHECK constraint would have to be re-verified against the new partition's OID, and that cannot be done faithfully either: the row movement runs under RestrictSearchPath(), so a search_path dependent expression such as tableoid::regclass::text does not evaluate the way it would for a regular INSERT, which makes the re-verification both unreliable and confusing. As nothing is recomputed or re-verified anymore, the machinery that did so during the row move is gone: createTableConstraints() no longer records generated columns in AlteredTableInfo.newvals nor CHECK constraints in AlteredTableInfo.constraints, and the two row-move helpers that evaluated them are removed, along with the work queue entry and arguments that only existed to carry them. Document the behavior and add regression coverage for all three rejections. Existing MERGE/SPLIT tests that relied on recomputation now assert the rejection or use a generation expression matching the partitioned table, and a function-change test shows a plain generated column's value preserved. Discussion: https://postgr.es/m/CAN4CZFNCU=t09M=+r2t9hHLJuujdM4oQ8hCK_Sx-GpfiwMAicw@mail.gmail.com --- doc/src/sgml/ref/alter_table.sgml | 30 ++ src/backend/commands/tablecmds.c | 395 +++++++++--------- src/test/regress/expected/partition_merge.out | 127 ++++-- src/test/regress/expected/partition_split.out | 74 +++- src/test/regress/sql/partition_merge.sql | 88 +++- src/test/regress/sql/partition_split.sql | 55 ++- 6 files changed, 485 insertions(+), 284 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 04ab3d08bbd..7f1c6133337 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -1281,6 +1281,21 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM dependencies are not silently lost during merge. </para> + <para> + Stored generated columns keep their existing values; the merge does not + recompute them from the partitioned table's generation expression, which + is consistent with how other commands preserve generated column values. + Because the values are moved rather than recomputed, the merge is + rejected if a merged partition's generation expression differs from the + partitioned table's, which would otherwise leave the new partition's + stored data inconsistent with its own generation expression. + As a further exception, if a stored generated column's expression + references a system column such as <structfield>tableoid</structfield> + (whose value would change when a row is moved to another partition), the + command is rejected, because the row-movement path cannot safely recompute + the value while re-verifying all of the table's constraints against it. + </para> + <para> The new partition takes its replica identity from the merged partitions when they all use the same simple setting @@ -1419,6 +1434,21 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM from the source partition's indexes. </para> + <para> + Stored generated columns keep their existing values; the split does not + recompute them from the partitioned table's generation expression, which + is consistent with how other commands preserve generated column values. + Because the values are moved rather than recomputed, the split is + rejected if the split partition's generation expression differs from the + partitioned table's, which would otherwise leave the new partitions' + stored data inconsistent with their own generation expression. + As a further exception, if a stored generated column's expression + references a system column such as <structfield>tableoid</structfield> + (whose value would change when a row is moved to another partition), the + command is rejected, because the row-movement path cannot safely recompute + the value while re-verifying all of the table's constraints against it. + </para> + <para> The new partitions take their replica identity from the split partition, unless it uses <literal>REPLICA IDENTITY USING INDEX</literal>, in which diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 46396117006..7a76ebf3e3f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -22859,92 +22859,6 @@ GetAttributeStorage(Oid atttypid, const char *storagemode) return cstorage; } -/* - * buildExpressionExecutionStates: build the needed expression execution states - * for new partition (newPartRel) checks and initialize expressions for - * generated columns. All expressions should be created in "tab" - * (AlteredTableInfo structure). - */ -static void -buildExpressionExecutionStates(AlteredTableInfo *tab, Relation newPartRel, EState *estate) -{ - /* - * Build the needed expression execution states. Here, we expect only NOT - * NULL and CHECK constraint. - */ - foreach_ptr(NewConstraint, con, tab->constraints) - { - switch (con->contype) - { - case CONSTR_CHECK: - - /* - * We already expanded virtual expression in - * createTableConstraints. - */ - con->qualstate = ExecPrepareExpr((Expr *) con->qual, estate); - break; - case CONSTR_NOTNULL: - /* Nothing to do here. */ - break; - default: - elog(ERROR, "unrecognized constraint type: %d", - (int) con->contype); - } - } - - /* Expression already planned in createTableConstraints */ - foreach_ptr(NewColumnValue, ex, tab->newvals) - ex->exprstate = ExecInitExpr((Expr *) ex->expr, NULL); -} - -/* - * evaluateGeneratedExpressionsAndCheckConstraints: evaluate any generated - * expressions for "tab" (AlteredTableInfo structure) whose inputs come from - * the new tuple (insertslot) of the new partition (newPartRel). - */ -static void -evaluateGeneratedExpressionsAndCheckConstraints(AlteredTableInfo *tab, - Relation newPartRel, - TupleTableSlot *insertslot, - ExprContext *econtext) -{ - econtext->ecxt_scantuple = insertslot; - - foreach_ptr(NewColumnValue, ex, tab->newvals) - { - if (!ex->is_generated) - continue; - - insertslot->tts_values[ex->attnum - 1] - = ExecEvalExpr(ex->exprstate, - econtext, - &insertslot->tts_isnull[ex->attnum - 1]); - } - - foreach_ptr(NewConstraint, con, tab->constraints) - { - switch (con->contype) - { - case CONSTR_CHECK: - if (!ExecCheck(con->qualstate, econtext)) - ereport(ERROR, - errcode(ERRCODE_CHECK_VIOLATION), - errmsg("check constraint \"%s\" of relation \"%s\" is violated by some row", - con->name, RelationGetRelationName(newPartRel)), - errtableconstraint(newPartRel, con->name)); - break; - case CONSTR_NOTNULL: - case CONSTR_FOREIGN: - /* Nothing to do here */ - break; - default: - elog(ERROR, "unrecognized constraint type: %d", - (int) con->contype); - } - } -} - /* * getAttributesList: build a list of columns (ColumnDef) based on parent_rel */ @@ -22995,15 +22909,171 @@ getAttributesList(Relation parent_rel) return colList; } +/* + * expression_references_system_column: walker that returns true if the given + * expression references any system column (a Var with a negative attribute + * number, such as tableoid). Used to decide whether a stored generated column + * must be recomputed when a row is relocated between partitions. + */ +static bool +expression_references_system_column(Node *node, void *context) +{ + if (node == NULL) + return false; + if (IsA(node, Var) && ((Var *) node)->varattno < 0) + return true; + return expression_tree_walker(node, expression_references_system_column, + context); +} + +/* + * checkPartitionSystemColumnRefs: reject MERGE/SPLIT PARTITION when the + * partitioned table has a generated column or a CHECK constraint whose + * expression references a system column. + * + * Only tableoid may appear in such expressions, and it is precisely the value + * that changes when a row is relocated into the new partition. Neither + * dependency can be honored during the row movement: + * + * - A stored generated column would have to be recomputed, but the row-movement + * path does not re-verify NOT NULL, foreign-key, or generated-column-dependent + * CHECK constraints the way a normal insert does, so a recomputed value could + * silently violate them. A virtual generated column is not stored at all, so + * its value silently changes as soon as the rows live in the new partition. + * + * - A CHECK constraint would have to be re-verified against the new partition's + * OID. We cannot do that faithfully either: the row movement runs under + * RestrictSearchPath(), so a search_path-dependent expression such as + * tableoid::regclass::text does not evaluate the way it would for a regular + * INSERT, which would make the re-verification both unreliable and confusing. + * + * So reject these cases and let the user handle such columns and constraints + * explicitly. In the future we may implement recomputation together with a + * full re-validation of the affected constraints. + */ +static void +checkPartitionSystemColumnRefs(Relation parent_rel) +{ + TupleDesc tupleDesc = RelationGetDescr(parent_rel); + TupleConstr *constr = tupleDesc->constr; + + if (constr == NULL) + return; + + /* Generated columns, both stored and virtual. */ + if (constr->has_generated_stored || constr->has_generated_virtual) + { + for (AttrNumber attno = 1; attno <= tupleDesc->natts; attno++) + { + Form_pg_attribute attr = TupleDescAttr(tupleDesc, attno - 1); + + if (attr->attisdropped || attr->attgenerated == '\0') + continue; + + if (expression_references_system_column(build_generation_expression(parent_rel, attno), + NULL)) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot merge or split partitions when a generated column depends on a system column"), + errdetail("Column \"%s\" of relation \"%s\" is generated from an expression that references a system column such as tableoid.", + NameStr(attr->attname), + RelationGetRelationName(parent_rel))); + } + } + + /* CHECK constraints. */ + for (int ccnum = 0; ccnum < constr->num_check; ccnum++) + { + if (expression_references_system_column(stringToNode(constr->check[ccnum].ccbin), + NULL)) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot merge or split partitions when a check constraint depends on a system column"), + errdetail("Constraint \"%s\" of relation \"%s\" references a system column such as tableoid.", + constr->check[ccnum].ccname, + RelationGetRelationName(parent_rel))); + } +} + +/* + * checkPartitionGenExprMatchesParent: reject MERGE/SPLIT PARTITION when a + * source partition has a generated column whose generation expression differs + * from the partitioned table's. + * + * MERGE/SPLIT PARTITION relocates rows into the new partition and copies stored + * generated columns as-is rather than recomputing them (see + * createTableConstraints()). Since the new partition is created from the + * partitioned table as a template, moving values as-is is only correct when the + * source partition's generation expression matches the partitioned table's. + * Otherwise the moved value would not match the new partition's generation + * expression, silently storing data inconsistent with the schema and possibly + * violating NOT NULL, CHECK, or foreign-key constraints. + * + * A partition can end up with a generation expression different from the + * partitioned table's via ATTACH PARTITION, which requires the generated-column + * kind to match but does not compare the expressions themselves (see + * MergeAttributesIntoExisting()). + */ +static void +checkPartitionGenExprMatchesParent(Relation parent_rel, Relation partRel) +{ + TupleDesc parentDesc = RelationGetDescr(parent_rel); + TupleConstr *constr = parentDesc->constr; + AttrMap *attmap = NULL; + + /* Nothing to compare if the partitioned table has no generated columns. */ + if (constr == NULL || + !(constr->has_generated_stored || constr->has_generated_virtual)) + return; + + for (AttrNumber parent_attno = 1; parent_attno <= parentDesc->natts; + parent_attno++) + { + Form_pg_attribute pattr = TupleDescAttr(parentDesc, parent_attno - 1); + AttrNumber child_attno; + Node *parentExpr; + Node *childExpr; + bool found_whole_row; + + if (pattr->attisdropped || pattr->attgenerated == '\0') + continue; + + /* + * Column names match between a partitioned table and its partitions, + * and so does the generated-column kind; only the expression can + * differ (all enforced/allowed by MergeAttributesIntoExisting()). + */ + child_attno = get_attnum(RelationGetRelid(partRel), NameStr(pattr->attname)); + Assert(child_attno != InvalidAttrNumber); + + parentExpr = build_generation_expression(parent_rel, parent_attno); + childExpr = build_generation_expression(partRel, child_attno); + + /* Rewrite the partition's expression into the parent's numbering. */ + if (attmap == NULL) + attmap = build_attrmap_by_name(parentDesc, + RelationGetDescr(partRel), false); + childExpr = map_variable_attnos(childExpr, 1, 0, attmap, + InvalidOid, &found_whole_row); + + if (found_whole_row || !equal(parentExpr, childExpr)) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot merge or split partitions when a partition's generation expression differs from the partitioned table"), + errdetail("Generated column \"%s\" of partition \"%s\" has a generation expression different from table \"%s\".", + NameStr(pattr->attname), + RelationGetRelationName(partRel), + RelationGetRelationName(parent_rel))); + } +} + /* * createTableConstraints: - * create check constraints, default values, and generated values for newRel - * based on parent_rel. tab is pending-work queue for newRel, we may need it in - * MergePartitionsMoveRows. + * create check constraints and column defaults (including generation + * expressions) for newRel based on parent_rel. */ static void -createTableConstraints(List **wqueue, AlteredTableInfo *tab, - Relation parent_rel, Relation newRel) +createTableConstraints(Relation parent_rel, Relation newRel) { TupleDesc tupleDesc; TupleConstr *constr; @@ -23045,7 +23115,6 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, bool found_whole_row; AttrNumber num; Node *def; - NewColumnValue *newval; if (attribute->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL) this_default = build_generation_expression(parent_rel, attribute->attnum); @@ -23067,19 +23136,18 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, StoreAttrDefault(newRel, num, def, false); /* - * Stored generated column expressions in parent_rel might - * reference the tableoid. newRel, parent_rel tableoid clear is - * not the same. If so, these stored generated columns require - * recomputation for newRel within MergePartitionsMoveRows. + * Relocating a row between partitions never changes a user + * column, so a stored generated column defined over user columns + * keeps the same value; we move it as-is rather than recomputing + * it, which is what every other command does. (A source + * partition whose generation expression differs from the + * partitioned table's has already been rejected by + * checkPartitionGenExprMatchesParent(), and an expression + * depending on a system column by + * checkPartitionSystemColumnRefs(); moving as-is here also avoids + * silently rewriting stored data when a function the expression + * calls has since been redefined.) */ - if (attribute->attgenerated == ATTRIBUTE_GENERATED_STORED) - { - newval = palloc0_object(NewColumnValue); - newval->attnum = num; - newval->expr = expression_planner((Expr *) def); - newval->is_generated = (attribute->attgenerated != '\0'); - tab->newvals = lappend(tab->newvals, newval); - } } } @@ -23138,40 +23206,13 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, CommandCounterIncrement(); /* - * parent_rel check constraint expression may reference tableoid, so later - * in MergePartitionsMoveRows, we need to evaluate the check constraint - * again for the newRel. We can check whether the check constraint - * contains a tableoid reference via pull_varattnos. + * The relocated rows satisfy the new partition's CHECK constraints + * without any re-verification here: the constraints are copied from the + * partitioned table, which the source partitions already inherited, and + * the row movement changes no column value. Constraints depending on a + * system column, the one thing that does change, were rejected by + * checkPartitionSystemColumnRefs(). */ - foreach_ptr(CookedConstraint, ccon, cookedConstraints) - { - if (!ccon->skip_validation) - { - Node *qual; - Bitmapset *attnums = NULL; - - Assert(ccon->contype == CONSTR_CHECK); - qual = expand_generated_columns_in_expr(ccon->expr, newRel, 1); - pull_varattnos(qual, 1, &attnums); - - /* - * Add a check only if it contains a tableoid - * (TableOidAttributeNumber). - */ - if (bms_is_member(TableOidAttributeNumber - FirstLowInvalidHeapAttributeNumber, - attnums)) - { - NewConstraint *newcon; - - newcon = palloc0_object(NewConstraint); - newcon->name = ccon->name; - newcon->contype = CONSTR_CHECK; - newcon->qual = qual; - - tab->constraints = lappend(tab->constraints, newcon); - } - } - } /* Don't need the cookedConstraints anymore. */ list_free_deep(cookedConstraints); @@ -23209,7 +23250,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, * Returns the created relation (locked in AccessExclusiveLock mode). */ static Relation -createPartitionTable(List **wqueue, RangeVar *newPartName, +createPartitionTable(RangeVar *newPartName, Relation parent_rel, Oid ownerId) { Relation newRel; @@ -23220,7 +23261,6 @@ createPartitionTable(List **wqueue, RangeVar *newPartName, List *colList = NIL; Oid relamId; Oid namespaceId; - AlteredTableInfo *new_partrel_tab; Form_pg_class parent_relform = parent_rel->rd_rel; /* If the existing rel is temp, it must belong to this session. */ @@ -23339,11 +23379,8 @@ createPartitionTable(List **wqueue, RangeVar *newPartName, */ newRel = table_open(newRelId, NoLock); - /* Find or create a work queue entry for the newly created table. */ - new_partrel_tab = ATGetQueueEntry(wqueue, newRel); - /* Create constraints, default values, and generated values. */ - createTableConstraints(wqueue, new_partrel_tab, parent_rel, newRel); + createTableConstraints(parent_rel, newRel); /* * Need to call CommandCounterIncrement, so a fresh relcache entry has @@ -23511,14 +23548,8 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart BulkInsertState bistate; /* state of bulk inserts for partition */ TupleTableSlot *dstslot; - /* Find the work queue entry for the new partition table: newPartRel. */ - tab = ATGetQueueEntry(wqueue, newPartRel); - - /* Generate the constraint and default execution states. */ estate = CreateExecutorState(); - buildExpressionExecutionStates(tab, newPartRel, estate); - mycid = GetCurrentCommandId(true); /* Prepare a BulkInsertState for table_tuple_insert. */ @@ -23594,22 +23625,6 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart ExecStoreVirtualTuple(insertslot); } - /* - * Constraints and GENERATED expressions might reference the - * tableoid column, so fill tts_tableOid with the desired value. - * (We must do this each time, because it gets overwritten with - * newrel's OID during storing.) - */ - insertslot->tts_tableOid = RelationGetRelid(newPartRel); - - /* - * Now, evaluate any generated expressions whose inputs come from - * the new tuple. We assume these columns won't reference each - * other, so that there's no ordering dependency. - */ - evaluateGeneratedExpressionsAndCheckConstraints(tab, newPartRel, - insertslot, econtext); - /* Write the tuple out to the new relation. */ table_tuple_insert(newPartRel, insertslot, mycid, ti_options, bistate); @@ -23906,6 +23921,13 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, int save_sec_context; int save_nestlevel; + /* + * The rows are relocated as-is, but a generated column or CHECK + * constraint depending on a system column would change meaning in the new + * partition. + */ + checkPartitionSystemColumnRefs(rel); + /* * Check ownership of merged partitions - partitions with different owners * cannot be merged. Also, collect the OIDs of these partitions during the @@ -23934,6 +23956,14 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, else ownerId = mergingPartition->rd_rel->relowner; + /* + * The new partition inherits the partitioned table's generation + * expressions, but rows are moved as-is; reject a partition whose + * generation expression differs, which would otherwise silently store + * inconsistent data. + */ + checkPartitionGenExprMatchesParent(rel, mergingPartition); + /* Store the next merging partition into the list. */ mergingPartitions = lappend_oid(mergingPartitions, RelationGetRelid(mergingPartition)); @@ -24033,7 +24063,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, * model. */ Assert(OidIsValid(ownerId)); - newPartRel = createPartitionTable(wqueue, cmd->name, rel, ownerId); + newPartRel = createPartitionTable(cmd->name, rel, ownerId); /* * Carry the source partitions' replica identity over to the new @@ -24212,11 +24242,6 @@ SplitPartitionMoveRows(List **wqueue, Relation rel, Relation splitRel, pc = createSplitPartitionContext((Relation) lfirst(listptr2)); - /* Find the work queue entry for the new partition table: newPartRel. */ - pc->tab = ATGetQueueEntry(wqueue, pc->partRel); - - buildExpressionExecutionStates(pc->tab, pc->partRel, estate); - if (sps->bound->is_default) { /* @@ -24334,22 +24359,6 @@ SplitPartitionMoveRows(List **wqueue, Relation rel, Relation splitRel, ExecStoreVirtualTuple(insertslot); } - /* - * Constraints and GENERATED expressions might reference the tableoid - * column, so fill tts_tableOid with the desired value. (We must do - * this each time, because it gets overwritten with newrel's OID - * during storing.) - */ - insertslot->tts_tableOid = RelationGetRelid(pc->partRel); - - /* - * Now, evaluate any generated expressions whose inputs come from the - * new tuple. We assume these columns won't reference each other, so - * that there's no ordering dependency. - */ - evaluateGeneratedExpressionsAndCheckConstraints(pc->tab, pc->partRel, - insertslot, econtext); - /* Write the tuple out to the new relation. */ table_tuple_insert(pc->partRel, insertslot, mycid, ti_options, pc->bistate); @@ -24405,6 +24414,16 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, splitRelOid = RelationGetRelid(splitRel); + /* + * The new partitions inherit the partitioned table's generation + * expressions, but rows are moved as-is; reject a split partition whose + * generation expression differs, which would otherwise silently store + * inconsistent data. Likewise reject expressions depending on a system + * column, whose value changes in the new partitions. + */ + checkPartitionSystemColumnRefs(rel); + checkPartitionGenExprMatchesParent(rel, splitRel); + /* Check descriptions of new partitions. */ foreach_node(SinglePartitionSpec, sps, cmd->partlist) { @@ -24480,7 +24499,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, { Relation newPartRel; - newPartRel = createPartitionTable(wqueue, sps->name, rel, + newPartRel = createPartitionTable(sps->name, rel, splitRel->rd_rel->relowner); newPartRels = lappend(newPartRels, newPartRel); } diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out index c00cd5b5599..6a9b2f97b2c 100644 --- a/src/test/regress/expected/partition_merge.out +++ b/src/test/regress/expected/partition_merge.out @@ -887,14 +887,14 @@ CREATE TABLE tp_0_1 (i int NOT NULL, t text STORAGE MAIN DEFAULT 'default_tp_0_1', b bigint, - d date GENERATED ALWAYS as ('2022-02-02') STORED); + d date GENERATED ALWAYS as ('2022-01-01') STORED); ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (1); COMMENT ON COLUMN tp_0_1.i IS 'tp_0_1.i'; CREATE TABLE tp_1_2 (i int NOT NULL, t text STORAGE MAIN DEFAULT 'default_tp_1_2', b bigint, - d date GENERATED ALWAYS as ('2022-03-03') STORED); + d date GENERATED ALWAYS as ('2022-01-01') STORED); ALTER TABLE t ATTACH PARTITION tp_1_2 FOR VALUES FROM (1) TO (2); COMMENT ON COLUMN tp_1_2.i IS 'tp_1_2.i'; CREATE STATISTICS t_stat (DEPENDENCIES) on i, b from t; @@ -926,7 +926,7 @@ CREATE TRIGGER tp_1_2_before_insert_row_trigger BEFORE INSERT ON tp_1_2 FOR EACH i | integer | | not null | | plain | | | tp_0_1.i t | text | | | 'default_tp_0_1'::text | main | | | b | bigint | | not null | | plain | | | - d | date | | | generated always as ('02-02-2022'::date) stored | plain | | | + d | date | | | generated always as ('01-01-2022'::date) stored | plain | | | Partition of: t FOR VALUES FROM (0) TO (1) Partition constraint: ((abs(i) IS NOT NULL) AND (abs(i) >= 0) AND (abs(i) < 1)) Check constraints: @@ -1030,37 +1030,50 @@ ERROR: insert or update on table "t_fk" violates foreign key constraint "t_fk_i DETAIL: Key (i)=(2) is not present in table "t". DROP TABLE t_fk; DROP TABLE t; --- Test for recomputation of stored generated columns. +-- A generated column whose expression references a system column (tableoid) is +-- the one whose value legitimately changes when a row is relocated to another +-- partition, so MERGE PARTITIONS is rejected. This holds for a stored column, +-- whose value cannot be recomputed while re-verifying all constraints ... CREATE TABLE t (i int, tab_id int generated always as (tableoid) stored) PARTITION BY RANGE (i); CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); -ALTER TABLE t ADD CONSTRAINT cc CHECK(tableoid <> 123456789); INSERT INTO t VALUES (0), (1); --- Should be 0 because partition identifier for row with i=0 is different from --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); - count -------- - 0 -(1 row) - --- "tab_id" column (stored generated column) with "tableoid" attribute requires --- recomputation here. -ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; --- Should be 1 because partition identifier for row with i=0 is the same as --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); - count -------- - 1 -(1 row) - +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +ERROR: cannot merge or split partitions when a generated column depends on a system column +DETAIL: Column "tab_id" of relation "t" is generated from an expression that references a system column such as tableoid. +DROP TABLE t; +-- ... and for a virtual one, which is not stored at all, so its value would +-- silently change as soon as the rows live in the new partition (here that +-- would also break the NOT NULL constraint if the new partition happened to +-- get the OID mentioned in the expression). A generated column over user +-- columns only is fine: its value is preserved, as exercised above. +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF(tableoid, 18470)) NOT NULL) + PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +INSERT INTO t VALUES (0), (1); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +ERROR: cannot merge or split partitions when a generated column depends on a system column +DETAIL: Column "g" of relation "t" is generated from an expression that references a system column such as tableoid. +DROP TABLE t; +-- A CHECK constraint referencing a system column is rejected for the same +-- reason: it would have to be re-verified against the new partition's OID, and +-- the row movement runs with a restricted search_path, so a search_path +-- dependent expression would not even evaluate the way it does for an INSERT. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE t ADD CONSTRAINT cc CHECK (tableoid::regclass::text <> 'tp_0_2'); +INSERT INTO t VALUES (0), (1); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +ERROR: cannot merge or split partitions when a check constraint depends on a system column +DETAIL: Constraint "cc" of relation "t" references a system column such as tableoid. DROP TABLE t; -- Test for generated columns (different order of columns in partitioned table -- and partitions). -CREATE TABLE t (i int, g int GENERATED ALWAYS AS (i + tableoid::int)) PARTITION BY RANGE (i); -CREATE TABLE tp_1 (g int GENERATED ALWAYS AS (i + tableoid::int), i int); -CREATE TABLE tp_2 (g int GENERATED ALWAYS AS (i + tableoid::int), i int); +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (i * 2)) PARTITION BY RANGE (i); +CREATE TABLE tp_1 (g int GENERATED ALWAYS AS (i * 2), i int); +CREATE TABLE tp_2 (g int GENERATED ALWAYS AS (i * 2), i int); ALTER TABLE t ATTACH PARTITION tp_1 FOR VALUES FROM (-1) TO (10); ALTER TABLE t ATTACH PARTITION tp_2 FOR VALUES FROM (10) TO (20); ALTER TABLE t ADD CHECK (g > 0); @@ -1070,24 +1083,17 @@ ALTER TABLE t MERGE PARTITIONS (tp_1, tp_2) INTO tp_12; INSERT INTO t VALUES (16); -- ERROR INSERT INTO t VALUES (0); -ERROR: new row for relation "tp_12" violates check constraint "t_i_check" +ERROR: new row for relation "tp_12" violates check constraint "t_g_check" DETAIL: Failing row contains (0, virtual). -- Should be 3 rows: (5), (15), (16): -SELECT i FROM t ORDER BY i; - i ----- - 5 - 15 - 16 +SELECT i, g FROM t ORDER BY i; + i | g +----+---- + 5 | 10 + 15 | 30 + 16 | 32 (3 rows) --- Should be 1 because for the same tableoid (15 + tableoid) = (5 + tableoid) + 10: -SELECT count(*) FROM t WHERE i = 15 AND g IN (SELECT g + 10 FROM t WHERE i = 5); - count -------- - 1 -(1 row) - DROP TABLE t; -- A merged partition needs its own TOAST table; otherwise an out-of-line -- varlena value carried over from one of the merging partitions has @@ -1167,6 +1173,45 @@ SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged'; 0 (1 row) +DROP TABLE t; +-- MERGE PARTITIONS preserves stored generated column values rather than +-- recomputing them (here the partitioned table's generation expression differs +-- from what actually produced the stored rows because the function changed). +CREATE FUNCTION merge_gen(i int) RETURNS int IMMUTABLE LANGUAGE sql AS 'SELECT i * 2'; +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (merge_gen(i)) STORED) + PARTITION BY RANGE (i); +CREATE TABLE tp_0_10 PARTITION OF t FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_10_20 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (3), (12); +CREATE OR REPLACE FUNCTION merge_gen(i int) RETURNS int IMMUTABLE LANGUAGE sql AS 'SELECT i * 100'; +ALTER TABLE t MERGE PARTITIONS (tp_0_10, tp_10_20) INTO tp_0_20; +-- Existing rows keep g = i * 2 (3->6, 12->24); only a fresh insert uses the new +-- expression (5->500). +INSERT INTO t VALUES (5); +SELECT i, g FROM t ORDER BY i; + i | g +----+----- + 3 | 6 + 5 | 500 + 12 | 24 +(3 rows) + +DROP TABLE t; +DROP FUNCTION merge_gen(int); +-- A partition can carry a generation expression different from the partitioned +-- table's (ATTACH PARTITION does not compare the expressions). Since values are +-- moved as-is, MERGE PARTITIONS is rejected in that case: otherwise the new +-- partition would store data inconsistent with its own generation expression +-- (and here even violate NOT NULL). +CREATE TABLE t (id int, g int GENERATED ALWAYS AS (NULLIF(id, 1)) STORED NOT NULL) + PARTITION BY RANGE (id); +CREATE TABLE tp_0_10 (g int GENERATED ALWAYS AS (id) STORED NOT NULL, id int); +ALTER TABLE t ATTACH PARTITION tp_0_10 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_10_20 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (2), (12); +ALTER TABLE t MERGE PARTITIONS (tp_0_10, tp_10_20) INTO tp_0_20; -- fails +ERROR: cannot merge or split partitions when a partition's generation expression differs from the partitioned table +DETAIL: Generated column "g" of partition "tp_0_10" has a generation expression different from table "t". DROP TABLE t; -- MERGE PARTITIONS carries over a uniform replica identity ... CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out index 3f7d49b2204..e5f07dacb25 100644 --- a/src/test/regress/expected/partition_split.out +++ b/src/test/regress/expected/partition_split.out @@ -1547,7 +1547,7 @@ CREATE TABLE tp_x (i int NOT NULL, t text STORAGE MAIN DEFAULT 'default_tp_x', b bigint, - d date GENERATED ALWAYS as ('2022-02-02') STORED); + d date GENERATED ALWAYS as ('2022-01-01') STORED); ALTER TABLE t ATTACH PARTITION tp_x FOR VALUES FROM (0) TO (2); COMMENT ON COLUMN tp_x.i IS 'tp_x.i'; CREATE STATISTICS t_stat (DEPENDENCIES) on i, b from t; @@ -1576,7 +1576,7 @@ CREATE TRIGGER tp_x_before_insert_row_trigger BEFORE INSERT ON tp_x FOR EACH ROW i | integer | | not null | | plain | | tp_x.i t | text | | | 'default_tp_x'::text | main | | b | bigint | | not null | | plain | | - d | date | | | generated always as ('02-02-2022'::date) stored | plain | | + d | date | | | generated always as ('01-01-2022'::date) stored | plain | | Partition of: t FOR VALUES FROM (0) TO (2) Partition constraint: ((abs(i) IS NOT NULL) AND (abs(i) >= 0) AND (abs(i) < 2)) Check constraints: @@ -1627,32 +1627,60 @@ SELECT tableoid::regclass, * FROM t ORDER BY tableoid::regclass::text COLLATE "C DROP TABLE t; DROP FUNCTION trigger_function(); --- Test for recomputation of stored generated columns. +-- A generated column whose expression references a system column (tableoid) is +-- the one whose value legitimately changes when a row is relocated to another +-- partition, so SPLIT PARTITION is rejected -- for a stored column, whose value +-- cannot be recomputed while re-verifying all constraints ... CREATE TABLE t (i int, tab_id int generated always as (tableoid) stored) PARTITION BY RANGE (i); CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); -ALTER TABLE t ADD CONSTRAINT cc CHECK(tableoid <> 123456789); INSERT INTO t VALUES (0), (1); --- Should be 1 because partition identifier for row with i=0 is the same as --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); - count -------- - 1 -(1 row) - --- "tab_id" column (stored generated column) with "tableoid" attribute requires --- recomputation here. ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), - PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); --- Should be 0 because partition identifier for row with i=0 is different from --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); - count -------- - 0 -(1 row) - + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +ERROR: cannot merge or split partitions when a generated column depends on a system column +DETAIL: Column "tab_id" of relation "t" is generated from an expression that references a system column such as tableoid. +DROP TABLE t; +-- ... and for a virtual one, which is not stored at all, so its value would +-- silently change as soon as the rows live in the new partitions. +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF(tableoid, 18470)) NOT NULL) + PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +INSERT INTO t VALUES (0), (1); +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +ERROR: cannot merge or split partitions when a generated column depends on a system column +DETAIL: Column "g" of relation "t" is generated from an expression that references a system column such as tableoid. +DROP TABLE t; +-- A CHECK constraint referencing a system column is rejected for the same +-- reason: it would have to be re-verified against the new partitions' OIDs, and +-- the row movement runs with a restricted search_path, so a search_path +-- dependent expression would not even evaluate the way it does for an INSERT. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +ALTER TABLE t ADD CONSTRAINT cc CHECK (tableoid::regclass::text <> 'tp_0_1'); +INSERT INTO t VALUES (0), (1); +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +ERROR: cannot merge or split partitions when a check constraint depends on a system column +DETAIL: Constraint "cc" of relation "t" references a system column such as tableoid. +DROP TABLE t; +-- A partition can carry a generation expression different from the partitioned +-- table's (ATTACH PARTITION does not compare the expressions). Since values are +-- moved as-is, SPLIT PARTITION is rejected in that case: otherwise the new +-- partitions would store data inconsistent with their own generation +-- expression. +CREATE TABLE t (id int, g int GENERATED ALWAYS AS (id * 2) STORED) + PARTITION BY RANGE (id); +CREATE TABLE tp_0_20 (id int, g int GENERATED ALWAYS AS (id * 100) STORED); +ALTER TABLE t ATTACH PARTITION tp_0_20 FOR VALUES FROM (0) TO (20); +INSERT INTO t VALUES (3), (12); +ALTER TABLE t SPLIT PARTITION tp_0_20 INTO + (PARTITION tp_0_10 FOR VALUES FROM (0) TO (10), + PARTITION tp_10_20 FOR VALUES FROM (10) TO (20)); -- fails +ERROR: cannot merge or split partitions when a partition's generation expression differs from the partitioned table +DETAIL: Generated column "g" of partition "tp_0_20" has a generation expression different from table "t". DROP TABLE t; -- Each new partition produced by SPLIT must get its own TOAST table so -- that out-of-line varlena attributes coming from the source partition diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql index 63f1ccd0fba..09c05d8d08f 100644 --- a/src/test/regress/sql/partition_merge.sql +++ b/src/test/regress/sql/partition_merge.sql @@ -649,7 +649,7 @@ CREATE TABLE tp_0_1 (i int NOT NULL, t text STORAGE MAIN DEFAULT 'default_tp_0_1', b bigint, - d date GENERATED ALWAYS as ('2022-02-02') STORED); + d date GENERATED ALWAYS as ('2022-01-01') STORED); ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (1); COMMENT ON COLUMN tp_0_1.i IS 'tp_0_1.i'; @@ -657,7 +657,7 @@ CREATE TABLE tp_1_2 (i int NOT NULL, t text STORAGE MAIN DEFAULT 'default_tp_1_2', b bigint, - d date GENERATED ALWAYS as ('2022-03-03') STORED); + d date GENERATED ALWAYS as ('2022-01-01') STORED); ALTER TABLE t ATTACH PARTITION tp_1_2 FOR VALUES FROM (1) TO (2); COMMENT ON COLUMN tp_1_2.i IS 'tp_1_2.i'; @@ -736,33 +736,49 @@ DROP TABLE t_fk; DROP TABLE t; --- Test for recomputation of stored generated columns. +-- A generated column whose expression references a system column (tableoid) is +-- the one whose value legitimately changes when a row is relocated to another +-- partition, so MERGE PARTITIONS is rejected. This holds for a stored column, +-- whose value cannot be recomputed while re-verifying all constraints ... CREATE TABLE t (i int, tab_id int generated always as (tableoid) stored) PARTITION BY RANGE (i); CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); -ALTER TABLE t ADD CONSTRAINT cc CHECK(tableoid <> 123456789); INSERT INTO t VALUES (0), (1); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +DROP TABLE t; --- Should be 0 because partition identifier for row with i=0 is different from --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); - --- "tab_id" column (stored generated column) with "tableoid" attribute requires --- recomputation here. -ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; +-- ... and for a virtual one, which is not stored at all, so its value would +-- silently change as soon as the rows live in the new partition (here that +-- would also break the NOT NULL constraint if the new partition happened to +-- get the OID mentioned in the expression). A generated column over user +-- columns only is fine: its value is preserved, as exercised above. +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF(tableoid, 18470)) NOT NULL) + PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +INSERT INTO t VALUES (0), (1); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails +DROP TABLE t; --- Should be 1 because partition identifier for row with i=0 is the same as --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); +-- A CHECK constraint referencing a system column is rejected for the same +-- reason: it would have to be re-verified against the new partition's OID, and +-- the row movement runs with a restricted search_path, so a search_path +-- dependent expression would not even evaluate the way it does for an INSERT. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE t ADD CONSTRAINT cc CHECK (tableoid::regclass::text <> 'tp_0_2'); +INSERT INTO t VALUES (0), (1); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails DROP TABLE t; -- Test for generated columns (different order of columns in partitioned table -- and partitions). -CREATE TABLE t (i int, g int GENERATED ALWAYS AS (i + tableoid::int)) PARTITION BY RANGE (i); -CREATE TABLE tp_1 (g int GENERATED ALWAYS AS (i + tableoid::int), i int); -CREATE TABLE tp_2 (g int GENERATED ALWAYS AS (i + tableoid::int), i int); +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (i * 2)) PARTITION BY RANGE (i); +CREATE TABLE tp_1 (g int GENERATED ALWAYS AS (i * 2), i int); +CREATE TABLE tp_2 (g int GENERATED ALWAYS AS (i * 2), i int); ALTER TABLE t ATTACH PARTITION tp_1 FOR VALUES FROM (-1) TO (10); ALTER TABLE t ATTACH PARTITION tp_2 FOR VALUES FROM (10) TO (20); ALTER TABLE t ADD CHECK (g > 0); @@ -775,9 +791,7 @@ INSERT INTO t VALUES (16); -- ERROR INSERT INTO t VALUES (0); -- Should be 3 rows: (5), (15), (16): -SELECT i FROM t ORDER BY i; --- Should be 1 because for the same tableoid (15 + tableoid) = (5 + tableoid) + 10: -SELECT count(*) FROM t WHERE i = 15 AND g IN (SELECT g + 10 FROM t WHERE i = 5); +SELECT i, g FROM t ORDER BY i; DROP TABLE t; @@ -839,6 +853,40 @@ SELECT reltablespace FROM pg_class WHERE relname = 'tp_merged'; DROP TABLE t; +-- MERGE PARTITIONS preserves stored generated column values rather than +-- recomputing them (here the partitioned table's generation expression differs +-- from what actually produced the stored rows because the function changed). +CREATE FUNCTION merge_gen(i int) RETURNS int IMMUTABLE LANGUAGE sql AS 'SELECT i * 2'; +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (merge_gen(i)) STORED) + PARTITION BY RANGE (i); +CREATE TABLE tp_0_10 PARTITION OF t FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_10_20 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (3), (12); +CREATE OR REPLACE FUNCTION merge_gen(i int) RETURNS int IMMUTABLE LANGUAGE sql AS 'SELECT i * 100'; +ALTER TABLE t MERGE PARTITIONS (tp_0_10, tp_10_20) INTO tp_0_20; +-- Existing rows keep g = i * 2 (3->6, 12->24); only a fresh insert uses the new +-- expression (5->500). +INSERT INTO t VALUES (5); +SELECT i, g FROM t ORDER BY i; +DROP TABLE t; +DROP FUNCTION merge_gen(int); + + +-- A partition can carry a generation expression different from the partitioned +-- table's (ATTACH PARTITION does not compare the expressions). Since values are +-- moved as-is, MERGE PARTITIONS is rejected in that case: otherwise the new +-- partition would store data inconsistent with its own generation expression +-- (and here even violate NOT NULL). +CREATE TABLE t (id int, g int GENERATED ALWAYS AS (NULLIF(id, 1)) STORED NOT NULL) + PARTITION BY RANGE (id); +CREATE TABLE tp_0_10 (g int GENERATED ALWAYS AS (id) STORED NOT NULL, id int); +ALTER TABLE t ATTACH PARTITION tp_0_10 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_10_20 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (2), (12); +ALTER TABLE t MERGE PARTITIONS (tp_0_10, tp_10_20) INTO tp_0_20; -- fails +DROP TABLE t; + + -- MERGE PARTITIONS carries over a uniform replica identity ... CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i); CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1); diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql index c470c42be71..db383c1ff30 100644 --- a/src/test/regress/sql/partition_split.sql +++ b/src/test/regress/sql/partition_split.sql @@ -1122,7 +1122,7 @@ CREATE TABLE tp_x (i int NOT NULL, t text STORAGE MAIN DEFAULT 'default_tp_x', b bigint, - d date GENERATED ALWAYS as ('2022-02-02') STORED); + d date GENERATED ALWAYS as ('2022-01-01') STORED); ALTER TABLE t ATTACH PARTITION tp_x FOR VALUES FROM (0) TO (2); COMMENT ON COLUMN tp_x.i IS 'tp_x.i'; @@ -1162,26 +1162,57 @@ DROP TABLE t; DROP FUNCTION trigger_function(); --- Test for recomputation of stored generated columns. +-- A generated column whose expression references a system column (tableoid) is +-- the one whose value legitimately changes when a row is relocated to another +-- partition, so SPLIT PARTITION is rejected -- for a stored column, whose value +-- cannot be recomputed while re-verifying all constraints ... CREATE TABLE t (i int, tab_id int generated always as (tableoid) stored) PARTITION BY RANGE (i); CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); -ALTER TABLE t ADD CONSTRAINT cc CHECK(tableoid <> 123456789); INSERT INTO t VALUES (0), (1); +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +DROP TABLE t; + +-- ... and for a virtual one, which is not stored at all, so its value would +-- silently change as soon as the rows live in the new partitions. +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF(tableoid, 18470)) NOT NULL) + PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +INSERT INTO t VALUES (0), (1); +ALTER TABLE t SPLIT PARTITION tp_0_2 INTO + (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +DROP TABLE t; --- Should be 1 because partition identifier for row with i=0 is the same as --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); --- "tab_id" column (stored generated column) with "tableoid" attribute requires --- recomputation here. +-- A CHECK constraint referencing a system column is rejected for the same +-- reason: it would have to be re-verified against the new partitions' OIDs, and +-- the row movement runs with a restricted search_path, so a search_path +-- dependent expression would not even evaluate the way it does for an INSERT. +CREATE TABLE t (i int) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); +ALTER TABLE t ADD CONSTRAINT cc CHECK (tableoid::regclass::text <> 'tp_0_1'); +INSERT INTO t VALUES (0), (1); ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), - PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); + PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails +DROP TABLE t; --- Should be 0 because partition identifier for row with i=0 is different from --- partition identifier for row with i=1. -SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1); +-- A partition can carry a generation expression different from the partitioned +-- table's (ATTACH PARTITION does not compare the expressions). Since values are +-- moved as-is, SPLIT PARTITION is rejected in that case: otherwise the new +-- partitions would store data inconsistent with their own generation +-- expression. +CREATE TABLE t (id int, g int GENERATED ALWAYS AS (id * 2) STORED) + PARTITION BY RANGE (id); +CREATE TABLE tp_0_20 (id int, g int GENERATED ALWAYS AS (id * 100) STORED); +ALTER TABLE t ATTACH PARTITION tp_0_20 FOR VALUES FROM (0) TO (20); +INSERT INTO t VALUES (3), (12); +ALTER TABLE t SPLIT PARTITION tp_0_20 INTO + (PARTITION tp_0_10 FOR VALUES FROM (0) TO (10), + PARTITION tp_10_20 FOR VALUES FROM (10) TO (20)); -- fails DROP TABLE t; -- Each new partition produced by SPLIT must get its own TOAST table so -- 2.55.0
