Re: Support NOT VALID / VALIDATE constraint options for named NOT NULL constraints - Mailing list pgsql-hackers

On Fri, Mar 28, 2025 at 3:25 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2025-Mar-24, jian he wrote:
>
> > hi.
> > you may like the attached. it's based on your idea: attnotnullvalid.
>
> This is quite close to what I was thinking, yeah.  I noticed a couple of
> bugs however, and ended up cleaning up the whole thing.  Here's what I
> have so far.  I'm not sure the pg_dump bits are okay (apart from what
> you report below) -- I think it's losing the constraint names, which is
> of course unacceptable.
>

hi.

        /*
         * Update constraint/default info has_not_null also include invalid
         * not-null constraint
         */
this comment needs a period. it should be:
        /*
         * Update constraint/default info. has_not_null also include invalid
         * not-null constraint
         */


gram.y:
                    /* no NOT VALID support yet */
                    processCASbits($4, @4, "NOT NULL",
                                   NULL, NULL, NULL, &n->skip_validation,
                                   &n->is_no_inherit, yyscanner);
                    n->initially_valid = !n->skip_validation;
                    $$ = (Node *) n;
comment  "/* no NOT VALID support yet */" should be removed?


get_relation_constraints:
- if (att->attnotnull && !att->attisdropped)
+ if (att->attnotnull /* && att->attnotnullvalid */ && !att->attisdropped)
looking at how we deal with check constraints,
i think we need
+ if (att->attnotnull && att->attnotnullvalid && !att->attisdropped)


set_attnotnull comments should say something about attnotnullvalid?
since it will change pg_attribute.attnotnullvalid


ATPrepAddPrimaryKey
+ if (!conForm->convalidated)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("not-null constraint \"%s\" of table \"%s\" has not been validated",
+   NameStr(conForm->conname),
+   RelationGetRelationName(rel)),
+ errhint("You will need to use ALTER TABLE ... VALIDATE CONSTRAINT to
validate it."));

I think the error message is less helpful.
Overall, I think we should say that:
to add the primary key on column x requires a validated not-null
constraint on column x.

------------------------------------------------------------------------
i think your patch messed up with pg_constraint.conislocal.
for example:

CREATE TABLE parted (id bigint default 1,id_abc bigint) PARTITION BY LIST (id);
alter TABLE parted add CONSTRAINT dummy_constr not null id not valid;
CREATE TABLE parted_1 (id bigint default 1,id_abc bigint);
alter TABLE parted_1 add CONSTRAINT dummy_constr not null id;
ALTER TABLE parted ATTACH PARTITION parted_1 FOR VALUES IN ('1');

select conrelid::regclass, conname, conislocal
from pg_constraint where conname = 'dummy_constr';

 conrelid |   conname    | conislocal
----------+--------------+------------
 parted   | dummy_constr | t
 parted_1 | dummy_constr | f
(2 rows)


if you  do pg_dump, and execute the pg_dump output
pg_dump  --no-statistics --clean --table-and-children=*parted*
--no-owner --verbose --column-inserts --file=dump.sql --no-acl

select conrelid::regclass, conname, conislocal
from pg_constraint where conname = 'dummy_constr';
output is

 conrelid |   conname    | conislocal
----------+--------------+------------
 parted   | dummy_constr | t
 parted_1 | dummy_constr | t
(2 rows)



because pg_dump will produce
CREATE TABLE public.parted (id bigint DEFAULT 1, id_abc bigint )
PARTITION BY LIST (id);
CREATE TABLE public.parted_1 ( id bigint DEFAULT 1 CONSTRAINT
dummy_constr NOT NULL, id_abc bigint);
ALTER TABLE public.parted ADD CONSTRAINT dummy_constr NOT NULL id NOT VALID;
the third sql command didn't override the parted_1 constraint
dummy_constr  conislocal value.
you may check my code change in AdjustNotNullInheritance