pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
5+ messages / 4 participants
[nested] [flat]

* Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
@ 2025-05-01 12:14  Alvaro Herrera <alvherre@alvh.no-ip.org>
  0 siblings, 1 reply; 5+ messages in thread

From: Alvaro Herrera @ 2025-05-01 12:14 UTC (permalink / raw)
  To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; fastcat@gmail.com, pgsql-bugs@lists.postgresql.org, Christoph Berg <myon@debian.org>; +Cc: PostgreSQL Developers <pgsql-hackers@postgresql.org>; Guillaume Lelarge <guillaume@lelarge.info>; Luca Vallisa <luca.vallisa@gmail.com>

Hello,

I've been looking at this bug once again and I think I finally
understood what's going on and how to fix it.

Ref 1: https://postgr.es/m/20230707175859.17c91538@karst
       Re: Issue attaching a table to a partitioned table with an
       auto-referenced foreign key
       (Guillaume Lelarge)
Ref 2: https://postgr.es/m/18156-a44bc7096f0683e6@postgresql.org
       BUG #18156: Self-referential foreign key in partitioned table not
       enforced on deletes
       (Matthew Gabeler-Lee)
Ref 3: https://postgr.es/m/myvsiF-Attja5DcWoUWh21R12R-sfXECY2-3ynt8kaOqjw@mail.gmail.com
       Self referential foreign keys in partitioned table not working as
       expected
       (Luca Vallisa)

First of all -- apparently we broke this in commit 5914a22f6ea5 (which
fixed the other problems that had been reported by G. Lelarge in Ref 1)
even worse than how it was before, by having the new functions just skip
processing the referenced side altogether.  Previously we were at least
partially setting up the necessary triggers, at least some of the time.
So what the report by Luca is saying is, plain and simple, that the
referenced-side action triggers just do not exist, which is why no error
is thrown even on the most trivial cases, on the releases that contain
that commit (17.1, 16.5, 15.9).

The solution I came up with, is to no longer skip creating the
referenced-side objects when the FK is self-referencing.  But in order
for this to work, when cloning FKs to partitions, we must process the
referencing side first rather than the referenced side first as we did
up to now; if we don't do it that way, the code there gets confused
about multiple constraint entries already existing for the same table.
Which one gets processed first shouldn't really have any other important
effect, unless I have overlooked something.

The patch also adds equivalent test cases to what was reported; if I
remove the code fix, these cases fail because they no longer report the
expected errors.  (The changes to the other regression expected fails
reflect the triggers that are missing.)

