On Wed, Dec 11, 2024 at 6:12 PM jian he <jian.universal...@gmail.com> wrote:
>
> On Tue, Dec 10, 2024 at 7:48 PM Amul Sul <sula...@gmail.com> wrote:
> >
> > >
> > > static bool
> > > MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
> > > bool allow_merge, bool is_local,
> > > + bool is_enforced,
> > > bool is_initially_valid,
> > > bool is_no_inherit)
> > > {
> > > @@ -2729,12 +2738,24 @@ MergeWithExistingConstraint(Relation rel,
> > > const char *ccname, Node *expr,
> > > * If the child constraint is "not valid" then cannot merge with a
> > > * valid parent constraint.
> > > */
> > > - if (is_initially_valid && !con->convalidated)
> > > + if (is_initially_valid && con->conenforced && !con->convalidated)
> > > ereport(ERROR,
> > > (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> > > errmsg("constraint \"%s\" conflicts with NOT VALID constraint on
> > > relation \"%s\"",
> > > ccname, RelationGetRelationName(rel))));
> > >
> > > There are no tests for this change. I think this change is not necessary.
> > >
> >
> > It is necessary; otherwise, it would raise an error for a NOT ENFORCED
> > constraint, which is NOT VALID by default.
> >
> got it.
> overall v8-0001 looks good to me!
>
Thank you.
> do you have a patch for
> alter check constraint set [not] enforced?
> If not, I will probably try to work on it.
>
Not yet; I believe I need to first look into allowing NOT VALID
foreign key constraints on partitioned tables.
>
> I am playing around with the remaining patch.
>
> ATExecAlterConstrRecurse
> ATExecAlterConstrEnforceability
> ATExecAlterChildConstr
> AlterConstrTriggerDeferrability
> These four functions take a lot of arguments.
> more comments about these arguments would be helpful.
> we only need to mention it at ATExecAlterConstrRecurse.
>
> for example:
> ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel,
> const Oid fkrelid, const Oid pkrelid,
> HeapTuple contuple, List **otherrelids,
> LOCKMODE lockmode, Oid ReferencedParentDelTrigger,
> Oid ReferencedParentUpdTrigger,
> Oid ReferencingParentInsTrigger,
> Oid ReferencingParentUpdTrigger)
> the comments only explained otherrelids.
>
> LOCKMODE lockmode,
> Oid ReferencedParentDelTrigger,
> Oid ReferencedParentUpdTrigger,
> Oid ReferencingParentInsTrigger,
> Oid ReferencingParentUpdTrigger
>
> The above arguments are pretty intuitive.
>
> Constraint *cmdcon
> Relation conrel
> Relation tgrel
> HeapTuple contuple
>
> but these arguments are not that very intuitive,
> especially these arguments passing to another function.
Those are the existing ones; let me think what can be done with them.
Regards,
Amul
