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.patch
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.
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