We already fixed some bits in 614a406b4ff1 and siblings, but apparently
the test cases we added there were insufficient, or we would have
detected that we were making things worse in 5914a22f6ea5 :-(

Anyway, if people have a chance to give this a look, it would be
helpful.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Linux transformó mi computadora, de una `máquina para hacer cosas',
en un aparato realmente entretenido, sobre el cual cada día aprendo
algo nuevo" (Jaime Salinas)

Attachments:

  [text/x-diff] 0001-Fix-self-referencing-foreign-keys-on-partitioned-tab.patch (0B, ../../202505011214.hrsl4g36ngh2@alvherre.pgsql/2-0001-Fix-self-referencing-foreign-keys-on-partitioned-tab.patch)
  download

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
@ 2025-05-01 15:21  Tender Wang <tndrwang@gmail.com>
  parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
  0 siblings, 1 reply; 5+ messages in thread

From: Tender Wang @ 2025-05-01 15:21 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; fastcat@gmail.com, pgsql-bugs@lists.postgresql.org, Christoph Berg <myon@debian.org>; PostgreSQL Developers <pgsql-hackers@postgresql.org>; Guillaume Lelarge <guillaume@lelarge.info>; Luca Vallisa <luca.vallisa@gmail.com>

Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月1日周四 20:17写道:

> Hello,
>
> I've been looking at this bug once again and I think I finally
> understood what's going on and how to fix it.
>
> Ref 1: https://postgr.es/m/20230707175859.17c91538@karst
>        Re: Issue attaching a table to a partitioned table with an
>        auto-referenced foreign key
>        (Guillaume Lelarge)
> Ref 2: https://postgr.es/m/18156-a44bc7096f0683e6@postgresql.org
>        BUG #18156: Self-referential foreign key in partitioned table not
>        enforced on deletes
>        (Matthew Gabeler-Lee)
> Ref 3:
> https://postgr.es/m/myvsiF-Attja5DcWoUWh21R12R-sfXECY2-3ynt8kaOqjw@mail.gmail.com
>        Self referential foreign keys in partitioned table not working as
>        expected
>        (Luca Vallisa)
>
> First of all -- apparently we broke this in commit 5914a22f6ea5 (which
> fixed the other problems that had been reported by G. Lelarge in Ref 1)
> even worse than how it was before, by having the new functions just skip
> processing the referenced side altogether.  Previously we were at least
> partially setting up the necessary triggers, at least some of the time.
> So what the report by Luca is saying is, plain and simple, that the
> referenced-side action triggers just do not exist, which is why no error
> is thrown even on the most trivial cases, on the releases that contain
> that commit (17.1, 16.5, 15.9).
>

Hmm.  I didn't get the same conclusion.
Before commit 5914a22f6ea5, the issue reported by Luca could have happened.
Look at the test below on v17.0:
psql (17.0)
Type "help" for help.

postgres=# create table test (
    id_1 int4 not null,
    id_2 int4 not null,
    parent_id_2 int4 null,
    primary key (id_1, id_2),
    foreign key (id_1, parent_id_2) references test (id_1, id_2)
) partition by list (id_1);
create table test_1 partition of test for values in (1);
insert into test values (1, 1, null), (1, 2, 1);
delete from test where (id_1, id_2) = (1, 1);
CREATE TABLE
CREATE TABLE
INSERT 0 2
DELETE 1

You can see from the above test that no error was reported.
But if I revert the commit 614a406b4ff1,  above test would report error on
v16devel:
psql (16devel)
Type "help" for help.

postgres=# create table test (
    id_1 int4 not null,
    id_2 int4 not null,
    parent_id_2 int4 null,
    primary key (id_1, id_2),
    foreign key (id_1, parent_id_2) references test (id_1, id_2)
) partition by list (id_1);
create table test_1 partition of test for values in (1);
insert into test values (1, 1, null), (1, 2, 1);
delete from test where (id_1, id_2) = (1, 1);
CREATE TABLE
CREATE TABLE
INSERT 0 2
ERROR:  update or delete on table "test_1" violates foreign key constraint
"test_id_1_parent_id_2_fkey1" on table "test"
DETAIL:  Key (id_1, id_2)=(1, 1) is still referenced from table "test".


> Anyway, if people have a chance to give this a look, it would be
> helpful.
>

It's midnight in my time zone. I will look at this tomorrow.

-- 
Thanks,
Tender Wang

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
@ 2025-05-03 13:09  Alvaro Herrera <alvherre@kurilemu.de>
  parent: Tender Wang <tndrwang@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Alvaro Herrera @ 2025-05-03 13:09 UTC (permalink / raw)
  To: Tender Wang <tndrwang@gmail.com>; +Cc: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; fastcat@gmail.com, pgsql-bugs@lists.postgresql.org, Christoph Berg <myon@debian.org>; PostgreSQL Developers <pgsql-hackers@postgresql.org>; Guillaume Lelarge <guillaume@lelarge.info>; Luca Vallisa <luca.vallisa@gmail.com>

On 2025-May-01, Tender Wang wrote:

> Hmm.  I didn't get the same conclusion.
> Before commit 5914a22f6ea5, the issue reported by Luca could have happened.
[...]
> You can see from the above test that no error was reported.
> But if I revert the commit 614a406b4ff1,  above test would report error on
> v16devel:

Yeah, I was mistaken to blame 5914a22f6ea5 for this issue when the real
culprit was 614a406b4ff1.  Anyway, I pushed the proposed fix to all
branches last night, so hopefully it works correctly for all cases now. 

(As context -- it took me several weeks or months to get FKs on
partitioned tables to work.  People would make fun at the "spider"
diagrams I drew on whiteboards, of the relationships between
pg_constraint and pg_trigger entries.  And for some reason at no point
did the idea of self-referencing FKs occurred to me.  I should have
realized that the complexity was getting out of hand!  At the very least
I should have pressed for some more QA help.)

Y'all are still on time to test this a bit more before next week's
releases ... if I have made things even worse I can still revert the
patch.  With luck, that won't be necessary.

Regards

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Java is clearly an example of money oriented programming"  (A. Stepanov)





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
@ 2025-05-08 17:14  Matthew Gabeler-Lee <fastcat@gmail.com>
  parent: Alvaro Herrera <alvherre@kurilemu.de>
  0 siblings, 1 reply; 5+ messages in thread

From: Matthew Gabeler-Lee @ 2025-05-08 17:14 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@kurilemu.de>; +Cc: Tender Wang <tndrwang@gmail.com>; Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; pgsql-bugs@lists.postgresql.org, Christoph Berg <myon@debian.org>; PostgreSQL Developers <pgsql-hackers@postgresql.org>; Guillaume Lelarge <guillaume@lelarge.info>; Luca Vallisa <luca.vallisa@gmail.com>

On Sat, May 3, 2025 at 9:09 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:
> Y'all are still on time to test this a bit more before next week's
> releases ... if I have made things even worse I can still revert the
> patch.  With luck, that won't be necessary.

I was not able to get to testing this before the release, but I was
able to test today after the release at least.

My earlier test case, and an adjusted version of it that is closer to
my production application (notably adding ON DELETE CASCADE to the
FK), now passes with 15.13.

Thank you :D





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key
@ 2025-05-08 17:35  Alvaro Herrera <alvherre@kurilemu.de>
  parent: Matthew Gabeler-Lee <fastcat@gmail.com>
  0 siblings, 0 replies; 5+ messages in thread

From: Alvaro Herrera @ 2025-05-08 17:35 UTC (permalink / raw)
  To: Matthew Gabeler-Lee <fastcat@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; pgsql-bugs@lists.postgresql.org, Christoph Berg <myon@debian.org>; PostgreSQL Developers <pgsql-hackers@postgresql.org>; Guillaume Lelarge <guillaume@lelarge.info>; Luca Vallisa <luca.vallisa@gmail.com>

On 2025-May-08, Matthew Gabeler-Lee wrote:

> My earlier test case, and an adjusted version of it that is closer to
> my production application (notably adding ON DELETE CASCADE to the
> FK), now passes with 15.13.

Thank you, that's a relief to know.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Porque Kim no hacía nada, pero, eso sí,
con extraordinario éxito" ("Kim", Kipling)





^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2025-05-08 17:35 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-05-01 12:14 Re: Issue attaching a table to a partitioned table with an auto-referenced foreign key Alvaro Herrera <alvherre@alvh.no-ip.org>
2025-05-01 15:21 ` Tender Wang <tndrwang@gmail.com>
2025-05-03 13:09   ` Alvaro Herrera <alvherre@kurilemu.de>
2025-05-08 17:14     ` Matthew Gabeler-Lee <fastcat@gmail.com>
2025-05-08 17:35       ` Alvaro Herrera <alvherre@kurilemu.de>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox