hi. attached patch is for address pg_dump inconsistency when parent is "not null not valid" while child is "not null".
The following query before/after pg_dump should return the same result. select conrelid::regclass::text, conname, convalidated, coninhcount, conislocal, conparentid, contype from pg_constraint where conrelid::regclass::text = ANY('{inhnn, inhnn_cc, inhnn_cc_1}') order by 1,2; --test cases: CREATE TABLE inhnn (a INTEGER); ALTER TABLE inhnn ADD CONSTRAINT cc not null a NOT VALID; CREATE TABLE inhnn_cc(a INTEGER) INHERITS(inhnn); CREATE TABLE inhnn_cc_1(a INTEGER) INHERITS(inhnn_cc, inhnn); master pg_dump output is: CREATE TABLE public.inhnn (a integer); CREATE TABLE public.inhnn_cc (a integer) INHERITS (public.inhnn); CREATE TABLE public.inhnn_cc_1 (a integer) INHERITS (public.inhnn_cc, public.inhnn); ALTER TABLE public.inhnn ADD CONSTRAINT cc NOT NULL a NOT VALID; with the attached patch, pg_dump output is: CREATE TABLE public.inhnn (a integer); CREATE TABLE public.inhnn_cc (a integer CONSTRAINT cc NOT NULL) INHERITS (public.inhnn); CREATE TABLE public.inhnn_cc_1 (a integer CONSTRAINT cc NOT NULL) INHERITS (public.inhnn_cc, public.inhnn); ALTER TABLE public.inhnn ADD CONSTRAINT cc NOT NULL a NOT VALID; ------------- As you can see, in master, pg_dump will make {inhnn, inhnn_cc, inhnn_cc_1} not-null constraint's pg_constraint.convalidated set as false. but we should only make inhnn's not-null constraint convalidated as false.
From 9505f36287403aa8efd7642dddf71b77996796dd Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Wed, 9 Apr 2025 13:07:58 +0800 Subject: [PATCH v1 1/1] pg_dump not null not valid make sure pg_dump have the same pg_constraint meta before and after pg_dump. if the parent not-null constraint is invalid, child is valid. then pg_dump need locally print the not-null constraint on that child too. otherwise pg_dump may make child's convalidated may set to false. that means we also need adjust conislocal in AdjustNotNullInheritance. --- src/backend/catalog/pg_constraint.c | 10 ++++++++++ src/bin/pg_dump/common.c | 4 ++++ src/bin/pg_dump/pg_dump.c | 17 +++++++++++++++++ src/bin/pg_dump/pg_dump.h | 5 +++++ 4 files changed, 36 insertions(+) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 2f73085961b..9e65b96143f 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -776,6 +776,16 @@ AdjustNotNullInheritance(Oid relid, AttrNumber attnum, ereport(ERROR, errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("too many inheritance parents")); + + /* + * If the child already has a valid constraint and we are + * creating an invalid one with same definition on it. The + * child's constraint will remain valid, but can no longer be + * marked as local. + */ + if (is_notvalid && conform->convalidated && conform->conenforced) + conform->conislocal = false; + changed = true; } else if (!conform->conislocal) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 56b6c368acf..ff6a4eacda0 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -546,6 +546,10 @@ flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables parent->notnull_constrs[inhAttrInd] != NULL) foundNotNull = true; + if (fout->remoteVersion >= 180000 && + parent->notnull_invalid[inhAttrInd]) + tbinfo->notnull_parent_invalid[j] = true; + foundDefault |= (parentDef != NULL && strcmp(parentDef->adef_expr, "NULL") != 0 && !parent->attgenerated[inhAttrInd]); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 25264f8c9fb..8d131523366 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -9255,6 +9255,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attfdwoptions = (char **) pg_malloc(numatts * sizeof(char *)); tbinfo->attmissingval = (char **) pg_malloc(numatts * sizeof(char *)); tbinfo->notnull_constrs = (char **) pg_malloc(numatts * sizeof(char *)); + tbinfo->notnull_invalid = (bool *) pg_malloc(numatts * sizeof(bool)); + tbinfo->notnull_parent_invalid = (bool *) pg_malloc(numatts * sizeof(bool)); tbinfo->notnull_noinh = (bool *) pg_malloc(numatts * sizeof(bool)); tbinfo->notnull_islocal = (bool *) pg_malloc(numatts * sizeof(bool)); tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *)); @@ -9280,6 +9282,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attlen[j] = atoi(PQgetvalue(res, r, i_attlen)); tbinfo->attalign[j] = *(PQgetvalue(res, r, i_attalign)); tbinfo->attislocal[j] = (PQgetvalue(res, r, i_attislocal)[0] == 't'); + tbinfo->notnull_parent_invalid[j] = false; /* it only change in flagInhAttrs */ + tbinfo->notnull_invalid[j] = false; /* it only change in determineNotNullFlags */ /* Handle not-null constraint name and flags */ determineNotNullFlags(fout, res, r, @@ -9758,6 +9762,7 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r, /* nothing else to do */ tbinfo->notnull_constrs[j] = NULL; + tbinfo->notnull_invalid[j] = true; return; } @@ -16986,6 +16991,18 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) tbinfo->attrdefs[j]->adef_expr); } + /* + * if parent have invalid not-null, child have valid + * not-null, then we print not null on child too. later + * parent's invalid not-null will generate a ALTER TABLE ADD + * CONSTRAINT, which will cascade to children, which is + * fine. + */ + if (!print_notnull && + tbinfo->notnull_constrs[j] != NULL && + tbinfo->notnull_parent_invalid[j]) + print_notnull = true; + if (print_notnull) { if (tbinfo->notnull_constrs[j][0] == '\0') diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index b426b5e4736..6c2f963dcc4 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -365,6 +365,11 @@ typedef struct _tableInfo * there isn't one on this column. If * empty string, unnamed constraint * (pre-v17) */ + bool *notnull_invalid; /* true for NOT NULL NOT VALID */ + + /* true if parent table NOT NULL constraint is NOT VALID */ + bool *notnull_parent_invalid; + bool *notnull_noinh; /* NOT NULL is NO INHERIT */ bool *notnull_islocal; /* true if NOT NULL has local definition */ struct _attrDefInfo **attrdefs; /* DEFAULT expressions */ -- 2.34.1