From f6c556aa4bf9aaf14fcdf4de3ad8b8cc0dbc6ff5 Mon Sep 17 00:00:00 2001 From: jian he Date: Thu, 6 Aug 2026 11:36:58 +0800 Subject: [PATCH v2 1/1] misc fix for Don-t-recalculate-generated-columns-during-MERGE-SPLIT --- src/backend/commands/tablecmds.c | 73 ++++++++----------- src/test/regress/expected/partition_merge.out | 41 +++-------- src/test/regress/expected/partition_split.out | 11 +++ src/test/regress/sql/partition_merge.sql | 29 ++------ src/test/regress/sql/partition_split.sql | 9 +++ 5 files changed, 67 insertions(+), 96 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 7c6669a68dd..c2a35a2dd63 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -795,6 +795,7 @@ static void ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, static List *collectPartitionIndexExtDeps(List *partitionOids); static void applyPartitionIndexExtDeps(Oid newPartOid, List *extDepState); static void freePartitionIndexExtDeps(List *extDepState); +static bool expression_references_system_column(Node *node, void *context); /* ---------------------------------------------------------------- * DefineRelation @@ -23030,8 +23031,8 @@ checkPartitionGenExprMatchesParent(Relation parent_rel, Relation partRel) /* * 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()). + * 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); @@ -23054,6 +23055,24 @@ checkPartitionGenExprMatchesParent(Relation parent_rel, Relation partRel) NameStr(pattr->attname), RelationGetRelationName(partRel), RelationGetRelationName(parent_rel))); + + /* + * A generated column whose expression references a system column (in + * practice only tableoid is allowed there) is the one case whose + * value would legitimately change on the move. We can't recompute it + * safely: 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. Rather than risk that, reject the operation and let + * the user handle such columns explicitly. + */ + if (expression_references_system_column(parentExpr, NULL)) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot merge or split partitions when a stored 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(pattr->attname), + RelationGetRelationName(parent_rel))); } } @@ -23127,34 +23146,6 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, /* Add a pre-cooked default expression. */ StoreAttrDefault(newRel, num, def, false); - - /* - * 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(); - * moving as-is here also avoids silently rewriting stored data when - * a function the expression calls has since been redefined.) - * - * A stored generated column whose expression references a system - * column (in practice only tableoid is allowed there) is the one - * case whose value would legitimately change on the move. We can't - * recompute it safely: 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. Rather than risk that, reject the - * operation and let the user handle such columns explicitly. - */ - if (attribute->attgenerated == ATTRIBUTE_GENERATED_STORED && - expression_references_system_column(def, NULL)) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot merge or split partitions when a stored 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(attribute->attname), - RelationGetRelationName(parent_rel))); } } @@ -23215,26 +23206,22 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, /* * 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. + * again for the newRel. */ 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)) + if (expression_references_system_column(qual, NULL)) { NewConstraint *newcon; @@ -23624,7 +23611,7 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart * other, so that there's no ordering dependency. */ checkPartitionRowConstraints(tab, newPartRel, - insertslot, econtext); + insertslot, econtext); /* Write the tuple out to the new relation. */ table_tuple_insert(newPartRel, insertslot, mycid, @@ -24060,8 +24047,9 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, 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. + * 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)); @@ -24370,7 +24358,7 @@ SplitPartitionMoveRows(List **wqueue, Relation rel, Relation splitRel, * that there's no ordering dependency. */ checkPartitionRowConstraints(pc->tab, pc->partRel, - insertslot, econtext); + insertslot, econtext); /* Write the tuple out to the new relation. */ table_tuple_insert(pc->partRel, insertslot, mycid, @@ -24516,8 +24504,9 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, } /* - * Carry the split partition's replica identity over to the new partitions, - * and reject cases that would silently change replication behavior. + * 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); diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out index 14cfe8aebeb..98b90bf4887 100644 --- a/src/test/regress/expected/partition_merge.out +++ b/src/test/regress/expected/partition_merge.out @@ -1044,38 +1044,15 @@ ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- fails ERROR: cannot merge or split partitions when a stored 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; --- 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); -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); -ALTER TABLE t ADD CHECK (i > 0); -INSERT INTO t VALUES (5), (15); -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" -DETAIL: Failing row contains (0, virtual). --- Should be 3 rows: (5), (15), (16): -SELECT i FROM t ORDER BY i; - i ----- - 5 - 15 - 16 -(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) - +-- Virtual generated columns whose expressions reference system columns (e.g., +-- tableoid) are not supported because MERGE PARTITIONS currently do not +-- reverify new partitions's NOT NULL constraints +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF (tableoid, -1)) NOT NULL) PARTITION BY RANGE (i); +CREATE TABLE tp_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE t MERGE PARTITIONS (tp_1, tp_2) INTO tp_12; -- error +ERROR: cannot merge or split partitions when a stored 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 merged partition needs its own TOAST table; otherwise an out-of-line -- varlena value carried over from one of the merging partitions has diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out index f484045983a..f2bd4b303d5 100644 --- a/src/test/regress/expected/partition_split.out +++ b/src/test/regress/expected/partition_split.out @@ -1640,6 +1640,17 @@ ALTER TABLE t SPLIT PARTITION tp_0_2 INTO ERROR: cannot merge or split partitions when a stored 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; +-- Virtual generated columns whose expressions reference system columns (e.g., +-- tableoid) are not supported because SPLIT PARTITIONS currently do not +-- reverify new partitions's NOT NULL constraints +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF (tableoid, -1)) NOT NULL) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (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 partitions when a stored 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 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 diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql index ab45df61bb5..42add853901 100644 --- a/src/test/regress/sql/partition_merge.sql +++ b/src/test/regress/sql/partition_merge.sql @@ -749,28 +749,13 @@ 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); -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); -ALTER TABLE t ADD CHECK (i > 0); -INSERT INTO t VALUES (5), (15); - -ALTER TABLE t MERGE PARTITIONS (tp_1, tp_2) INTO tp_12; - -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); - +-- Virtual generated columns whose expressions reference system columns (e.g., +-- tableoid) are not supported because MERGE PARTITIONS currently do not +-- reverify new partitions's NOT NULL constraints +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF (tableoid, -1)) NOT NULL) PARTITION BY RANGE (i); +CREATE TABLE tp_1 PARTITION OF t FOR VALUES FROM (0) TO (1); +CREATE TABLE tp_2 PARTITION OF t FOR VALUES FROM (1) TO (2); +ALTER TABLE t MERGE PARTITIONS (tp_1, tp_2) INTO tp_12; -- error DROP TABLE t; -- A merged partition needs its own TOAST table; otherwise an out-of-line diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql index e100c8fd27c..188550be272 100644 --- a/src/test/regress/sql/partition_split.sql +++ b/src/test/regress/sql/partition_split.sql @@ -1174,6 +1174,15 @@ ALTER TABLE t SPLIT PARTITION tp_0_2 INTO PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); -- fails DROP TABLE t; +-- Virtual generated columns whose expressions reference system columns (e.g., +-- tableoid) are not supported because SPLIT PARTITIONS currently do not +-- reverify new partitions's NOT NULL constraints +CREATE TABLE t (i int, g int GENERATED ALWAYS AS (NULLIF (tableoid, -1)) NOT NULL) PARTITION BY RANGE (i); +CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (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 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 -- 2.34.1