pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [BUG] parenting a PK constraint to a self-FK one  (Was: Self FK oddity when attaching a partition)
Date: Tue, 23 Aug 2022 18:30:06 +0200
Message-ID: <20220823163006.smdpovyl4dhcakvr@alvherre.pgsql> (raw)
In-Reply-To: <20220823170737.30d29668@karst>

On 2022-Aug-23, Jehan-Guillaume de Rorthais wrote:

Hi,

[...]

> However, it seems get_relation_idx_constraint_oid(), introduced in eb7ed3f3063,
> assume there could be only ONE constraint depending to an index. But in fact,
> multiple constraints can rely on the same index, eg.: the PK and a self
> referencing FK. In consequence, when looking for a constraint depending on an
> index for the given relation, either the FK or a PK can appears first depending
> on various conditions. It is then possible to trick it make a FK constraint a
> parent of a PK...

Hmm, wow, that sounds extremely stupid.  I think a sufficient fix might
be to have get_relation_idx_constraint_oid ignore any constraints that
are not unique or primary keys.  I tried your scenario with the attached
and it seems to work correctly.  Can you confirm?  (I only ran the
pg_regress tests, not anything else for now.)

If this is OK, we should make this API quirkiness very explicit in the
comments, so the patch needs to be a few lines larger in order to be
committable.  Also, perhaps the check should be that contype equals
either primary or unique, rather than it doesn't equal foreign.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

Attachments:

0001-test-fix.patch (text/x-diff, 906B) download From a39299db23f4b2495b3ba10e7adb1121f0271694 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvherre@alvh.no-ip.org> Date: Tue, 23 Aug 2022 18:17:24 +0200 Subject: [PATCH] test fix --- src/backend/catalog/pg_constraint.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index bb65fb1e0a..71b6fb35e6 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -1011,6 +1011,14 @@ get_relation_idx_constraint_oid(Oid relationId, Oid indexId) Form_pg_constraint constrForm; constrForm = (Form_pg_constraint) GETSTRUCT(tuple); + + /* + * Foreign key constraints are ignored for these purposes, + * because ...? + */ + if (constrForm->contype == CONSTRAINT_FOREIGN) + continue; + if (constrForm->conindid == indexId) { constraintId = constrForm->oid; -- 2.30.2

view thread (18+ messages)  latest in thread

Message-ID: <20220823163006.smdpovyl4dhcakvr@alvherre.pgsql>
Permalink:  ../20220823163006.smdpovyl4dhcakvr@alvherre.pgsql/
Also on:    postgresql.org/message-id/20220823163006.smdpovyl4dhcakvr@alvherre.pgsql

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@alvh.no-ip.org, jgdr@dalibo.com
  Subject: Re: [BUG] parenting a PK constraint to a self-FK one  (Was: Self FK oddity when attaching a partition)
  In-Reply-To: <20220823163006.smdpovyl4dhcakvr@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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