On Sat Aug 1, 2026 at 7:57 AM CEST, jian he wrote: > Hi. > > Reproducer: > > drop table if exists t; > CREATE TABLE t ( > a int, > b int GENERATED ALWAYS AS (NULLIF(a, 1)) STORED NOT NULL, > constraint cc check (b > 0)) PARTITION BY RANGE (a); > CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (10) TO (20); > ALTER TABLE tp_0_1 ALTER COLUMN b SET EXPRESSION AS (a); > > -- expect two row, now nothing > select conname, contype from pg_constraint where conrelid = > 'tp_0_1'::regclass;
It's admittedly a long shot, since I'm still studying all of this, but: what if, when we RememberConstraintForRebuilding() a constraint with conislocal=false, we also RememberConstraintForRebuilding() its corresponding parent constraint? I have not fully considered the implications of this and I'm only thinking about CHECK and NN constraints. The attached patch is not meant for code-review, but just to illustrate the idea and get the conversation started. What do you think? Regards, Alberto -- Alberto Piai Sensational AG Zürich, Switzerland
>From a032841bc087743dea4095238060cd01f74a7350 Mon Sep 17 00:00:00 2001 From: Alberto Piai <[email protected]> Date: Mon, 3 Aug 2026 16:46:33 +0200 Subject: [PATCH v1] WIP: fix ATPostAlterTypeCleanup dropping constraints on partitions When called on a partition / child table, ATPostAlterTypeCleanup doesn't enqueue rebuild commands for inherited constraint, with the assumption that they will be rebuilt later when the function is called on the parent table. When an ALTER TABLE command which causes a rewrite is called directly on the partition / child table though, ATPostAlterTypeCleanup is never called on the parent table, so the constraint is never recreated. The attached test case reproduces this using SET EXPRESSION. This is not meant for code-review but just to illustrate the idea I'm exploring: what if when we RememberConstraintForRebuilding() a constraint with conislocal=false, we also RememberConstraintForRebuilding() its corresponding parent constraint? The code is incomplete, I haven't fully considered all the implications and I'm only thinking about CHECK and NN constraints. It's just to illustrate the idea and get a discussion started. Reported-by: Jian He <[email protected]> --- src/backend/commands/tablecmds.c | 105 +++++++++++++++++++++- src/test/regress/expected/alter_table.out | 17 ++++ src/test/regress/sql/alter_table.sql | 8 ++ 3 files changed, 127 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 2fa534413ea..af3a2c95c6b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -15561,6 +15561,84 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, return address; } +static Oid +get_parent_constraint(HeapTuple constraintTup) +{ + AttrMap *attmap; + Relation pg_constraints; + ScanKeyData key[3]; + SysScanDesc scan; + HeapTuple tuple; + Relation parentRel; + Relation conRel; + AttrNumber child_attno; + Oid result = InvalidOid; + Oid parentRelid = InvalidOid; + Form_pg_constraint child_con = GETSTRUCT(constraintTup); + + child_attno = child_con->contype == CONSTRAINT_NOTNULL ? extractNotNullColumn(constraintTup) : 0; + + parentRelid = get_partition_parent(child_con->conrelid, false); + + /* scan the parent constraints, matching by name for CHECK and by attno for NN */ + pg_constraints = table_open(ConstraintRelationId, AccessShareLock); + ScanKeyInit(&key[0], Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(parentRelid)); + ScanKeyInit(&key[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + if (child_con->contype == CONSTRAINT_CHECK) + ScanKeyInit(&key[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(NameStr(child_con->conname))); + + scan = systable_beginscan(pg_constraints, + ConstraintRelidTypidNameIndexId, true, NULL, + child_con->contype == CONSTRAINT_CHECK ? 3 : 2, key); + + while (HeapTupleIsValid(tuple = systable_getnext(scan))) + { + Form_pg_constraint con = GETSTRUCT(tuple); + + if (con->contype != child_con->contype) + continue; + + if (con->connoinherit) + continue; + + if (con->contype == CONSTRAINT_CHECK) + { + /* Got a match by name */ + result = con->oid; + break; + } + if (con->contype == CONSTRAINT_NOTNULL) + { + AttrNumber parent_attno = extractNotNullColumn(tuple); + + parentRel = RelationIdGetRelation(parentRelid); + conRel = RelationIdGetRelation(child_con->conrelid); + attmap = build_attrmap_by_name(RelationGetDescr(parentRel), + RelationGetDescr(conRel), + true); + RelationClose(conRel); + RelationClose(parentRel); + + /* Got a match by attno */ + if (parent_attno == attmap->attnums[child_attno - 1]) + result = con->oid; + break; + } + } + + systable_endscan(scan); + table_close(pg_constraints, AccessShareLock); + + return result; +} + /* * Subroutine for ATExecAlterColumnType and ATExecSetExpression: Find everything * that depends on the column (constraints, indexes, etc), and record enough @@ -15634,9 +15712,30 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, } case ConstraintRelationId: - Assert(foundObject.objectSubId == 0); - RememberConstraintForRebuilding(foundObject.objectId, tab); - break; + { + Oid parentConstraintId = InvalidOid; + HeapTuple tuple; + Form_pg_constraint con; + + Assert(foundObject.objectSubId == 0); + + tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(foundObject.objectId)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for constraint %u", + foundObject.objectId); + + con = (Form_pg_constraint) GETSTRUCT(tuple); + + if (!con->conislocal && OidIsValid(parentConstraintId = get_parent_constraint(tuple))) + { + RememberConstraintForRebuilding(parentConstraintId, tab); + } + + ReleaseSysCache(tuple); + + RememberConstraintForRebuilding(foundObject.objectId, tab); + break; + } case ProcedureRelationId: diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index e167a41ce79..60f5302d02e 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -4876,3 +4876,20 @@ drop publication pub1; drop schema alter1 cascade; drop schema alter2 cascade; NOTICE: drop cascades to table alter2.t1 +-- Check that non-recursive SET EXPRESSION restores the constraints correctly +create table tab (a int, b int not null generated always as (a+1) stored) partition by list (a); +create table part partition of tab for values in (1); +select conname, contype from pg_constraint where conrelid = 'part'::regclass; + conname | contype +----------------+--------- + tab_b_not_null | n +(1 row) + +alter table part alter b set expression as (a); +select conname, contype from pg_constraint where conrelid = 'part'::regclass; + conname | contype +----------------+--------- + tab_b_not_null | n +(1 row) + +drop table tab; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index f5f13bbd3e7..da37d591d73 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -3159,3 +3159,11 @@ alter table alter1.t1 set schema alter2; drop publication pub1; drop schema alter1 cascade; drop schema alter2 cascade; + +-- Check that non-recursive SET EXPRESSION restores the constraints correctly +create table tab (a int, b int not null generated always as (a+1) stored) partition by list (a); +create table part partition of tab for values in (1); +select conname, contype from pg_constraint where conrelid = 'part'::regclass; +alter table part alter b set expression as (a); +select conname, contype from pg_constraint where conrelid = 'part'::regclass; +drop table tab; base-commit: fd2b89854d93d70fe8c9a69d5b8fafd5b9302cfc -- 2.47.0
