Hi Alvaro, As discussed, I have added a few more tests related to pg_dump and pg_upgrade. Specifically:
- Added INHERIT and PARTITION test cases to constraints.sql, keeping the
objects unchanged so they can be used in the pg_upgrade test
- Included additional tests in 002_pg_dump.pl for pg_dump-specific
scenarios.
Please find attached the updated patch based on the V6 patch.
Thanks,
On Wed, Apr 2, 2025 at 1:52 AM Alvaro Herrera <alvhe...@alvh.no-ip.org>
wrote:
> On 2025-Mar-28, jian he wrote:
>
> > 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 you're right that this isn't saying what the problem is; we
> should be saying something like
>
> ERROR: cannot add primary key because of invalid not-null constraint
> "the_constr"
> HINT: You will need to use ALTER TABLE .. VALIDATE CONSTRAINT to validate
> it.
>
> > ------------------------------------------------------------------------
> > 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');
>
> It's still not clear to me what to do to fix this problem. But while
> trying to understand it, I had the chance to rework the pg_dump code
> somewhat, so here it is. Feel free to propose fixes on top of this.
> (BTW, I think the business of assigning to tbinfo->checkexprs both the
> block for check constraints and the one for not-null constraints is
> bogus. I didn't find what this breaks, but it looks wrong. We probably
> need another struct _constraintInfo pointer in TableInfo.)
>
> --
> Álvaro Herrera 48°01'N 7°57'E —
> https://www.EnterpriseDB.com/
> "¿Cómo puedes confiar en algo que pagas y que no ves,
> y no confiar en algo que te dan y te lo muestran?" (Germán Poo)
>
Thanks
Rushabh Lathia
v6-0001-NOT-NULL-NOT-VALID.patch
Description: Binary data
not_valid_not_null_testcase.patch
Description: Binary data
