v43-0001-MERGE-PARTITIONS-constraint-revalidation.no-cfbot
application/octet-stream
Filename: v43-0001-MERGE-PARTITIONS-constraint-revalidation.no-cfbot
Type: application/octet-stream
Part: 0
From 2892e780adc08ed63c1a6e99855f5689a5c5b74c Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
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