From 2892e780adc08ed63c1a6e99855f5689a5c5b74c Mon Sep 17 00:00:00 2001 From: jian he Date: Wed, 11 Jun 2025 11:59:41 +0800 Subject: [PATCH v43 1/1] MERGE PARTITIONS constraint revalidation. ALTER TABLE MERGE PARTITIONS, partitioned table check constraints and generated columns can sometimes reference the tableoid column. Because of this, ALTER TABLE MERGE PARTITIONS must re-validate these constraints. To ensure data integrity, it might be safest to unconditionally recompute all generated expressions and re-evaluate all the check constraints again. --- src/backend/commands/tablecmds.c | 178 +++++++++++++++++- src/test/regress/expected/partition_merge.out | 12 ++ src/test/regress/sql/partition_merge.sql | 4 + 3 files changed, 186 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 92bed4a2dd7..07a3742d25d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -22129,15 +22129,18 @@ getAttributesList(Relation modelRel) /* * createTableConstraints: create constraints, default values and generated * values (prototype is function expandTableLikeClause). + * tab is pending-work queue for newRel, we may need it in moveMergedTablesRows. */ static void -createTableConstraints(Relation modelRel, Relation newRel) +createTableConstraints(List **wqueue, AlteredTableInfo *tab, + Relation modelRel, Relation newRel) { TupleDesc tupleDesc; TupleConstr *constr; AttrMap *attmap; AttrNumber parent_attno; int ccnum; + ListCell *lcon; List *cookedConstraints = NIL; tupleDesc = RelationGetDescr(modelRel); @@ -22172,6 +22175,7 @@ createTableConstraints(Relation modelRel, Relation newRel) bool found_whole_row; AttrNumber num; Node *def; + NewColumnValue *newval; if (attribute->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL) this_default = build_generation_expression(modelRel, attribute->attnum); @@ -22201,6 +22205,21 @@ createTableConstraints(Relation modelRel, Relation newRel) /* Add a pre-cooked default expression. */ StoreAttrDefault(newRel, num, def, true); + + /* + * Stored generated column expressions in modelRel might reference + * tableoid. newRel, modelRel tableoid clear is not the same. If + * so, these stored generated columns require recomputation for + * newRel within moveMergedTablesRows. + */ + if (attribute->attgenerated == ATTRIBUTE_GENERATED_STORED) + { + newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue)); + newval->attnum = num; + newval->expr = expression_planner((Expr *) def); + newval->is_generated = (attribute->attgenerated != '\0'); + tab->newvals = lappend(tab->newvals, newval); + } } } @@ -22252,6 +22271,29 @@ createTableConstraints(Relation modelRel, Relation newRel) /* Store CHECK constraints. */ StoreConstraints(newRel, cookedConstraints, true); + /* + * modelRel check constraint expresssion may reference tableoid, so later in + * moveMergedTablesRows, we need evulate the check constraint again for the + * newRel. We can check weather check constraint contain tableoid reference + * or not via pull_varattnos. But unconditionaly revaulate check cosntraint + * seems more safe. + */ + foreach(lcon, cookedConstraints) + { + CookedConstraint *ccon = (CookedConstraint *) lfirst(lcon); + + if (!ccon->skip_validation && ccon->contype == CONSTR_CHECK) + { + NewConstraint *newcon; + newcon = (NewConstraint *) palloc0(sizeof(NewConstraint)); + newcon->name = ccon->name; + newcon->contype = ccon->contype; + newcon->qual = ccon->expr; + + tab->constraints = lappend(tab->constraints, newcon); + } + } + /* Don't need the cookedConstraints any more. */ list_free_deep(cookedConstraints); @@ -22287,7 +22329,8 @@ createTableConstraints(Relation modelRel, Relation newRel) * Function returns the created relation (locked in AccessExclusiveLock mode). */ static Relation -createPartitionTable(RangeVar *newPartName, Relation modelRel, Oid ownerId) +createPartitionTable(List **wqueue, RangeVar *newPartName, + Relation modelRel, Oid ownerId) { Relation newRel; Oid newRelId; @@ -22296,6 +22339,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel, Oid ownerId) List *colList = NIL; Oid relamId; Oid namespaceId; + AlteredTableInfo *new_partrel_tab; /* If existing rel is temp, it must belong to this session */ if (modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP && @@ -22356,6 +22400,9 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel, Oid ownerId) */ newRel = table_open(newRelId, NoLock); + /* Find or create work queue entry for newly created table */ + new_partrel_tab = ATGetQueueEntry(wqueue, newRel); + /* * We intended to create the partition with the same persistence as the * parent table, but we still need to recheck because that might be @@ -22378,7 +22425,12 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel, Oid ownerId) RelationGetRelationName(modelRel))); /* Create constraints, default values and generated values */ - createTableConstraints(modelRel, newRel); + createTableConstraints(wqueue, new_partrel_tab, modelRel, newRel); + + /* + * need CCI, so fresh relcache entry have newly installed constraint info. + */ + CommandCounterIncrement(); return newRel; } @@ -22386,19 +22438,64 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel, Oid ownerId) /* * moveMergedTablesRows: scan partitions to be merged (mergingPartitionsList) * of the partitioned table (rel) and move rows into the new partition - * (newPartRel). + * (newPartRel). We also reevaulate check constraints against these rows. */ static void -moveMergedTablesRows(Relation rel, List *mergingPartitionsList, - Relation newPartRel) +moveMergedTablesRows(List **wqueue, Relation rel, + List *mergingPartitionsList, Relation newPartRel) { CommandId mycid; + EState *estate; + ExprContext *econtext; + AlteredTableInfo *tab; + ListCell *l; + ListCell *ltab; /* The FSM is empty, so don't bother using it. */ int ti_options = TABLE_INSERT_SKIP_FSM; BulkInsertState bistate; /* state of bulk inserts for partition */ TupleTableSlot *dstslot; + /* Find the work queue entry for new partition table: newPartRel */ + tab = ATGetQueueEntry(wqueue, newPartRel); + + /* + * Generate the constraint and default execution states + */ + estate = CreateExecutorState(); + + /* Build the needed expression execution states */ + foreach(l, tab->constraints) + { + NewConstraint *con = lfirst(l); + + switch (con->contype) + { + case CONSTR_CHECK: + con->qualstate = ExecPrepareExpr((Expr *) expand_generated_columns_in_expr(con->qual, newPartRel, 1), estate); + break; + case CONSTR_FOREIGN: + /* Nothing to do here */ + break; + case CONSTR_NOTNULL: + /* Nothing to do here */ + break; + default: + elog(ERROR, "unrecognized constraint type: %d", + (int) con->contype); + } + } + + foreach(l, tab->newvals) + { + NewColumnValue *ex = lfirst(l); + + /* expr already planned */ + ex->exprstate = ExecInitExpr((Expr *) ex->expr, NULL); + } + + econtext = GetPerTupleExprContext(estate); + mycid = GetCurrentCommandId(true); /* Prepare a BulkInsertState for table_tuple_insert. */ @@ -22456,9 +22553,62 @@ moveMergedTablesRows(Relation rel, List *mergingPartitionsList, 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. + */ + econtext->ecxt_scantuple = insertslot; + foreach(l, tab->newvals) + { + NewColumnValue *ex = lfirst(l); + + if (!ex->is_generated) + continue; + + insertslot->tts_values[ex->attnum - 1] + = ExecEvalExpr(ex->exprstate, + econtext, + &insertslot->tts_isnull[ex->attnum - 1]); + } + + foreach(l, tab->constraints) + { + NewConstraint *con = lfirst(l); + + 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); + } + } + /* Write the tuple out to the new relation. */ table_tuple_insert(newPartRel, insertslot, mycid, ti_options, bistate); + + ResetExprContext(econtext); } table_endscan(scan); @@ -22470,10 +22620,22 @@ moveMergedTablesRows(Relation rel, List *mergingPartitionsList, ExecDropSingleTupleTableSlot(srcslot); } + FreeExecutorState(estate); ExecDropSingleTupleTableSlot(dstslot); FreeBulkInsertState(bistate); table_finish_bulk_insert(newPartRel, ti_options); + + /* + * in Phase3, we don't need process this newPartRel. since we already + * processed in here, so delete the ALTER TABLE queue of it. + */ + foreach(ltab, *wqueue) + { + tab = (AlteredTableInfo *) lfirst(ltab); + if (tab->relid == RelationGetRelid(newPartRel)) + *wqueue = list_delete_cell(*wqueue, ltab); + } } /* @@ -22597,7 +22759,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Create table for new partition, use partitioned table as model. */ Assert(OidIsValid(ownerId)); - newPartRel = createPartitionTable(cmd->name, rel, ownerId); + newPartRel = createPartitionTable(wqueue, cmd->name, rel, ownerId); /* * Switch to the table owner's userid, so that any index functions are run @@ -22613,7 +22775,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, RestrictSearchPath(); /* Copy data from merged partitions to new partition. */ - moveMergedTablesRows(rel, mergingPartitionsList, newPartRel); + moveMergedTablesRows(wqueue, rel, mergingPartitionsList, newPartRel); /* Drop the current partitions before attaching the new one. */ foreach_ptr(RelationData, mergingPartition, mergingPartitionsList) diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out index e76cea2a03b..064206b9737 100644 --- a/src/test/regress/expected/partition_merge.out +++ b/src/test/regress/expected/partition_merge.out @@ -1036,6 +1036,8 @@ ALTER TABLE t ADD CONSTRAINT t_b_check CHECK (b > 0); ALTER TABLE t ADD CONSTRAINT t_b_check1 CHECK (b > 0) NOT ENFORCED; ALTER TABLE t ADD CONSTRAINT t_b_check2 CHECK (b > 0) NOT VALID; ALTER TABLE t ADD CONSTRAINT t_b_nn NOT NULL b NOT VALID; +INSERT INTO tp_0_1(i, t, b) VALUES(0, DEFAULT, 1); +INSERT INTO tp_1_2(i, t, b) VALUES(1, DEFAULT, 2); CREATE OR REPLACE FUNCTION trigger_function() RETURNS trigger LANGUAGE 'plpgsql' AS $BODY$ BEGIN @@ -1093,6 +1095,16 @@ Not-null constraints: Triggers: t_before_insert_row_trigger BEFORE INSERT ON tp_0_1 FOR EACH ROW EXECUTE FUNCTION trigger_function('t'), ON TABLE t +INSERT INTO t(i, t, b) VALUES(1, DEFAULT, 3); +NOTICE: trigger(t) called: action = INSERT, when = BEFORE, level = ROW +SELECT tableoid::regclass, * FROM t ORDER BY b; + tableoid | i | t | b | d +----------+---+----------------+---+------------ + tp_0_1 | 0 | default_tp_0_1 | 1 | 01-01-2022 + tp_0_1 | 1 | default_tp_1_2 | 2 | 01-01-2022 + tp_0_1 | 1 | default_t | 3 | 01-01-2022 +(3 rows) + DROP TABLE t; DROP FUNCTION trigger_function(); -- Test MERGE PARTITIONS with not valid foreign key constraint diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql index d72af062eee..a832eb855ed 100644 --- a/src/test/regress/sql/partition_merge.sql +++ b/src/test/regress/sql/partition_merge.sql @@ -698,6 +698,8 @@ ALTER TABLE t ADD CONSTRAINT t_b_check1 CHECK (b > 0) NOT ENFORCED; ALTER TABLE t ADD CONSTRAINT t_b_check2 CHECK (b > 0) NOT VALID; ALTER TABLE t ADD CONSTRAINT t_b_nn NOT NULL b NOT VALID; +INSERT INTO tp_0_1(i, t, b) VALUES(0, DEFAULT, 1); +INSERT INTO tp_1_2(i, t, b) VALUES(1, DEFAULT, 2); CREATE OR REPLACE FUNCTION trigger_function() RETURNS trigger LANGUAGE 'plpgsql' AS $BODY$ BEGIN @@ -717,6 +719,8 @@ CREATE TRIGGER tp_1_2_before_insert_row_trigger BEFORE INSERT ON tp_1_2 FOR EACH ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_1; \d+ tp_0_1 +INSERT INTO t(i, t, b) VALUES(1, DEFAULT, 3); +SELECT tableoid::regclass, * FROM t ORDER BY b; DROP TABLE t; DROP FUNCTION trigger_function(); -- 2.34.1