Re: [BUG] parenting a PK constraint to a self-FK one (Was: Self FK oddity when attaching a partition) - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: [BUG] parenting a PK constraint to a self-FK one (Was: Self FK oddity when attaching a partition)
Date
Msg-id CALNJ-vTo5Drs_=VP0M4FfoMQ8wVYx3op-_r0XDPVuJE1syWMpA@mail.gmail.com
Whole thread Raw
In response to Re: [BUG] parenting a PK constraint to a self-FK one (Was: Self FK oddity when attaching a partition)  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: [BUG] parenting a PK constraint to a self-FK one (Was: Self FK oddity when attaching a partition)
List pgsql-hackers


On Tue, Aug 23, 2022 at 9:30 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
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/

I was thinking of the following patch.
Basically, if there is only one matching constraint. we still return it.

diff --git a/src/postgres/src/backend/catalog/pg_constraint.c b/src/postgres/src/backend/catalog/pg_constraint.c
index f0726e9aa0..ddade138b4 100644
--- a/src/postgres/src/backend/catalog/pg_constraint.c
+++ b/src/postgres/src/backend/catalog/pg_constraint.c
@@ -1003,7 +1003,8 @@ get_relation_idx_constraint_oid(Oid relationId, Oid indexId)
  constrForm = (Form_pg_constraint) GETSTRUCT(tuple);
  if (constrForm->conindid == indexId)
  {
- constraintId = HeapTupleGetOid(tuple);
+ if (constraintId == InvalidOid || constrForm->confrelid == 0)
+ constraintId = HeapTupleGetOid(tuple);
  break;
  }
  } 

pgsql-hackers by date:

Previous
From: Nikita Glukhov
Date:
Subject: Re: SQL/JSON features for v15
Next
From: Alvaro Herrera
Date:
Subject: Re: [BUG] parenting a PK constraint to a self-FK one (Was: Self FK oddity when attaching a partition)