From 6b89e131f59a44021a24717fc893b33ac4b65936 Mon Sep 17 00:00:00 2001 From: jian he Date: Sat, 1 Aug 2026 17:46:08 +0800 Subject: [PATCH v1 1/1] reverify constraint for ALTER TABLE MERGE PARTITION Previously, we assumed that ALTER TABLE ... MERGE PARTITION simply combined the contents of multiple partitions into a new partition. However, the generation expressions of the source partitions may differ from that of the partitioned table. If the partitioned table contains generated columns, the data in the newly created partition may not be identical to the combined contents of the merged partitions. Therefore, when the partitioned table contains generated columns, we need to reverify constraints after the merge, including NOT NULL constraints, CHECK constraints, and foreign key constraints. --- src/backend/commands/tablecmds.c | 203 ++++++++++++++++-- src/test/regress/expected/partition_merge.out | 49 +++++ src/test/regress/sql/partition_merge.sql | 48 +++++ 3 files changed, 279 insertions(+), 21 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6d4c457b820..1a54ffe47ad 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -23011,6 +23011,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, int ccnum; List *constraints = NIL; List *cookedConstraints = NIL; + bool newRelhasGenerated; tupleDesc = RelationGetDescr(parent_rel); constr = tupleDesc->constr; @@ -23037,6 +23038,9 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, if (attribute->attisdropped) continue; + if (attribute->attnotnull) + tab->verify_new_notnull = true; + /* Copy the default, if present, and it should be copied. */ if (attribute->atthasdef) { @@ -23079,6 +23083,9 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, newval->is_generated = (attribute->attgenerated != '\0'); tab->newvals = lappend(tab->newvals, newval); } + + if (!newRelhasGenerated && attribute->attgenerated != '\0') + newRelhasGenerated = true; } } @@ -23154,11 +23161,18 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab, pull_varattnos(qual, 1, &attnums); /* - * Add a check only if it contains a tableoid + * Add a check if it contains a tableoid * (TableOidAttributeNumber). + * + * ALTER TABLE MERGE PARTITIONS may change the data of the new + * partition compared to the combination of the old, merged + * partitions, because the generated column expression on the new + * partition may differ from the one on the merged partitions. + * Therefore, CHECK constraints on the new merged need reverify + * again whenever the new table has generated columns. */ - if (bms_is_member(TableOidAttributeNumber - FirstLowInvalidHeapAttributeNumber, - attnums)) + if (bms_is_member(TableOidAttributeNumber - FirstLowInvalidHeapAttributeNumber, attnums) || + newRelhasGenerated) { NewConstraint *newcon; @@ -23364,12 +23378,15 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart CommandId mycid; EState *estate; AlteredTableInfo *tab; - ListCell *ltab; /* The FSM is empty, so don't bother using it. */ uint32 ti_options = TABLE_INSERT_SKIP_FSM; BulkInsertState bistate; /* state of bulk inserts for partition */ TupleTableSlot *dstslot; + ResultRelInfo *rInfo = NULL; + List *notnull_attrs; + List *notnull_virtual_attrs; + TupleDesc newTupDesc; /* Find the work queue entry for the new partition table: newPartRel. */ tab = ATGetQueueEntry(wqueue, newPartRel); @@ -23387,6 +23404,61 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart /* Create the necessary tuple slot. */ dstslot = table_slot_create(newPartRel, NULL); + newTupDesc = RelationGetDescr(newPartRel); + notnull_attrs = notnull_virtual_attrs = NIL; + + if (tab->verify_new_notnull) + { + /* + * If we are rebuilding the tuples OR if we added any new but not + * verified not-null constraints, check all *valid* not-null + * constraints. This is a bit of overkill but it minimizes risk of + * bugs. + * + * notnull_attrs does *not* collect attribute numbers for valid + * not-null constraints over virtual generated columns; instead, they + * are collected in notnull_virtual_attrs for verification elsewhere. + */ + for (int i = 0; i < newTupDesc->natts; i++) + { + CompactAttribute *attr = TupleDescCompactAttr(newTupDesc, i); + + if (attr->attnullability == ATTNULLABLE_VALID && + !attr->attisdropped) + { + Form_pg_attribute wholeatt = TupleDescAttr(newTupDesc, i); + + if (wholeatt->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL) + notnull_attrs = lappend_int(notnull_attrs, wholeatt->attnum); + else + notnull_virtual_attrs = lappend_int(notnull_virtual_attrs, + wholeatt->attnum); + } + } + } + + /* + * When adding or changing a virtual generated column with a not-null + * constraint, we need to evaluate whether the generation expression is + * null. For that, we borrow ExecRelGenVirtualNotNull(). Here, we + * prepare a dummy ResultRelInfo. + */ + if (notnull_virtual_attrs != NIL) + { + MemoryContext oldcontext; + + Assert(newTupDesc->constr->has_generated_virtual); + Assert(newTupDesc->constr->has_not_null); + oldcontext = MemoryContextSwitchTo(estate->es_query_cxt); + rInfo = makeNode(ResultRelInfo); + InitResultRelInfo(rInfo, + newPartRel, + 0, /* dummy rangetable index */ + NULL, + estate->es_instrument); + MemoryContextSwitchTo(oldcontext); + } + foreach_oid(merging_oid, mergingPartitions) { ExprContext *econtext; @@ -23470,6 +23542,41 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart evaluateGeneratedExpressionsAndCheckConstraints(tab, newPartRel, insertslot, econtext); + foreach_int(attn, notnull_attrs) + { + if (slot_attisnull(insertslot, attn)) + { + Form_pg_attribute attr = TupleDescAttr(newTupDesc, attn - 1); + + ereport(ERROR, + errcode(ERRCODE_NOT_NULL_VIOLATION), + errmsg("column \"%s\" of relation \"%s\" contains null values", + NameStr(attr->attname), + RelationGetRelationName(newPartRel)), + errtablecol(newPartRel, attn)); + } + } + + if (notnull_virtual_attrs != NIL) + { + AttrNumber attnum; + + attnum = ExecRelGenVirtualNotNull(rInfo, insertslot, + estate, + notnull_virtual_attrs); + if (attnum != InvalidAttrNumber) + { + Form_pg_attribute attr = TupleDescAttr(newTupDesc, attnum - 1); + + ereport(ERROR, + errcode(ERRCODE_NOT_NULL_VIOLATION), + errmsg("column \"%s\" of relation \"%s\" contains null values", + NameStr(attr->attname), + RelationGetRelationName(newPartRel)), + errtablecol(newPartRel, attnum)); + } + } + /* Write the tuple out to the new relation. */ table_tuple_insert(newPartRel, insertslot, mycid, ti_options, bistate); @@ -23493,20 +23600,6 @@ MergePartitionsMoveRows(List **wqueue, List *mergingPartitions, Relation newPart FreeBulkInsertState(bistate); table_finish_bulk_insert(newPartRel, ti_options); - - /* - * We don't need to process this newPartRel since we already processed it - * here, so delete the ALTER TABLE queue for it. - */ - foreach(ltab, *wqueue) - { - tab = (AlteredTableInfo *) lfirst(ltab); - if (tab->relid == RelationGetRelid(newPartRel)) - { - *wqueue = list_delete_cell(*wqueue, ltab); - break; - } - } } /* @@ -23756,6 +23849,7 @@ static void ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, PartitionCmd *cmd, AlterTableUtilityContext *context) { + ListCell *ltab; Relation newPartRel; List *mergingPartitions = NIL; List *extDepState = NIL; @@ -23765,6 +23859,9 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, Oid save_userid; int save_sec_context; int save_nestlevel; + AlteredTableInfo *new_partrel_tab; + Relation thisrel = NULL; + bool hasGenerated = false; /* * Check ownership of merged partitions - partitions with different owners @@ -23926,11 +24023,61 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, list_free(mergingPartitions); + /* Attach a new partition to the partitioned table. */ + attachPartitionTable(wqueue, rel, newPartRel, cmd->bound); + + /* Find the work queue entry for the new partition table: newPartRel. */ + new_partrel_tab = ATGetQueueEntry(wqueue, newPartRel); + /* - * Attach a new partition to the partitioned table. wqueue = NULL: - * verification for each cloned constraint is not needed. + * ALTER TABLE MERGE PARTITIONS may change the data of the new partition + * compared to the combination of the old, merged partitions, because the + * gneration expression on the new partition may differ from the one on + * the merged partitions. Therefore, foreign key constraints on the new + * merged table need reverify whenever the new table has generated + * columns. */ - attachPartitionTable(NULL, rel, newPartRel, cmd->bound); + foreach_ptr(NewColumnValue, ex, new_partrel_tab->newvals) + { + if (ex->is_generated) + { + hasGenerated = true; + break; + } + } + + if (hasGenerated) + { + foreach_ptr(NewConstraint, con, new_partrel_tab->constraints) + { + Constraint *fkconstraint; + Relation refrel; + + if (con->contype != CONSTR_FOREIGN) + continue; + + fkconstraint = (Constraint *) con->qual; + + if (thisrel == NULL) + { + /* Long since locked, no need for another */ + thisrel = table_open(new_partrel_tab->relid, NoLock); + } + + refrel = table_open(con->refrelid, RowShareLock); + + validateForeignKeyConstraint(fkconstraint->conname, thisrel, refrel, + con->refindid, + con->conid, + con->conwithperiod); + + /* + * No need to mark the constraint row as validated, we did that + * when we inserted the row earlier. + */ + table_close(refrel, NoLock); + } + } /* * Apply extension dependencies to the new partition's indexes. This @@ -23949,6 +24096,20 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Restore the userid and security context. */ SetUserIdAndSecContext(save_userid, save_sec_context); + + /* + * We don't need to process this newPartRel since we already processed it + * here, so delete the ALTER TABLE queue for it. + */ + foreach(ltab, *wqueue) + { + tab = (AlteredTableInfo *) lfirst(ltab); + if (tab->relid == RelationGetRelid(newPartRel)) + { + *wqueue = list_delete_cell(*wqueue, ltab); + break; + } + } } /* diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out index ccda2b5843b..77deded5faa 100644 --- a/src/test/regress/expected/partition_merge.out +++ b/src/test/regress/expected/partition_merge.out @@ -1055,6 +1055,55 @@ SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = 1 (1 row) +DROP TABLE t; +-- TEST for recomputation of generated columns with not-null, foreign key and check constraint +CREATE TABLE t ( + id int, + g int GENERATED ALWAYS AS (NULLIF(id, 1)) STORED NOT NULL) PARTITION BY RANGE (id); +CREATE TABLE tp_0_1 (g int GENERATED ALWAYS AS (id) STORED NOT NULL, id int); +ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- error +ERROR: column "g" of relation "tp_0_2" contains null values +DROP TABLE t; +CREATE TABLE t ( + id int, + g int GENERATED ALWAYS AS (NULLIF(id, 1)) NOT NULL) PARTITION BY RANGE (id); +CREATE TABLE tp_0_1 (g int GENERATED ALWAYS AS (id) NOT NULL, id int); +ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- error +ERROR: column "g" of relation "tp_0_2" contains null values +DROP TABLE t; +CREATE TABLE t ( + id int NOT NULL, + g int GENERATED ALWAYS AS (id + 1000) STORED, + CONSTRAINT gcheck CHECK (g < 100)) PARTITION BY RANGE (id); +CREATE TABLE tp_0_1( + id int NOT NULL, + g int GENERATED ALWAYS AS (id) STORED, + CONSTRAINT gcheck CHECK (g < 100)); +ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_02; -- error +ERROR: check constraint "gcheck" of relation "tp_02" is violated by some row +DROP TABLE t; +CREATE TABLE t ( + id int NOT NULL, + pid int GENERATED ALWAYS AS (id + 1000) STORED, + PRIMARY KEY (id), + FOREIGN KEY (pid) REFERENCES t (id) +) PARTITION BY RANGE (id); +CREATE TABLE t1 (id int NOT NULL, pid int GENERATED ALWAYS AS (id) STORED); +ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (0) TO (10); +CREATE TABLE t2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2), (3); +ALTER TABLE t MERGE PARTITIONS (t1, t2) INTO t12; -- error +ERROR: insert or update on table "t12" violates foreign key constraint "t_pid_fkey" +DETAIL: Key (pid)=(1001) is not present in table "t". DROP TABLE t; -- Test for generated columns (different order of columns in partitioned table -- and partitions). diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql index 80dc365b0ce..bd1c7569be5 100644 --- a/src/test/regress/sql/partition_merge.sql +++ b/src/test/regress/sql/partition_merge.sql @@ -757,6 +757,54 @@ SELECT count(*) FROM t WHERE i = 0 AND tab_id IN (SELECT tab_id FROM t WHERE i = DROP TABLE t; +-- TEST for recomputation of generated columns with not-null, foreign key and check constraint +CREATE TABLE t ( + id int, + g int GENERATED ALWAYS AS (NULLIF(id, 1)) STORED NOT NULL) PARTITION BY RANGE (id); +CREATE TABLE tp_0_1 (g int GENERATED ALWAYS AS (id) STORED NOT NULL, id int); +ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- error +DROP TABLE t; + +CREATE TABLE t ( + id int, + g int GENERATED ALWAYS AS (NULLIF(id, 1)) NOT NULL) PARTITION BY RANGE (id); +CREATE TABLE tp_0_1 (g int GENERATED ALWAYS AS (id) NOT NULL, id int); +ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2; -- error +DROP TABLE t; + +CREATE TABLE t ( + id int NOT NULL, + g int GENERATED ALWAYS AS (id + 1000) STORED, + CONSTRAINT gcheck CHECK (g < 100)) PARTITION BY RANGE (id); +CREATE TABLE tp_0_1( + id int NOT NULL, + g int GENERATED ALWAYS AS (id) STORED, + CONSTRAINT gcheck CHECK (g < 100)); +ALTER TABLE t ATTACH PARTITION tp_0_1 FOR VALUES FROM (0) TO (10); +CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2); +ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_02; -- error +DROP TABLE t; + +CREATE TABLE t ( + id int NOT NULL, + pid int GENERATED ALWAYS AS (id + 1000) STORED, + PRIMARY KEY (id), + FOREIGN KEY (pid) REFERENCES t (id) +) PARTITION BY RANGE (id); +CREATE TABLE t1 (id int NOT NULL, pid int GENERATED ALWAYS AS (id) STORED); +ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (0) TO (10); +CREATE TABLE t2 PARTITION OF t FOR VALUES FROM (10) TO (20); +INSERT INTO t VALUES (1), (2), (3); +ALTER TABLE t MERGE PARTITIONS (t1, t2) INTO t12; -- error +DROP TABLE t; + -- Test for generated columns (different order of columns in partitioned table -- and partitions). -- 2.34.1