Hello,
Thanks!
I noticed a typo 'constrint' in several places; fixed in the attached.
I discovered that this sequence, taken from added regression tests,
CREATE TABLE notnull_tbl1 (a int);
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn_parent not null a not valid;
CREATE TABLE notnull_chld (a int);
ALTER TABLE notnull_chld ADD CONSTRAINT nn_child not null a not valid;
ALTER TABLE notnull_chld INHERIT notnull_tbl1;
ALTER TABLE notnull_tbl1 VALIDATE CONSTRAINT nn_parent;
does mark the constraint as validated in the child, but only in
pg_constraint -- pg_attribute continues to be marked as 'i', so if you
try to use it for a PK, it fails:
alter table notnull_chld add constraint foo primary key (a);
ERROR: column "a" of table "notnull_chld" is marked as NOT VALID NOT NULL
constrint
I thought that was going to be a quick fix, so I tried to do so; since
we already have a function 'set_attnotnull', I thought it was the
perfect tool to changing attnotnull. However, it's not great, because
since that part of the code is already doing the validation, I don't
want it to queue the validation again, so the API needs a tweak; I
changed it to receiving separately which new value to update attnotnull
to, and whether to queue validation. With that change it works
correctly, but it is a bit ugly at the callers' side. Maybe it works to
pass two booleans instead? Please have a look at whether that can be
improved.
I also noticed the addition of function getNNConnameForAttnum(), which
does pretty much the same as findNotNullConstraintAttnum(), only it
ignores all validate constraints instead of ignoring all non-validated
constraints. So after looking at the callers of the existing function
and wondering which ones of them really wanted only the validated
constraints? It turns that the answer is none of them. So I decided to
remove the check for that, and instead we need to add checks to every
caller of both findNotNullConstraintAttnum() and findNotNullConstraint()
so that it acts appropriately when a non-validated constraint is
returned. I added a few elog(WARNING)s when this happens; running the
tests I notice that none of them fire. I'm pretty sure this indicates
holes in testing: we have no test cases for these scenarios, and we
should have them for assurance that we're doing the right things. I
recommend that you go over those WARNINGs, add test cases that make them
fire, and then fix the code so that the test cases do the right thing.
Also, just to be sure, please go over _all_ the callers of
those two functions and make sure all cases are covered by tests that
catch invalid constraints.
I also noticed that in the one place where getNNConnameForAttnum() was
called, we were passing the parent table's column number. But in child
tables, even in partitions, the column numbers can differ from parent to
children. So we need to walk down the hierarchy using the column name,
not the column number. This would have become visible if the test cases
had included inheritance trees with varying column shapes.
The docs continue to say this:
This form adds a new constraint to a table using the same constraint
syntax as <link linkend="sql-createtable"><command>CREATE
TABLE</command></link>, plus the option <literal>NOT
VALID</literal>, which is currently only allowed for foreign key
and CHECK constraints.
which is missing to indicate that NOT VALID is valid for NOT NULL.
Also I think the docs for attnotnull in catalogs.sgml are a bit too
terse; I would write "The value 't' indicates that a not-null constraint
exists for the column; 'i' for an invalid constraint, 'f' for none."
which please feel free to use if you want, but if you want to come up
with your own wording, that's great too.
The InsertOneNull() function used in bootstrap would not test values for
nullness in presence of invalid constraints. This change is mostly
pro-forma, since we don't expect invalid constraints during bootstrap,
but it seemed better to be tidy.
I have not looked at the pg_dump code yet, so the changes included here
are just pgindent.
Thank you!
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
>From ac8e690c64d354faad421a48f5adf0e26a15202b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@alvh.no-ip.org>
Date: Thu, 20 Feb 2025 13:05:35 +0100
Subject: [PATCH] fixup
---
src/backend/bootstrap/bootstrap.c | 4 +-
src/backend/catalog/pg_constraint.c | 17 +-
src/backend/commands/tablecmds.c | 229 ++++++++++------------
src/bin/pg_dump/pg_dump.c | 15 +-
src/bin/pg_dump/pg_dump.h | 2 +-
src/test/regress/expected/constraints.out | 10 +-
src/test/regress/sql/constraints.sql | 8 +-
7 files changed, 131 insertions(+), 154 deletions(-)
diff --git a/src/backend/bootstrap/bootstrap.c
b/src/backend/bootstrap/bootstrap.c
index 1e95dc32f46..919972dc409 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -582,10 +582,8 @@ DefineAttr(char *name, char *type, int attnum, int
nullness)
attrtypes[attnum]->atttypmod = -1;
attrtypes[attnum]->attislocal = true;
- /* set default to false */
attrtypes[attnum]->attnotnull = ATTRIBUTE_NOTNULL_FALSE;
-
if (nullness == BOOTCOL_NULL_FORCE_NOT_NULL)
{
attrtypes[attnum]->attnotnull = ATTRIBUTE_NOTNULL_TRUE;
@@ -699,7 +697,7 @@ InsertOneNull(int i)
{
elog(DEBUG4, "inserting column %d NULL", i);
Assert(i >= 0 && i < MAXATTR);
- if (TupleDescAttr(boot_reldesc->rd_att, i)->attnotnull ==
ATTRIBUTE_NOTNULL_TRUE)
+ if (TupleDescAttr(boot_reldesc->rd_att, i)->attnotnull !=
ATTRIBUTE_NOTNULL_FALSE)
elog(ERROR,
"NULL value specified for not-null column \"%s\" of
relation \"%s\"",
NameStr(TupleDescAttr(boot_reldesc->rd_att,
i)->attname),
diff --git a/src/backend/catalog/pg_constraint.c
b/src/backend/catalog/pg_constraint.c
index ac80652baf2..f856f387502 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -574,7 +574,7 @@ ChooseConstraintName(const char *name1, const char *name2,
/*
* Find and return a copy of the pg_constraint tuple that implements a
- * validated not-null constraint for the given column of the given relation.
+ * not-null constraint for the given column of the given relation.
* If no such constraint exists, return NULL.
*
* XXX This would be easier if we had pg_attribute.notnullconstr with the OID
@@ -604,13 +604,11 @@ findNotNullConstraintAttnum(Oid relid, AttrNumber attnum)
AttrNumber conkey;
/*
- * We're looking for a NOTNULL constraint that's marked
validated,
- * with the column we're looking for as the sole element in
conkey.
+ * We're looking for a NOTNULL constraint with the column we're
+ * looking for as the sole element in conkey.
*/
if (con->contype != CONSTRAINT_NOTNULL)
continue;
- if (!con->convalidated)
- continue;
conkey = extractNotNullColumn(conTup);
if (conkey != attnum)
@@ -628,9 +626,9 @@ findNotNullConstraintAttnum(Oid relid, AttrNumber attnum)
}
/*
- * Find and return the pg_constraint tuple that implements a validated
- * not-null constraint for the given column of the given relation. If
- * no such column or no such constraint exists, return NULL.
+ * Find and return the pg_constraint tuple that implements a
+ * not-null constraint for the given column of the given relation.
+ * If no such column or no such constraint exists, return NULL.
*/
HeapTuple
findNotNullConstraint(Oid relid, const char *colname)
@@ -743,6 +741,9 @@ AdjustNotNullInheritance(Oid relid, AttrNumber attnum,
pg_constraint = table_open(ConstraintRelationId,
RowExclusiveLock);
conform = (Form_pg_constraint) GETSTRUCT(tup);
+ if (!conform->convalidated)
+ elog(WARNING, "got an unvalidated constraint");
+
/*
* If the NO INHERIT flag we're asked for doesn't match what the
* existing constraint has, throw an error.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1edda86fafd..793b81fc6c8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -406,7 +406,7 @@ static void QueueCheckConstraintValidation(List **wqueue,
Relation conrel, Relat
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueNNConstraintValidation(List **wqueue, Relation conrel,
Relation rel,
-
char *constrName, HeapTuple contuple,
+
HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
static int transformColumnNameList(Oid relId, List *colList,
int16
*attnums, Oid *atttypids, Oid *attcollids);
@@ -471,7 +471,8 @@ static void add_column_collation_dependency(Oid relid,
int32 attnum, Oid collid)
static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, bool
recurse,
LOCKMODE lockmode);
static void set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum,
- bool skip_validation,
LOCKMODE lockmode);
+ char newvalue, bool
queue_validation,
+ LOCKMODE lockmode);
static ObjectAddress ATExecSetNotNull(List **wqueue, Relation rel,
char
*constrname, char *colName,
bool
recurse, bool recursing,
@@ -701,7 +702,6 @@ static void ATDetachCheckNoForeignKeyRefs(Relation
partition);
static char GetAttributeCompression(Oid atttypid, const char *compression);
static char GetAttributeStorage(Oid atttypid, const char *storagemode);
static bool check_for_invalid_notnull(Oid relid, const char *attname);
-static char *getNNConnameForAttnum(Oid relid, AttrNumber attnum);
/* ----------------------------------------------------------------
@@ -1319,7 +1319,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid
ownerId,
nncols = AddRelationNotNullConstraints(rel, stmt->nnconstraints,
old_notnulls);
foreach_int(attrnum, nncols)
- set_attnotnull(NULL, rel, attrnum, false, NoLock);
+ set_attnotnull(NULL, rel, attrnum, ATTRIBUTE_NOTNULL_TRUE,
+ false, NoLock);
ObjectAddressSet(address, RelationRelationId, relationId);
@@ -7717,10 +7718,13 @@ ATExecDropNotNull(Relation rel, const char *colName,
bool recurse,
*/
static void
set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum,
- bool skip_validation, LOCKMODE lockmode)
+ char newvalue, bool queue_validation, LOCKMODE
lockmode)
{
Form_pg_attribute attr;
+
+ Assert(!queue_validation || wqueue != NULL);
+
CheckAlterTableIsSafe(rel);
/*
@@ -7731,7 +7735,7 @@ set_attnotnull(List **wqueue, Relation rel, AttrNumber
attnum,
if (attr->attisdropped)
return;
- if (attr->attnotnull == ATTRIBUTE_NOTNULL_FALSE)
+ if (attr->attnotnull != newvalue)
{
Relation attr_rel;
HeapTuple tuple;
@@ -7744,20 +7748,17 @@ set_attnotnull(List **wqueue, Relation rel, AttrNumber
attnum,
attnum, RelationGetRelid(rel));
attr = (Form_pg_attribute) GETSTRUCT(tuple);
- Assert(attr->attnotnull == ATTRIBUTE_NOTNULL_FALSE);
+ attr->attnotnull = newvalue;
- if (skip_validation)
- attr->attnotnull = ATTRIBUTE_NOTNULL_INVALID;
- else
- attr->attnotnull = ATTRIBUTE_NOTNULL_TRUE;
CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
/*
- * If the nullness isn't already proven by validated
constraints, have
- * ALTER TABLE phase 3 test for it.
+ * Queue later validation of this constraint, if necessary and
+ * requested by caller.
*/
- if (!skip_validation &&
- wqueue && !NotNullImpliedByRelConstraints(rel, attr))
+ if (queue_validation &&
+ newvalue == ATTRIBUTE_NOTNULL_TRUE &&
+ !NotNullImpliedByRelConstraints(rel, attr))
{
AlteredTableInfo *tab;
@@ -7844,6 +7845,9 @@ ATExecSetNotNull(List **wqueue, Relation rel, char
*conName, char *colName,
NameStr(conForm->conname),
RelationGetRelationName(rel)));
+ if (!conForm->convalidated)
+ elog(WARNING, "trying to add a constraint where an
invalid one already exists");
+
/*
* If we find an appropriate constraint, we're almost done, but
just
* need to change some properties on it: if we're recursing,
increment
@@ -7925,8 +7929,12 @@ ATExecSetNotNull(List **wqueue, Relation rel, char
*conName, char *colName,
InvokeObjectPostAlterHook(RelationRelationId,
RelationGetRelid(rel), attnum);
- /* Mark pg_attribute.attnotnull for the column */
- set_attnotnull(wqueue, rel, attnum, constraint->skip_validation,
lockmode);
+ /* Mark pg_attribute.attnotnull for the column and request validation */
+ set_attnotnull(wqueue, rel, attnum,
+ constraint->skip_validation ?
+ ATTRIBUTE_NOTNULL_INVALID :
+ ATTRIBUTE_NOTNULL_TRUE,
+ !constraint->skip_validation, lockmode);
/*
* Recurse to propagate the constraint to children that don't have one.
@@ -9368,22 +9376,19 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel,
AlterTableCmd *cmd,
}
/* Insert not-null constraints in the queue for the PK columns */
- foreach(lc, pkconstr->keys)
+ foreach_node(String, colname, pkconstr->keys)
{
AlterTableCmd *newcmd;
Constraint *nnconstr;
- String *colname = lfirst(lc);
- /*
- * Throw an error if relation key column has invalid not null
- * constraint.
- */
- if (check_for_invalid_notnull(RelationGetRelid(rel),
colname->sval))
+ /* Verify that the not-null constraint has been validated */
+ if (check_for_invalid_notnull(RelationGetRelid(rel),
strVal(colname)))
ereport(ERROR,
- errmsg("column \"%s\" of table \"%s\"
is marked as NOT VALID NOT NULL constrint",
- colname->sval,
RelationGetRelationName(rel)));
+
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("column \"%s\" of table \"%s\"
is marked as NOT VALID NOT NULL constraint",
+ strVal(colname),
RelationGetRelationName(rel)));
- nnconstr = makeNotNullConstraint(lfirst(lc));
+ nnconstr = makeNotNullConstraint(colname);
newcmd = makeNode(AlterTableCmd);
newcmd->subtype = AT_AddConstraint;
@@ -9785,7 +9790,12 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo
*tab, Relation rel,
* phase 3 to verify existing rows, if needed.
*/
if (constr->contype == CONSTR_NOTNULL)
- set_attnotnull(wqueue, rel, ccon->attnum,
ccon->skip_validation, lockmode);
+ set_attnotnull(wqueue, rel, ccon->attnum,
+ ccon->skip_validation ?
+ ATTRIBUTE_NOTNULL_INVALID :
+ ATTRIBUTE_NOTNULL_TRUE,
+ !ccon->skip_validation,
+ lockmode);
ObjectAddressSet(address, ConstraintRelationId, ccon->conoid);
}
@@ -12203,7 +12213,7 @@ ATExecValidateConstraint(List **wqueue, Relation rel,
char *constrName,
con->contype != CONSTRAINT_NOTNULL)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("constraint \"%s\" of relation \"%s\"
is not a foreign key or check constraint",
+ errmsg("constraint \"%s\" of relation \"%s\"
is not a foreign key, not-null, or check constraint",
constrName,
RelationGetRelationName(rel))));
if (!con->conenforced)
@@ -12224,7 +12234,7 @@ ATExecValidateConstraint(List **wqueue, Relation rel,
char *constrName,
}
else if (con->contype == CONSTRAINT_NOTNULL)
{
- QueueNNConstraintValidation(wqueue, conrel, rel,
constrName,
+ QueueNNConstraintValidation(wqueue, conrel, rel,
tuple, recurse, recursing, lockmode);
}
@@ -12356,9 +12366,7 @@ QueueCheckConstraintValidation(List **wqueue, Relation
conrel, Relation rel,
AlteredTableInfo *tab;
HeapTuple copyTuple;
Form_pg_constraint copy_con;
-
List *children = NIL;
- ListCell *child;
NewConstraint *newcon;
Datum val;
char *conbin;
@@ -12367,24 +12375,19 @@ QueueCheckConstraintValidation(List **wqueue,
Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_CHECK);
/*
- * If we're recursing, the parent has already done this, so skip it.
Also,
- * if the constraint is a NO INHERIT constraint, we shouldn't try to
look
- * for it in the children.
- */
- if (!recursing && !con->connoinherit)
- children = find_all_inheritors(RelationGetRelid(rel),
-
lockmode, NULL);
-
- /*
- * For CHECK constraints, we must ensure that we only mark the
constraint
- * as validated on the parent if it's already validated on the children.
+ * For constraints that aren't NO INHERIT, we must ensure that we only
+ * mark the constraint as validated on the parent if it's already
+ * validated on the children.
+ *
+ * If we're recursing, the parent has already done this, so skip it.
*
* We recurse before validating on the parent, to reduce risk of
* deadlocks.
*/
- foreach(child, children)
+ if (!recursing && !con->connoinherit)
+ children = find_all_inheritors(RelationGetRelid(rel), lockmode,
NULL);
+ foreach_oid(childoid, children)
{
- Oid childoid = lfirst_oid(child);
Relation childrel;
if (childoid == RelationGetRelid(rel))
@@ -12446,23 +12449,22 @@ QueueCheckConstraintValidation(List **wqueue,
Relation conrel, Relation rel,
/*
* QueueNNConstraintValidation
*
- * Add an entry to the wqueue to validate the given notnull constraint in
Phase 3
- * and update the convalidated field in the pg_constraint catalog for the
- * specified relation and all its inheriting children.
+ * Add an entry to the wqueue to validate the given not-null constraint in
+ * Phase 3 and update the convalidated field in the pg_constraint catalog for
+ * the specified relation and all its inheriting children.
*/
static void
QueueNNConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- char *constrName, HeapTuple
contuple,
- bool recurse, bool recursing,
LOCKMODE lockmode)
+ HeapTuple contuple,
bool recurse, bool recursing,
+ LOCKMODE lockmode)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
HeapTuple copyTuple;
Form_pg_constraint copy_con;
-
List *children = NIL;
- ListCell *child;
- AttrNumber attnum;
+ AttrNumber attnum;
+ char *colname;
con = (Form_pg_constraint) GETSTRUCT(contuple);
Assert(con->contype == CONSTRAINT_NOTNULL);
@@ -12470,25 +12472,24 @@ QueueNNConstraintValidation(List **wqueue, Relation
conrel, Relation rel,
attnum = extractNotNullColumn(contuple);
/*
- * If we're recursing, the parent has already done this, so skip it.
Also,
- * if the constraint is a NO INHERIT constraint, we shouldn't try to
look
- * for it in the children.
- */
- if (!recursing && !con->connoinherit)
- children = find_all_inheritors(RelationGetRelid(rel),
-
lockmode, NULL);
-
- /*
- * For CHECK constraints, we must ensure that we only mark the
constraint
- * as validated on the parent if it's already validated on the children.
+ * For constraints that aren't NO INHERIT, we must ensure that we only
+ * mark the constraint as validated on the parent if it's already
+ * validated on the children.
+ *
+ * If we're recursing, the parent has already done this, so skip it.
*
* We recurse before validating on the parent, to reduce risk of
* deadlocks.
*/
- foreach(child, children)
+ if (!recursing && !con->connoinherit)
+ children = find_all_inheritors(RelationGetRelid(rel), lockmode,
NULL);
+
+ colname = get_attname(RelationGetRelid(rel), attnum, false);
+ foreach_oid(childoid, children)
{
- Oid childoid = lfirst_oid(child);
Relation childrel;
+ HeapTuple contup;
+ Form_pg_constraint childcon;
char *conname;
if (childoid == RelationGetRelid(rel))
@@ -12501,21 +12502,31 @@ QueueNNConstraintValidation(List **wqueue, Relation
conrel, Relation rel,
*/
if (!recurse)
ereport(ERROR,
-
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("constraint must be validated
on child tables too")));
+
errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("constraint must be validated on
child tables too"));
+
+ /*
+ * The column on child might have a different attnum, so search
by
+ * column name.
+ */
+ contup = findNotNullConstraint(childoid, colname);
+ if (!contup)
+ elog(ERROR, "cache lookup failed for not-null
constraint on column \"%s\" of relation \"%s\"",
+ colname, get_rel_name(childoid));
+ childcon = (Form_pg_constraint) GETSTRUCT(contup);
+ if (childcon->convalidated)
+ continue;
/* find_all_inheritors already got lock */
childrel = table_open(childoid, NoLock);
- conname = getNNConnameForAttnum(childoid, attnum);
- if (conname == NULL)
- continue;
+ conname = pstrdup(NameStr(childcon->conname));
+ /* XXX improve ATExecValidateConstraint API to avoid double
search */
ATExecValidateConstraint(wqueue, childrel, conname,
false, true,
lockmode);
table_close(childrel, NoLock);
}
-
tab = ATGetQueueEntry(wqueue, rel);
tab->verify_new_notnull = true;
@@ -12525,64 +12536,22 @@ QueueNNConstraintValidation(List **wqueue, Relation
conrel, Relation rel,
CacheInvalidateRelcache(rel);
/*
- * Now update the catalog, while we have the door open.
+ * Now update the catalogs, while we have the door open.
*/
copyTuple = heap_copytuple(contuple);
copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
copy_con->convalidated = true;
CatalogTupleUpdate(conrel, ©Tuple->t_self, copyTuple);
+ /* Also flip attnotnull */
+ set_attnotnull(wqueue, rel, attnum, ATTRIBUTE_NOTNULL_TRUE, false,
+ lockmode);
+
InvokeObjectPostAlterHook(ConstraintRelationId, con->oid, 0);
heap_freetuple(copyTuple);
}
-/*
- * Function returns the invalid not null constrint name for the given
- * relation and attnumber.
- */
-static char *
-getNNConnameForAttnum(Oid relid, AttrNumber attnum)
-{
- Relation constrRel;
- HeapTuple htup;
- SysScanDesc conscan;
- ScanKeyData skey;
- char *conname = NULL;
-
- constrRel = table_open(ConstraintRelationId, AccessShareLock);
- ScanKeyInit(&skey,
- Anum_pg_constraint_conrelid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(relid));
- conscan = systable_beginscan(constrRel,
ConstraintRelidTypidNameIndexId, true,
- NULL, 1,
&skey);
-
- while (HeapTupleIsValid(htup = systable_getnext(conscan)))
- {
- Form_pg_constraint conForm = (Form_pg_constraint)
GETSTRUCT(htup);
- AttrNumber colnum;
-
- if (conForm->contype != CONSTRAINT_NOTNULL)
- continue;
- if (conForm->convalidated == true)
- continue;
-
- colnum = extractNotNullColumn(htup);
-
- if (colnum != attnum)
- continue;
-
- conname = pstrdup(NameStr(conForm->conname));
- break;
- }
-
- systable_endscan(conscan);
- table_close(constrRel, AccessShareLock);
-
- return conname;
-}
-
/*
* transformColumnNameList - transform list of column names
*
@@ -13454,7 +13423,7 @@ dropconstraint_internal(Relation rel, HeapTuple
constraintTup, DropBehavior beha
RelationGetRelationName(rel)));
/* All good -- reset attnotnull if needed */
- if (attForm->attnotnull == ATTRIBUTE_NOTNULL_TRUE)
+ if (attForm->attnotnull != ATTRIBUTE_NOTNULL_FALSE)
{
attForm->attnotnull = ATTRIBUTE_NOTNULL_FALSE;
CatalogTupleUpdate(attrel, &atttup->t_self, atttup);
@@ -16790,12 +16759,20 @@ MergeAttributesIntoExisting(Relation child_rel,
Relation parent_rel, bool ispart
contup =
findNotNullConstraintAttnum(RelationGetRelid(parent_rel),
parent_att->attnum);
- if (HeapTupleIsValid(contup) &&
- !((Form_pg_constraint)
GETSTRUCT(contup))->connoinherit)
- ereport(ERROR,
-
errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("column \"%s\"
in child table \"%s\" must be marked NOT NULL",
-
parent_attname, RelationGetRelationName(child_rel)));
+ if (HeapTupleIsValid(contup))
+ {
+ Form_pg_constraint childcon;
+
+ childcon = (Form_pg_constraint)
GETSTRUCT(contup);
+ if (!childcon->connoinherit)
+ ereport(ERROR,
+
errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("column
\"%s\" in child table \"%s\" must be marked NOT NULL",
+
parent_attname, RelationGetRelationName(child_rel)));
+
+ if (!childcon->convalidated)
+ elog(WARNING, "found an invalid
constraint");
+ }
}
/*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c63b4504cee..7f0c39db88d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -9124,7 +9124,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int
numTables)
tbinfo->notnull_valid[j] = (PQgetvalue(res, r,
i_notnull_valid)[0] == 't');
/*
- * Dump the invalid NOT NULL constrint like the Check
constraints
+ * Dump the invalid NOT NULL constraint like the Check
constraints
*/
if (tbinfo->notnull_valid[j])
/* Handle not-null constraint name and flags */
@@ -9132,13 +9132,14 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int
numTables)
tbinfo, j,
i_notnull_name, i_notnull_noinherit,
i_notnull_islocal);
- else if(!PQgetisnull(res, r, i_notnull_name))
+ else if (!PQgetisnull(res, r, i_notnull_name))
{
/*
- * Add the entry into invalidnotnull list so
NOT NULL constraint get
- * dump as separate constrints.
+ * Add the entry into invalidnotnull list so
NOT NULL
+ * constraint get dump as separate constraints.
*/
- if (invalidnotnulloids->len > 1) /* do we have
more than the '{'? */
+ if (invalidnotnulloids->len > 1) /* do
we have more than
+
* the '{'? */
appendPQExpBufferChar(invalidnotnulloids, ',');
appendPQExpBuffer(invalidnotnulloids, "%u",
tbinfo->dobj.catId.oid);
@@ -9429,8 +9430,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int
numTables)
}
/*
- * Get info about table INVALID NOT NULL constraints. This is skipped
for a
- * data-only dump, as it is only needed for table schemas.
+ * Get info about table INVALID NOT NULL constraints. This is skipped
for
+ * a data-only dump, as it is only needed for table schemas.
*/
if (dopt->dumpSchema && invalidnotnulloids->len > 2)
{
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 25d65b357a4..a393d993330 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -361,7 +361,7 @@ typedef struct _tableInfo
char *attcompression; /* per-attribute compression method */
char **attfdwoptions; /* per-attribute fdw options */
char **attmissingval; /* per attribute missing value */
- bool *notnull_valid; /* NOT NULL status */
+ bool *notnull_valid; /* NOT NULL status */
char **notnull_constrs; /* NOT NULL constraint names. If null,
*
there isn't one on this column. If
*
empty string, unnamed constraint
diff --git a/src/test/regress/expected/constraints.out
b/src/test/regress/expected/constraints.out
index a16422b681b..341956f1cea 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -941,7 +941,7 @@ SELECT * FROM notnull_tbl1;
-- Try to add primary key on table column marked as NOT VALID NOT NULL
-- constraint. This should throw an error.
ALTER TABLE notnull_tbl1 add primary key (a);
-ERROR: column "a" of table "notnull_tbl1" is marked as NOT VALID NOT NULL
constrint
+ERROR: column "a" of table "notnull_tbl1" is marked as NOT VALID NOT NULL
constraint
-- INHERITS table having NOT VALID NOT NULL constraints.
CREATE TABLE notnull_tbl1_child(a INTEGER, b INTEGER) INHERITS(notnull_tbl1);
NOTICE: merging column "a" with inherited definition
@@ -965,13 +965,13 @@ Not-null constraints:
"nn" NOT NULL "a"
DROP TABLE notnull_tbl1;
--- Test the different Not null constrint name for parent and child table
+-- Test the different Not null constraint name for parent and child table
CREATE TABLE notnull_tbl1 (a int);
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn_parent not null a not valid;
CREATE TABLE notnull_chld (a int);
ALTER TABLE notnull_chld ADD CONSTRAINT nn_child not null a not valid;
ALTER TABLE notnull_chld INHERIT notnull_tbl1;
--- This statement should validate not null constrint for parent as well as
+-- This statement should validate not null constraint for parent as well as
-- child.
ALTER TABLE notnull_tbl1 VALIDATE CONSTRAINT nn_parent;
SELECT conname, convalidated FROM pg_catalog.pg_constraint WHERE conrelid
@@ -984,7 +984,7 @@ in ('notnull_tbl1'::regclass, 'notnull_chld'::regclass);
DROP TABLE notnull_tbl1 CASCADE;
NOTICE: drop cascades to table notnull_chld
---Create table with NOT NULL INVALID constrint, for pg_upgrade.
+-- Create table with NOT NULL INVALID constraint, for pg_upgrade.
CREATE TABLE notnull_tbl1_upg (a INTEGER, b INTEGER);
INSERT INTO notnull_tbl1_upg VALUES (NULL, 1);
INSERT INTO notnull_tbl1_upg VALUES (NULL, 2);
@@ -995,7 +995,7 @@ CREATE TABLE notnull_tbl1 (a INTEGER, b INTEGER) PARTITION
BY RANGE (a);
ALTER TABLE notnull_tbl1 ADD CONSTRAINT notnull_con NOT NULL a NOT VALID;
CREATE TABLE notnull_tbl1_1 PARTITION OF notnull_tbl1 FOR VALUES FROM (0) TO
(10);
CREATE TABLE notnull_tbl1_2 PARTITION OF notnull_tbl1 FOR VALUES FROM (20) TO
(30);
--- Parent table NOT NULL constraints will be market as validated false, where
+-- Parent table NOT NULL constraints will be marked as validated false, where
-- for child table it will be true
SELECT conrelid::regclass, conname, convalidated FROM pg_catalog.pg_constraint
WHERE
conrelid IN ('notnull_tbl1'::regclass, 'notnull_tbl1_1'::regclass,
'notnull_tbl1_2'::regclass);
diff --git a/src/test/regress/sql/constraints.sql
b/src/test/regress/sql/constraints.sql
index b62d8c69ff4..6dd7e4c79c7 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -670,19 +670,19 @@ DROP TABLE notnull_tbl1_child;
ALTER TABLE notnull_tbl1 validate constraint nn;
\d+ notnull_tbl1
DROP TABLE notnull_tbl1;
--- Test the different Not null constrint name for parent and child table
+-- Test the different Not null constraint name for parent and child table
CREATE TABLE notnull_tbl1 (a int);
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn_parent not null a not valid;
CREATE TABLE notnull_chld (a int);
ALTER TABLE notnull_chld ADD CONSTRAINT nn_child not null a not valid;
ALTER TABLE notnull_chld INHERIT notnull_tbl1;
--- This statement should validate not null constrint for parent as well as
+-- This statement should validate not null constraint for parent as well as
-- child.
ALTER TABLE notnull_tbl1 VALIDATE CONSTRAINT nn_parent;
SELECT conname, convalidated FROM pg_catalog.pg_constraint WHERE conrelid
in ('notnull_tbl1'::regclass, 'notnull_chld'::regclass);
DROP TABLE notnull_tbl1 CASCADE;
---Create table with NOT NULL INVALID constrint, for pg_upgrade.
+-- Create table with NOT NULL INVALID constraint, for pg_upgrade.
CREATE TABLE notnull_tbl1_upg (a INTEGER, b INTEGER);
INSERT INTO notnull_tbl1_upg VALUES (NULL, 1);
INSERT INTO notnull_tbl1_upg VALUES (NULL, 2);
@@ -694,7 +694,7 @@ CREATE TABLE notnull_tbl1 (a INTEGER, b INTEGER) PARTITION
BY RANGE (a);
ALTER TABLE notnull_tbl1 ADD CONSTRAINT notnull_con NOT NULL a NOT VALID;
CREATE TABLE notnull_tbl1_1 PARTITION OF notnull_tbl1 FOR VALUES FROM (0) TO
(10);
CREATE TABLE notnull_tbl1_2 PARTITION OF notnull_tbl1 FOR VALUES FROM (20) TO
(30);
--- Parent table NOT NULL constraints will be market as validated false, where
+-- Parent table NOT NULL constraints will be marked as validated false, where
-- for child table it will be true
SELECT conrelid::regclass, conname, convalidated FROM pg_catalog.pg_constraint
WHERE
conrelid IN ('notnull_tbl1'::regclass, 'notnull_tbl1_1'::regclass,
'notnull_tbl1_2'::regclass);
--
2.39.5
