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 <[email protected]>
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
