pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
[BUG] wrong FK constraint name when colliding name on ATTACH
8+ messages / 5 participants
[nested] [flat]

Commits from this thread:
  2022-09-09 68b0da6779 8c848cd4b8  Fix GetForeignKey*Triggers for self-referential FKs
  2022-09-08 e7936f8b3e ade2409b18 640c20d626 80ef25b1ad 562e100aee  Choose FK name correctly during partition attachment
* [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-01 16:41  Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Jehan-Guillaume de Rorthais @ 2022-09-01 16:41 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-hackers

Hi,

While studying and hacking on the parenting constraint issue, I found an
incoherent piece of code leading to badly chosen fk name. If a constraint
name collision is detected, while choosing a new name for the constraint,
the code uses fkconstraint->fk_attrs which is not yet populated:

  /* No dice.  Set up to create our own constraint */
  fkconstraint = makeNode(Constraint);
  if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
                           RelationGetRelid(partRel),
                           NameStr(constrForm->conname)))
      fkconstraint->conname =
          ChooseConstraintName(RelationGetRelationName(partRel),
                               ChooseForeignKeyConstraintNameAddition(
                                  fkconstraint->fk_attrs),  // <= WOO000OPS
                               "fkey",
                               RelationGetNamespace(partRel), NIL);
  else
      fkconstraint->conname = pstrdup(NameStr(constrForm->conname));
  fkconstraint->fk_upd_action = constrForm->confupdtype;
  fkconstraint->fk_del_action = constrForm->confdeltype;
  fkconstraint->deferrable = constrForm->condeferrable;
  fkconstraint->initdeferred = constrForm->condeferred;
  fkconstraint->fk_matchtype = constrForm->confmatchtype;
  for (int i = 0; i < numfks; i++)
  {
      Form_pg_attribute att;
  
      att = TupleDescAttr(RelationGetDescr(partRel),
                          mapped_conkey[i] - 1);
      fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, // <= POPULATING
                                       makeString(NameStr(att->attname)));
  }

The following SQL script showcase the bad constraint name:

  DROP TABLE IF EXISTS parent, child1;
  
  CREATE TABLE parent (
      id bigint NOT NULL default 1,
      no_part smallint NOT NULL,
      id_abc bigint,
      CONSTRAINT dummy_constr FOREIGN KEY (id_abc, no_part)
          REFERENCES parent(id, no_part) ON UPDATE RESTRICT ON DELETE RESTRICT,
      PRIMARY KEY (id, no_part)
  )
  PARTITION BY LIST (no_part);
  
  CREATE TABLE child1 (
      id bigint NOT NULL default 1,
      no_part smallint NOT NULL,
      id_abc bigint,
      PRIMARY KEY (id, no_part),
      CONSTRAINT dummy_constr CHECK ((no_part = 1))
  );

  ALTER TABLE parent ATTACH PARTITION child1 FOR VALUES IN ('1');

  SELECT conname
  FROM pg_constraint
  WHERE conrelid = 'child1'::regclass
    AND contype = 'f';

  DROP TABLE
  CREATE TABLE
  CREATE TABLE
  ALTER TABLE
  
     conname    
  --------------
   child1__fkey
  (1 row)

The resulting constraint name "child1__fkey" is missing the attributes name the
original code wanted to add. The expected name is "child1_id_abc_no_part_fkey".

Find in attachment a simple fix, moving the name assignation after the
FK attributes are populated.

Regards,

Attachments:

0001-Fix-FK-name-when-colliding-during-partition-attachme.patch (text/x-patch, 2.3K) download From f1eeacb1eb3face38d76325666aff57019ef84c9 Mon Sep 17 00:00:00 2001 From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Date: Thu, 1 Sep 2022 17:41:55 +0200 Subject: [PATCH] Fix FK name when colliding during partition attachment During ATLER TABLE ATTACH PARTITION, if the name of a parent's foreign key constraint is already used on the partition, the code try to choose another one before the FK attributes list has been populated. The resulting constraint name was a strange "<relname>__fkey" instead of "<relname>_<attrs>_fkey". --- src/backend/commands/tablecmds.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index dacc989d85..53b0f3a9c1 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10304,16 +10304,6 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) /* No dice. Set up to create our own constraint */ fkconstraint = makeNode(Constraint); - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partRel), - NameStr(constrForm->conname))) - fkconstraint->conname = - ChooseConstraintName(RelationGetRelationName(partRel), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partRel), NIL); - else - fkconstraint->conname = pstrdup(NameStr(constrForm->conname)); fkconstraint->fk_upd_action = constrForm->confupdtype; fkconstraint->fk_del_action = constrForm->confdeltype; fkconstraint->deferrable = constrForm->condeferrable; @@ -10328,6 +10318,16 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, makeString(NameStr(att->attname))); } + if (ConstraintNameIsUsed(CONSTRAINT_RELATION, + RelationGetRelid(partRel), + NameStr(constrForm->conname))) + fkconstraint->conname = + ChooseConstraintName(RelationGetRelationName(partRel), + ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), + "fkey", + RelationGetNamespace(partRel), NIL); + else + fkconstraint->conname = pstrdup(NameStr(constrForm->conname)); indexOid = constrForm->conindid; constrOid = -- 2.37.2
^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-08 07:40  Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
  parent: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Jehan-Guillaume de Rorthais @ 2022-09-08 07:40 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-hackers

