Re: Foreign key validation failure in 18beta1

From: Amul Sul <sulamul(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, Tender Wang <tndrwang(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Antonin Houska <ah(at)cybertec(dot)at>
Subject: Re: Foreign key validation failure in 18beta1
Date: 2025-06-03 04:14:18
Message-ID: CAAJ_b94wohCCU8-HDbHs6THkQ_ze6FiUYkFzvkNWD_qFC7B1Gw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 2, 2025 at 9:56 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> Hello
>
> I find this coding somewhat confusing. Now you have a function
> "QueueFkConstraintValidation" which may queue a FK queue constraint
> validation, if the flag "queueValidation" is given, but it may also do
> something else -- makes no sense IMO.

I wouldn’t disagree with that.

> I think it's better to split this
> function in two; one does indeed validate, the other is a simple helper
> that only sets the catalog flag, and does its own recursion. This
> causes a bit of duplicate code, but is simpler to follow. (We could
> also refactor the duplicate code by adding another helper routine, but
> I'm not sure it's worth the trouble.)
>
> I would propose something like this instead, what do you think?
>
>

I found a third approach that requires only a few changes. The key
idea is to determine the root referenced table and pass it to
QueueFKConstraintValidation(). We would then enqueue phase 3
validation only if the constraint tuple’s confrelid matches that root
table -- similar to what is doing in ATExecAlterConstrEnforceability().

This would also ensure that the logic for adding/skipping phase 3
validation is consistent in both places.

Adding an extra parameter (i.e., pkrelid) doesn’t seem out of place,
as it aligns with other functions in the same file, such as
ATExecAlterConstraintInternal() and ATExecAlterConstrEnforceability().

Kindly take a look at the attached version and share your thoughts.

Regards,
Amul

Attachment Content-Type Size
v7-0001-Skip-adding-action-based-foreign-key-constraints-.patch application/x-patch 12.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Japin Li 2025-06-03 04:16:21 Encapsulate io_uring process count calculation
Previous Message Michael Paquier 2025-06-03 04:13:06 Re: Persist injection points across server restarts