> 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,
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.
I am open to suggestions here.
> 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.
Sure, will cover this.
I haven't looked at 0002 too closely, but I think it has the right shape.
> 3) Support for pg_dump, where we now dumping the INVALID NOT NULL as > 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.
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.
I think it'd be good to have NOT VALID NO INHERIT constraints in the tests as well.
Sure, will take care in the new version of the patch.
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?
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 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.