Hi there,

I believe this very small bug and its fix are really trivial and could be push
out of the way quite quickly. It's just about a bad constraint name fixed by
moving one assignation after the next one. This could easily be fixed for next
round of releases.

Well, I hope I'm not wrong :)

Regards,

On Thu, 1 Sep 2022 18:41:56 +0200
Jehan-Guillaume de Rorthais <jgdr@dalibo.com> wrote:

> While studying and hacking on the parenting constraint issue, I found an
> incoherent piece of code leading to badly chosen fk name. If a constraint
> name collision is detected, while choosing a new name for the constraint,
> the code uses fkconstraint->fk_attrs which is not yet populated:
> 
>   /* No dice.  Set up to create our own constraint */
>   fkconstraint = makeNode(Constraint);
>   if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
>                            RelationGetRelid(partRel),
>                            NameStr(constrForm->conname)))
>       fkconstraint->conname =
>           ChooseConstraintName(RelationGetRelationName(partRel),
>                                ChooseForeignKeyConstraintNameAddition(
>                                   fkconstraint->fk_attrs),  // <= WOO000OPS
>                                "fkey",
>                                RelationGetNamespace(partRel), NIL);
>   else
>       fkconstraint->conname = pstrdup(NameStr(constrForm->conname));
>   fkconstraint->fk_upd_action = constrForm->confupdtype;
>   fkconstraint->fk_del_action = constrForm->confdeltype;
>   fkconstraint->deferrable = constrForm->condeferrable;
>   fkconstraint->initdeferred = constrForm->condeferred;
>   fkconstraint->fk_matchtype = constrForm->confmatchtype;
>   for (int i = 0; i < numfks; i++)
>   {
>       Form_pg_attribute att;
>   
>       att = TupleDescAttr(RelationGetDescr(partRel),
>                           mapped_conkey[i] - 1);
>       fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, // <=
> POPULATING makeString(NameStr(att->attname)));
>   }
> 
> The following SQL script showcase the bad constraint name:
> 
>   DROP TABLE IF EXISTS parent, child1;
>   
>   CREATE TABLE parent (
>       id bigint NOT NULL default 1,
>       no_part smallint NOT NULL,
>       id_abc bigint,
>       CONSTRAINT dummy_constr FOREIGN KEY (id_abc, no_part)
>           REFERENCES parent(id, no_part) ON UPDATE RESTRICT ON DELETE
> RESTRICT, PRIMARY KEY (id, no_part)
>   )
>   PARTITION BY LIST (no_part);
>   
>   CREATE TABLE child1 (
>       id bigint NOT NULL default 1,
>       no_part smallint NOT NULL,
>       id_abc bigint,
>       PRIMARY KEY (id, no_part),
>       CONSTRAINT dummy_constr CHECK ((no_part = 1))
>   );
> 
>   ALTER TABLE parent ATTACH PARTITION child1 FOR VALUES IN ('1');
> 
>   SELECT conname
>   FROM pg_constraint
>   WHERE conrelid = 'child1'::regclass
>     AND contype = 'f';
> 
>   DROP TABLE
>   CREATE TABLE
>   CREATE TABLE
>   ALTER TABLE
>   
>      conname    
>   --------------
>    child1__fkey
>   (1 row)
> 
> The resulting constraint name "child1__fkey" is missing the attributes name
> the original code wanted to add. The expected name is
> "child1_id_abc_no_part_fkey".
> 
> Find in attachment a simple fix, moving the name assignation after the
> FK attributes are populated.
> 
> Regards,






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

