Horton

Re: Foreign key validation failure in 18beta1

jian he <jian.universality@gmail.com>

View thread
From: jian he <jian.universality@gmail.com>
To: Tender Wang <tndrwang@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>, Amul Sul <sulamul@gmail.com>
Date: 2025-05-29T07:08:07Z
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. Avoid bogus scans of partitions when marking FKs enforced

  2. Avoid bogus scans of partitions when validating FKs to partitioned tables

  3. Allow NOT VALID foreign key constraints on partitioned tables

Attachments

On Wed, May 28, 2025 at 8:38 PM Tender Wang <tndrwang@gmail.com> wrote:
>
>
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道：
>>
>> On 2025-May-28, Tender Wang wrote:
>>
>> > I dided the codes, in QueueFKConstraintValidation(),  we add three
>> > newconstraint for the
>> > fk rel, because the pk rel is partition table.
>> >
>> > During phase 3 of AlterTable, in ATRewriteTables(),
>> > call validateForeignKeyConstraint() three times.
>> > The first time the pk rel is pk, and it's ok.
>> > The second time the pk rel is only pk_1, and the type(1) is not in pk_1, so
>> > an error is reported.
>> >
>> > In this case, the two children newconstraint  should not be added to the
>> > queue.
>>
>> Yeah, I reached the same conclusion and this is the preliminary fix I
>> had written for it.  I don't like that I had to duplicate a few lines of
>> code, but maybe it's not too bad.  Also the comments need to be
>> clarified a bit more.
>
>
> If the child table is still a partitioned table, the patch seems not work.
>
> I figure out a quick fix as the attached. I add a bool argument into the QueueFKConstraintValidation().
> If it is true, it means we recursively call QueueFKConstraintValidation(), then we don't add the newconstraint to the  queue.
>
> I'm not sure about this fix. Any thoughts?

it will fail for the following case:

begin;
drop table if exists fk;
drop table if exists pk;
create table pk(i int primary key);
insert into pk values (0), (1);
create table fk(i int) partition by range (i);
create table fk_1 partition of fk for values from (0) to (1);
create table fk_2 partition of fk for values from (1) to (3);
insert into fk values (1),(2);
alter table fk add foreign key(i) references pk not valid;
commit;
alter table fk validate constraint fk_i_fkey; --error, should fail.

-----------
The attached *draft* patch is based on your idea.

The idea is that we only need to conditionally do
``tab->constraints = lappend(tab->constraints, newcon);`` within
QueueFKConstraintValidation.
but the catalog update needs to be done recursively.