On Fri, Oct 9, 2026 at 12:42 PM shihao zhong <[email protected]> wrote: > > Hi hackers, > > I used Opus to analyze the new features in PG19, and it found three > bugs with a CHECK constraint that a child enforces and its parent > does not. Each item has a script and a patch with its number. > "Before" is 19 and master today. "After" is with the patch, and is > the output I expect. > > 3. pg_dump prints only the parent's definition, so the child is NOT > ENFORCED after a restore. pg_upgrade is fine. > > create table p3 (a int, constraint c check (a > 0) not enforced); > create table k3 () inherits (p3); > alter table k3 alter constraint c enforced; > Before, pg_dump -s has: > ALTER TABLE public.p3 > ADD CONSTRAINT c CHECK ((a > 0)) NOT ENFORCED; > After 0003, it also has: > ALTER TABLE public.k3 ALTER CONSTRAINT c ENFORCED;
Hi. Let pg_dump produce ALTER CONSTRAINT seems weird. We can let pg_dump produce inline constraint (within CREATE TABLE statement) or ALTER TABLE ADD CONSTRAINT, see the attached. -- jian https://www.enterprisedb.com/
From c46cebb633ea3a369bf984b45ef82e9885785b2d Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Mon, 12 Oct 2026 16:11:54 +0800 Subject: [PATCH v2 1/1] pg_dump: keep child table CHECK constraint status Normally, dumping a parent's CHECK constraint also takes care of the child's non-local copy of it. However, if the parent's constraint is NOT ENFORCED but the child's is ENFORCED, the child would lose its ENFORCED status that way. In that case, treat the child's constraint as local, so that it is dumped with its own definition. In a partition hierarchy, the parent's constraint can be NOT ENFORCED while the partition's is ENFORCED NOT VALID. In that case, pg_dump must emit an ALTER TABLE ... ADD CONSTRAINT entry for partitioned table and the partition, and the partition's must be restored first; otherwise the partitioned table's ADD CONSTRAINT reaches the partition first, and the partition's own ADD CONSTRAINT then fails because the constraint already exists. --- src/bin/pg_dump/pg_dump.c | 104 ++++++++++++++++++++++++++++++- src/bin/pg_dump/pg_dump.h | 1 + src/bin/pg_dump/t/002_pg_dump.pl | 37 +++++++++++ 3 files changed, 139 insertions(+), 3 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 90971fbdac6..2ca94d328fe 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -355,6 +355,10 @@ static void getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables, c static void makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo); static void buildMatViewRefreshDependencies(Archive *fout); static void getTableDataFKConstraints(void); +static ConstraintInfo *findCheckConstraint(TableInfo *tbinfo, + const char *conname); +static void addAncestorCheckDependencies(TableInfo *tbinfo, + ConstraintInfo *con); static void determineNotNullFlags(Archive *fout, PGresult *res, int r, TableInfo *tbinfo, int j, int i_notnull_name, @@ -9758,14 +9762,20 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) int i_consrc; int i_conislocal; int i_convalidated; + int i_conenforced; pg_log_info("finding table check constraints"); resetPQExpBuffer(q); + appendPQExpBufferStr(q, + "SELECT c.tableoid, c.oid, conrelid, conname, " + "pg_catalog.pg_get_constraintdef(c.oid) AS consrc, " + "conislocal, convalidated, "); + if (fout->remoteVersion >= 180000) + appendPQExpBufferStr(q, "conenforced "); + else + appendPQExpBufferStr(q, "true AS conenforced "); appendPQExpBuffer(q, - "SELECT c.tableoid, c.oid, conrelid, conname, " - "pg_catalog.pg_get_constraintdef(c.oid) AS consrc, " - "conislocal, convalidated " "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_constraint c ON (src.tbloid = c.conrelid)\n" "WHERE contype = 'c' " @@ -9784,6 +9794,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) i_consrc = PQfnumber(res, "consrc"); i_conislocal = PQfnumber(res, "conislocal"); i_convalidated = PQfnumber(res, "convalidated"); + i_conenforced = PQfnumber(res, "conenforced"); /* As above, this loop iterates once per table, not once per row */ curtblindx = -1; @@ -9842,6 +9853,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) constrs[j].condeferrable = false; constrs[j].condeferred = false; constrs[j].conislocal = (PQgetvalue(res, j, i_conislocal)[0] == 't'); + constrs[j].conenforced = (PQgetvalue(res, j, i_conenforced)[0] == 't'); /* * An unvalidated constraint needs to be dumped separately, so @@ -9872,6 +9884,53 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) } } + /* + * Normally, dumping a parent's CHECK constraint also takes care of + * the child's non-local copy of it. However, if the parent's + * constraint is NOT ENFORCED but the child's is ENFORCED, the child + * would lose its ENFORCED status that way. In that case, treat the + * child's constraint as local, so that it is dumped with its own + * definition. + */ + for (int j = 0; j < numConstrs; j++) + { + ConstraintInfo *con = &constrs[j]; + TableInfo *tbinfo = con->contable; + bool parent_enforced = false; + + if (!con->conenforced || con->conislocal) + continue; + + /* + * NOT VALID constraints are already dumped separately, after the + * data. A partition's must also be ordered before its ancestors' + * constraints, so only those are handled here. + */ + if (con->separate && !tbinfo->ispartition) + continue; + + for (int p = 0; p < tbinfo->numParents; p++) + { + ConstraintInfo *pcon = findCheckConstraint(tbinfo->parents[p], + con->dobj.name); + + if (pcon && pcon->conenforced) + parent_enforced = true; + } + if (parent_enforced) + continue; + + con->conislocal = true; + + /* + * A separately dumped one must be restored before its ancestors' + * constraints: ADD CONSTRAINT on a partition cannot merge into + * the copy that an ancestor's ADD CONSTRAINT has already created. + */ + if (con->separate && tbinfo->ispartition) + addAncestorCheckDependencies(tbinfo, con); + } + PQclear(res); } @@ -9880,6 +9939,45 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) destroyPQExpBuffer(checkoids); } +/* + * findCheckConstraint + * + * Return tbinfo's CHECK constraint named conname, or NULL if there is none + * or the table's constraints were not loaded. + */ +static ConstraintInfo * +findCheckConstraint(TableInfo *tbinfo, const char *conname) +{ + for (int c = 0; tbinfo->checkexprs && c < tbinfo->ncheck; c++) + { + if (strcmp(tbinfo->checkexprs[c].dobj.name, conname) == 0) + return &tbinfo->checkexprs[c]; + } + return NULL; +} + +/* + * addAncestorCheckDependencies + * + * con is an ENFORCED, NOT VALID CHECK constraint of partition tbinfo whose + * ancestors have it as NOT ENFORCED. Make each ancestor's constraint depend + * on con, so that con is restored first: every ancestor's ADD CONSTRAINT + * recurses down to the partition, not only the direct parent's. + */ +static void +addAncestorCheckDependencies(TableInfo *tbinfo, ConstraintInfo *con) +{ + while (tbinfo->ispartition) + { + ConstraintInfo *pcon; + + tbinfo = tbinfo->parents[0]; + pcon = findCheckConstraint(tbinfo, con->dobj.name); + if (pcon) + addObjectDependency(&pcon->dobj, con->dobj.dumpId); + } +} + /* * Based on the getTableAttrs query's row corresponding to one column, set * the name and flags to handle a not-null constraint for that column in diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 2bbb5d5773b..e8b69240afa 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -524,6 +524,7 @@ typedef struct _constraintInfo bool condeferred; /* true if constraint is INITIALLY DEFERRED */ bool conperiod; /* true if the constraint is WITHOUT OVERLAPS */ bool conislocal; /* true if constraint has local definition */ + bool conenforced; /* true if CHECK constraint is ENFORCED */ bool separate; /* true if must dump as separate item */ } ConstraintInfo; diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 63d80268ca2..5bae901df18 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1120,6 +1120,43 @@ my %tests = ( }, }, + 'CONSTRAINT CHECK / ENFORCED only on child' => { + create_sql => 'CREATE TABLE dump_test.test_table_ne (col1 int CONSTRAINT ck CHECK (col1 > 0) NOT ENFORCED); + CREATE TABLE dump_test.test_table_ne_c () INHERITS (dump_test.test_table_ne); + ALTER TABLE dump_test.test_table_ne_c ALTER CONSTRAINT ck ENFORCED;', + regexp => qr/^ + \QCREATE TABLE dump_test.test_table_ne_c (\E\n + \s+\QCONSTRAINT ck CHECK ((col1 > 0))\E\n + /xm, + like => { + %full_runs, %dump_test_schema_runs, section_pre_data => 1, + }, + unlike => { + exclude_dump_test_schema => 1, + only_dump_measurement => 1, + binary_upgrade => 1, + }, + }, + + 'CONSTRAINT CHECK / NOT VALID only on partition' => { + create_sql => 'CREATE TABLE dump_test.test_table_nv (col1 int CONSTRAINT ck CHECK (col1 > 0) NOT ENFORCED) PARTITION BY LIST (col1); + CREATE TABLE dump_test.test_table_nv_1 (col1 int); + ALTER TABLE dump_test.test_table_nv_1 ADD CONSTRAINT ck CHECK (col1 > 0) NOT VALID; + ALTER TABLE dump_test.test_table_nv ATTACH PARTITION dump_test.test_table_nv_1 FOR VALUES IN (1);', + regexp => qr/^ + \QALTER TABLE dump_test.test_table_nv_1\E\n + .*^ + \QALTER TABLE dump_test.test_table_nv\E\n + /xms, + like => { + %full_runs, %dump_test_schema_runs, section_post_data => 1, + }, + unlike => { + exclude_dump_test_schema => 1, + only_dump_measurement => 1, + }, + }, + 'CONSTRAINT NOT NULL / NO INHERIT' => { create_sql => 'CREATE TABLE dump_test.test_table_nonn ( col1 int NOT NULL NO INHERIT, -- 2.34.1
