From 08080ab67c683ca308e997bce26dafab411c9ee7 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 19 Oct 2026 14:41:50 +0800 Subject: [PATCH v4 2/2] pg_dump: keep ENFORCED on a child's inherited CHECK constraint --- src/backend/commands/tablecmds.c | 23 ++++++- src/bin/pg_dump/pg_dump.c | 107 ++++++++++++++++++++++++++++++- src/bin/pg_dump/pg_dump.h | 1 + src/bin/pg_dump/t/002_pg_dump.pl | 37 +++++++++++ 4 files changed, 164 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 20ddcad4c11..cce4ea7856c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12750,10 +12750,31 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon, if (currcon->conenforced != target_enforced) { ATAlterConstraint updatecon = *cmdcon; + HeapTuple newtup = contuple; + + /* + * Directly setting an inherited constraint of a regular inheritance + * child to ENFORCED while its parents don't enforce it gives the child + * a local definition, just like ALTER TABLE ... ADD CONSTRAINT ... + * ENFORCED on the child would. That way, the constraint stays with the + * child if it is detached from its parents, and pg_dump can dump it as + * part of the child's definition rather than with an additional ALTER + * CONSTRAINT, which would require a rescan of the child. Partitions + * never have local definitions of inherited constraints. + */ + if (!recursing && target_enforced && !currcon->conislocal && + !rel->rd_rel->relispartition) + { + newtup = heap_copytuple(contuple); + ((Form_pg_constraint) GETSTRUCT(newtup))->conislocal = true; + } updatecon.is_enforced = target_enforced; - AlterConstrUpdateConstraintEntry(&updatecon, conrel, contuple); + AlterConstrUpdateConstraintEntry(&updatecon, conrel, newtup); changed = true; + + if (newtup != contuple) + heap_freetuple(newtup); } /* diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 69171d6b859..c8c854a49f4 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -355,6 +355,9 @@ 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 *findParentCheck(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, @@ -9767,14 +9770,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' " @@ -9793,6 +9802,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; @@ -9851,6 +9861,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 @@ -9881,6 +9892,40 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) } } + /* + * A separately dumped CHECK constraint of an inheritance child or + * partition must be restored before the same constraint of its + * ancestors. Otherwise the ancestor's ADD CONSTRAINT recurses and + * creates the child's copy first, so the child's own ADD CONSTRAINT + * then fails (for a partition) or merges into that copy (for an + * inheritance child) instead of creating the constraint. + */ + for (int j = 0; j < numConstrs; j++) + { + ConstraintInfo *con = &constrs[j]; + TableInfo *tbinfo = con->contable; + + if (!con->separate) + continue; + + /* + * A partition's inherited constraint is normally not dumped on + * its own, because the parent's ADD CONSTRAINT recreates it. But + * if the partition's is ENFORCED and the parent's is NOT + * ENFORCED, the recreated one would be NOT ENFORCED, so dump the + * partition's with its own definition, as if it were local. + */ + if (tbinfo->ispartition && !con->conislocal && con->conenforced) + { + ConstraintInfo *pcon = findParentCheck(tbinfo, con->dobj.name); + + if (pcon != NULL && !pcon->conenforced) + con->conislocal = true; + } + + addAncestorCheckDependencies(tbinfo, con); + } + PQclear(res); } @@ -9889,6 +9934,62 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) destroyPQExpBuffer(checkoids); } +/* + * findParentCheck + * + * Return the CHECK constraint named conname of partition tbinfo's parent, or + * NULL if there is none. + */ +static ConstraintInfo * +findParentCheck(TableInfo *tbinfo, const char *conname) +{ + TableInfo *parent; + + Assert(tbinfo->ispartition && tbinfo->numParents == 1); + + parent = tbinfo->parents[0]; + for (int c = 0; parent->checkexprs && c < parent->ncheck; c++) + { + if (strcmp(parent->checkexprs[c].dobj.name, conname) == 0) + return &parent->checkexprs[c]; + } + + return NULL; +} + +/* + * addAncestorCheckDependencies + * + * con is a separately dumped CHECK constraint of table tbinfo. Make the + * same-named CHECK constraint of every ancestor of tbinfo depend on con, so + * that con is restored first. All ancestors are visited, not only the direct + * parents, because each ancestor's ADD CONSTRAINT recurses all the way down. + * + * Only ancestors' separately dumped constraints are considered. One that is + * part of the ancestor's CREATE TABLE is necessarily restored before con, and + * making it depend on con would create a dependency loop. + */ +static void +addAncestorCheckDependencies(TableInfo *tbinfo, ConstraintInfo *con) +{ + for (int p = 0; p < tbinfo->numParents; p++) + { + TableInfo *parent = tbinfo->parents[p]; + + for (int c = 0; parent->checkexprs && c < parent->ncheck; c++) + { + ConstraintInfo *pcon = &parent->checkexprs[c]; + + if (pcon->separate && + strcmp(pcon->dobj.name, con->dobj.name) == 0) + addObjectDependency(&pcon->dobj, con->dobj.dumpId); + } + + /* check further up, in case of multiple levels of inheritance */ + addAncestorCheckDependencies(parent, con); + } +} + /* * 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 c89d699e816..7b09f0276d7 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