* Re: [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-08 11:25  Alvaro Herrera <alvherre@alvh.no-ip.org>
  parent: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
  0 siblings, 2 replies; 8+ messages in thread

From: Alvaro Herrera @ 2022-09-08 11:25 UTC (permalink / raw)
  To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; +Cc: pgsql-hackers

On 2022-Sep-08, Jehan-Guillaume de Rorthais wrote:

> Hi there,
> 
> I believe this very small bug and its fix are really trivial and could be push
> out of the way quite quickly. It's just about a bad constraint name fixed by
> moving one assignation after the next one. This could easily be fixed for next
> round of releases.
> 
> Well, I hope I'm not wrong :)

I think you're right, so pushed, and backpatched to 12.  I added the
test case to regression also.

For 11, I adjusted the test case so that it didn't depend on an FK
pointing to a partitioned table (which is not supported there); it turns
out that the old code is not smart enough to get into the problem in the
first place.  Setup is

CREATE TABLE parted_fk_naming_pk (id bigint primary key);
CREATE TABLE parted_fk_naming (
    id_abc bigint,
    CONSTRAINT dummy_constr FOREIGN KEY (id_abc)
        REFERENCES parted_fk_naming_pk (id)
)
PARTITION BY LIST (id_abc);
CREATE TABLE parted_fk_naming_1 (
    id_abc bigint,
    CONSTRAINT dummy_constr CHECK (true)
);

and then
ALTER TABLE parted_fk_naming ATTACH PARTITION parted_fk_naming_1 FOR VALUES IN ('1');
throws this error:

ERROR:  duplicate key value violates unique constraint "pg_constraint_conrelid_contypid_conname_index"
DETALLE:  Key (conrelid, contypid, conname)=(686125, 0, dummy_constr) already exists.

It seems fair to say that this case, with pg11, is unsupported and
people should upgrade if they want better behavior.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Those who use electric razors are infidels destined to burn in hell while
we drink from rivers of beer, download free vids and mingle with naked
well shaved babes." (http://slashdot.org/comments.pl?sid=44793&cid=4647152)





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

* Re: [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-08 12:07  Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
  parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
  1 sibling, 0 replies; 8+ messages in thread

From: Jehan-Guillaume de Rorthais @ 2022-09-08 12:07 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-hackers

On Thu, 8 Sep 2022 13:25:15 +0200
Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

> On 2022-Sep-08, Jehan-Guillaume de Rorthais wrote:
> 
> > Hi there,
> > 
> > I believe this very small bug and its fix are really trivial and could be
> > push out of the way quite quickly. It's just about a bad constraint name
> > fixed by moving one assignation after the next one. This could easily be
> > fixed for next round of releases.
> > 
> > Well, I hope I'm not wrong :)  
> 
> I think you're right, so pushed, and backpatched to 12.  I added the
> test case to regression also.

Great, thank you for the additional work on the regression test and the commit!

> For 11, I adjusted the test case so that it didn't depend on an FK
> pointing to a partitioned table (which is not supported there); it turns
> out that the old code is not smart enough to get into the problem in the
> first place.  [...]
> It seems fair to say that this case, with pg11, is unsupported and
> people should upgrade if they want better behavior.

That works for me.

Thanks!





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

* Re: [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-08 17:20  Andres Freund <andres@anarazel.de>
  parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
  1 sibling, 1 reply; 8+ messages in thread

From: Andres Freund @ 2022-09-08 17:20 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; pgsql-hackers

Hi,

On 2022-09-08 13:25:15 +0200, Alvaro Herrera wrote:
> I think you're right, so pushed, and backpatched to 12.  I added the
> test case to regression also.

Something here doesn't look to be quite right. Starting with this commit CI
[1] started to fail on freebsd (stack trace [2]), and in the meson branch I've
also seen the crash on windows (CI run[3], stack trace [4]).

There does appear to be some probabilistic aspect, I also saw a run succeed.

Greetings,

Andres Freund

[1] https://cirrus-ci.com/github/postgres/postgres/
[2] https://api.cirrus-ci.com/v1/task/6180840047640576/logs/cores.log
[3] https://cirrus-ci.com/task/6629440791773184
[4] https://api.cirrus-ci.com/v1/artifact/task/6629440791773184/crashlog/crashlog-postgres.exe_1468_2022...





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

* Re: [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-08 19:54  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Andres Freund <andres@anarazel.de>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2022-09-08 19:54 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; pgsql-hackers

Andres Freund <andres@anarazel.de> writes:
> Something here doesn't look to be quite right. Starting with this commit CI
> [1] started to fail on freebsd (stack trace [2]), and in the meson branch I've
> also seen the crash on windows (CI run[3], stack trace [4]).

The crash seems 100% reproducible if I remove the early-exit optimization
from GetForeignKeyActionTriggers:

inline patch 1: src/backend/commands/tablecmds.c (+0/-2 lines) download inline-1.patchdiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 53b0f3a9c1..112ca77d97 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10591,8 +10591,6 @@ GetForeignKeyActionTriggers(Relation trigrel, Assert(*updateTriggerOid == InvalidOid); *updateTriggerOid = trgform->oid; } - if (OidIsValid(*deleteTriggerOid) && OidIsValid(*updateTriggerOid)) - break; } if (!OidIsValid(*deleteTriggerOid))
With that in place, it's probabilistic whether the Asserts notice anything wrong, and mostly they don't. But there are multiple matching triggers: regression=# select oid, tgconstraint, tgrelid,tgconstrrelid, tgtype, tgname from pg_trigger where tgconstraint = 104301; oid | tgconstraint | tgrelid | tgconstrrelid | tgtype | tgname --------+--------------+---------+---------------+--------+------------------------------- 104302 | 104301 | 104294 | 104294 | 9 | RI_ConstraintTrigger_a_104302 104303 | 104301 | 104294 | 104294 | 17 | RI_ConstraintTrigger_a_104303 104304 | 104301 | 104294 | 104294 | 5 | RI_ConstraintTrigger_c_104304 104305 | 104301 | 104294 | 104294 | 17 | RI_ConstraintTrigger_c_104305 (4 rows) I suspect that the filter conditions being applied are inadequate for the case of a self-referential FK, which this evidently is given that tgrelid and tgconstrrelid are equal. I'd counsel dropping the early-exit optimization; it doesn't save much I expect, and it evidently hides bugs. Or maybe make it conditional on !USE_ASSERT_CHECKING. regards, tom lane ^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-09 07:16  Amit Langote <amitlangote09@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 8+ messages in thread

From: Amit Langote @ 2022-09-09 07:16 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Alvaro Herrera <alvherre@alvh.no-ip.org>; Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; pgsql-hackers

On Fri, Sep 9, 2022 at 4:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Something here doesn't look to be quite right. Starting with this commit CI
> > [1] started to fail on freebsd (stack trace [2]), and in the meson branch I've
> > also seen the crash on windows (CI run[3], stack trace [4]).
>
> The crash seems 100% reproducible if I remove the early-exit optimization
> from GetForeignKeyActionTriggers:

Indeed, reproduced here.

> diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
> index 53b0f3a9c1..112ca77d97 100644
> --- a/src/backend/commands/tablecmds.c
> +++ b/src/backend/commands/tablecmds.c
> @@ -10591,8 +10591,6 @@ GetForeignKeyActionTriggers(Relation trigrel,
>             Assert(*updateTriggerOid == InvalidOid);
>             *updateTriggerOid = trgform->oid;
>         }
> -       if (OidIsValid(*deleteTriggerOid) && OidIsValid(*updateTriggerOid))
> -           break;
>     }
>
>     if (!OidIsValid(*deleteTriggerOid))
>
> With that in place, it's probabilistic whether the Asserts notice anything
> wrong, and mostly they don't.  But there are multiple matching triggers:
>
> regression=# select oid, tgconstraint, tgrelid,tgconstrrelid, tgtype, tgname from pg_trigger where tgconstraint = 104301;
>   oid   | tgconstraint | tgrelid | tgconstrrelid | tgtype |            tgname
> --------+--------------+---------+---------------+--------+-------------------------------
>  104302 |       104301 |  104294 |        104294 |      9 | RI_ConstraintTrigger_a_104302
>  104303 |       104301 |  104294 |        104294 |     17 | RI_ConstraintTrigger_a_104303
>  104304 |       104301 |  104294 |        104294 |      5 | RI_ConstraintTrigger_c_104304
>  104305 |       104301 |  104294 |        104294 |     17 | RI_ConstraintTrigger_c_104305
> (4 rows)
>
> I suspect that the filter conditions being applied are inadequate
> for the case of a self-referential FK, which this evidently is
> given that tgrelid and tgconstrrelid are equal.

Yes, the loop in GetForeignKeyActionTriggers() needs this:

+       /* Only ever look at "action" triggers on the PK side. */
+       if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_PK)
+           continue;

Likewise, GetForeignKeyActionTriggers() needs this:

+       /* Only ever look at "check" triggers on the FK side. */
+       if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_FK)
+           continue;

