Horton

Re: NOT ENFORCED constraint feature

jian he <jian.universality@gmail.com>

View thread
From: jian he <jian.universality@gmail.com>
To: Amul Sul <sulamul@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers <pgsql-hackers@postgresql.org>, Joel Jacobson <joel@compiler.org>
Date: 2024-12-11T12:42:26Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add support for NOT ENFORCED in foreign key constraints

  2. Expand test a bit

  3. refactor: Pass relation OID instead of Relation to createForeignKeyCheckTriggers()

  4. refactor: Split ATExecAlterConstraintInternal()

  5. refactor: Move some code that updates pg_constraint to a separate function

  6. Move RemoveInheritedConstraint() call slightly earlier

  7. refactor: Split tryAttachPartitionForeignKey()

  8. refactor: re-add ATExecAlterChildConstr()

  9. Add ATAlterConstraint struct for ALTER .. CONSTRAINT

  10. refactor: split ATExecAlterConstrRecurse()

  11. Add support for NOT ENFORCED in CHECK constraints

On Tue, Dec 10, 2024 at 7:48 PM Amul Sul <sulamul@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!

do you have a patch for
alter check constraint set [not] enforced?
If not, I will probably try to work on it.


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.