Subject: Support NOT VALID / VALIDATE constraint options for named NOT NULL constraints

From: Rushabh Lathia <[email-masked]>
Date: Thu, 6 Feb 2025 18:08:46 +0530

Hi,

Commit 14e87ffa5c543b5f30ead7413084c25f7735039f
<https://github.com/postgres/postgres/commit/14e87ffa5c543b5f30ead7413084c25f7735039f>
added
the support for named NOT NULL constraints.   We can now support the NOT
VALID/VALID named NOT NULL constraints.

This patch supports the NOT VALID and VALIDATE CONSTRAINT for name NOT NULL
constraints.  In order to achieve this patch,

1) Converted the pg_attribute.attnotnull to CHAR type, so that it can hold
the INVALID flag for the constraint.
2) Added the support for NOT VALID as well as VALIDATE CONSTRAINT support.
3) Support for pg_dump, where we now dumping the INVALID NOT NULL as
separate from table schemes, just like CHECK Constraints.
4) Added related testcases.

Attaching the patch here.

Thanks Alvaro for your offline help and support for this feature.

Thanks
Rushabh Lathia
www.EnterpriseDB.com

---

From: Alvaro Herrera <[email-masked]>
Date: Thu, 6 Feb 2025 17:28:43 +0100

Hello Rushabh,

On 2025-Feb-06, Rushabh Lathia wrote:

> added the support for named NOT NULL constraints.   We can now support
> the NOT VALID/VALID named NOT NULL constraints.
> 
> This patch supports the NOT VALID and VALIDATE CONSTRAINT for name NOT
> NULL constraints.  In order to achieve this patch,

Thank you very much for working on this.

> 1) Converted the pg_attribute.attnotnull to CHAR type, so that it can
> hold the INVALID flag for the constraint.

This looks good to me.  It'll have implications for client-side queries,
but I think they will need to adapt.  One school of thought says we
should rename the column, so that every tool is forced to think about
the issue and adapt accordingly, instead of only realizing the problem
the first time they break.

> 4) Added related testcases.

Please remember to add test cases for tables with not-valid constraint
that are not dropped at the end.  That way, the pg_upgrade test will try
to process that table and we'll know if the roundtrip via pg_dump works
correctly.

I haven't looked at 0002 too closely, but I think it has the right
shape.

> separate from table schemes, just like CHECK Constraints.

I think you copied a little bit too much of the code for check
constraints.  If a constraint is accumulated in invalidnotnulloids, you
already know that it's not validated and needs to be dumped separately.
So your new query doesn't need to bring convalidated (we know it's
false).  This would simplify a few lines in this new code.  Also, the
pg_log_info() line is mistaken about what this block is doing.

I think it'd be good to have NOT VALID NO INHERIT constraints in the
tests as well.  Also consider the case where the child table is created
first with a valid constraint, then the parent table is created later
with a not valid constraint -- if the pg_dump table scans find the child
first, does pg_dump do the right thing or does it try to create the
parent constraint first?  Also, what if the constraint in the child has
a different name from the constraint in the parent?  This should be
pg_dump round-tripped as well.  (I bet there are tons of other corner
cases here that should be verified.)  Please add something to
pg_dump/t/002_pg_dump.pl.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

---

From: Alvaro Herrera <[email-masked]>
Date: Fri, 7 Feb 2025 12:14:15 +0100

Recursion to child tables is incorrectly trying to locate the constraint
by name:

create table notnull_tbl1 (a int);
alter table notnull_tbl1 add constraint foo not null a not valid;
create table notnull_chld (a int);
alter table notnull_chld add constraint blah not null a not valid;
alter table notnull_chld inherit notnull_tbl1 ;

-- this fails but shouldn't:
alter table notnull_tbl1 validate constraint foo;
ERROR:  constraint "foo" of relation "notnull_chld" does not exist

The end result here should be that the constraint `blah` in table
notnull_chld is marked as validated.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Syntax error: function hell() needs an argument.
Please choose what hell you want to involve.

---

From: Rushabh Lathia <[email-masked]>
Date: Sun, 9 Feb 2025 17:55:23 +0530

Thanks Alvaro.

On Thu, Feb 6, 2025 at 9:58 PM Alvaro Herrera <[email-masked]>
wrote:

> > <
> https://github.com/postgres/postgres/commit/14e87ffa5c543b5f30ead7413084c25f7735039f
> >

I am open to suggestions here.

> to process that table and we'll know if the roundtrip via pg_dump works

Sure, will cover this.

> pg_log_info() line is mistaken about what this block is doing.

Even though we are accumulating invalidnotnulloids, we need the condition
for the invalid not null constraint, as there can me multiple not null
constraint
on the table - mix of valid and invalid.

Initially, I tried re-using the code but it was not very clear, so thought
of
making at separate code for check and not null constraint.  That way it's
very clear.

> tests as well.

Sure, will take care in the new version of the patch.

> Also consider the case where the child table is created
> parent constraint first?

yes, I tested this with the patch and pg_dump doing the right this
for that scenario.

> Also, what if the constraint in the child has

Okay, I will add tests to pg_dump/t/002_pg_dump.pl.

regards.
Rushabh Lathia
www.EnterpriseDB.com

---

From: Rushabh Lathia <[email-masked]>
Date: Mon, 10 Feb 2025 00:28:45 +0530

> notnull_chld is marked as validated.

Yes, I agree.  Here we need a separate Queue for NotNull constraint
validation,
which fetches the respective Non-Validate-Not-Null constraint name from
the child table

I am working on the patch and will post the update patch soon.

-- 
Rushabh Lathia

---

From: Rushabh Lathia <[email-masked]>
Date: Mon, 10 Feb 2025 22:48:57 +0530

Hi Alvaro,

I have incorporated the suggested changes, and here is the latest version
of the patch:

   - Added more test cases to the regression suite.
   - Included tests in the pg_dump test.
   - Left objects with *INVALID NOT NULL* for pg_upgrade.
   - Fixed an issue where recursion to child tables was incorrectly
   attempting to locate the constraint by name.
   - Introduced a new function, QueueNNConstraintValidation(), for
handling *NOT
   NULL* constraints.

The only remaining task for this patch is updating the documentation. I
will work on that and submit the final version soon.

Please share your review comments.
Thanks,

-- 
Rushabh Lathia
www.EnterpriseDB.com

---

From: Rushabh Lathia <[email-masked]>
Date: Wed, 12 Feb 2025 16:52:36 +0530

On Mon, Feb 10, 2025 at 10:48 PM Rushabh Lathia <[email-masked]>
wrote:

>    handling *NOT NULL* constraints.
>

Attaching documentation and tab-complete patch here.

-- 
Rushabh Lathia