We evidently missed this in f4566345cf40b0.

> I'd counsel dropping the early-exit optimization; it doesn't
> save much I expect, and it evidently hides bugs.  Or maybe
> make it conditional on !USE_ASSERT_CHECKING.

While neither of these functions are called in hot paths, I am
inclined to keep the early-exit bit in non-assert builds.

Attached a patch.

-- 
Thanks, Amit Langote
EDB: http://www.enterprisedb.com

Attachments:

action-check-trigger-filter.patch (application/octet-stream, 1.7K) download diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 53b0f3a9c1..e96e2ead6c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10581,6 +10581,9 @@ GetForeignKeyActionTriggers(Relation trigrel, continue; if (trgform->tgrelid != confrelid) continue; + /* Only ever look at "action" triggers on the PK side. */ + if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_PK) + continue; if (TRIGGER_FOR_DELETE(trgform->tgtype)) { Assert(*deleteTriggerOid == InvalidOid); @@ -10591,8 +10594,11 @@ GetForeignKeyActionTriggers(Relation trigrel, Assert(*updateTriggerOid == InvalidOid); *updateTriggerOid = trgform->oid; } +#ifndef USE_ASSERT_CHECKING + /* In an assert-enabled build, continue looking to find duplicates. */ if (OidIsValid(*deleteTriggerOid) && OidIsValid(*updateTriggerOid)) break; +#endif } if (!OidIsValid(*deleteTriggerOid)) @@ -10636,6 +10642,9 @@ GetForeignKeyCheckTriggers(Relation trigrel, continue; if (trgform->tgrelid != conrelid) continue; + /* Only ever look at "check" triggers on the FK side. */ + if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_FK) + continue; if (TRIGGER_FOR_INSERT(trgform->tgtype)) { Assert(*insertTriggerOid == InvalidOid); @@ -10646,8 +10655,11 @@ GetForeignKeyCheckTriggers(Relation trigrel, Assert(*updateTriggerOid == InvalidOid); *updateTriggerOid = trgform->oid; } +#ifndef USE_ASSERT_CHECKING + /* In an assert-enabled build, continue looking to find duplicates. */ if (OidIsValid(*insertTriggerOid) && OidIsValid(*updateTriggerOid)) break; +#endif } if (!OidIsValid(*insertTriggerOid))
^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: [BUG] wrong FK constraint name when colliding name on ATTACH
@ 2022-09-09 10:31  Alvaro Herrera <alvherre@alvh.no-ip.org>
  parent: Amit Langote <amitlangote09@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alvaro Herrera @ 2022-09-09 10:31 UTC (permalink / raw)
  To: Amit Langote <amitlangote09@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; Jehan-Guillaume de Rorthais <jgdr@dalibo.com>; pgsql-hackers

On 2022-Sep-09, Amit Langote wrote:

> Yes, the loop in GetForeignKeyActionTriggers() needs this:
> 
> +       /* Only ever look at "action" triggers on the PK side. */
> +       if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_PK)
> +           continue;
> 
> Likewise, GetForeignKeyActionTriggers() needs this:
> 
> +       /* Only ever look at "check" triggers on the FK side. */
> +       if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_FK)
> +           continue;
> 
> We evidently missed this in f4566345cf40b0.

Ouch.  Thank you, pushed.

> On Fri, Sep 9, 2022 at 4:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> > I'd counsel dropping the early-exit optimization; it doesn't
> > save much I expect, and it evidently hides bugs.  Or maybe
> > make it conditional on !USE_ASSERT_CHECKING.
> 
> While neither of these functions are called in hot paths, I am
> inclined to keep the early-exit bit in non-assert builds.

I kept it that way.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"El hombre nunca sabe de lo que es capaz hasta que lo intenta" (C. Dickens)





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


end of thread, other threads:[~2022-09-09 10:31 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-09-01 16:41 [BUG] wrong FK constraint name when colliding name on ATTACH Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
2022-09-08 07:40 ` Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
2022-09-08 11:25   ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2022-09-08 12:07     ` Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
2022-09-08 17:20     ` Andres Freund <andres@anarazel.de>
2022-09-08 19:54       ` Tom Lane <tgl@sss.pgh.pa.us>
2022-09-09 07:16         ` Amit Langote <amitlangote09@gmail.com>
2022-09-09 10:31           ` Alvaro Herrera <alvherre@alvh.no-ip.org>

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