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

From: Jehan-Guillaume de Rorthais <jgdr(at)dalibo(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Subject: [BUG] parenting a PK constraint to a self-FK one (Was: Self FK oddity when attaching a partition)
Date: 2022-08-23 15:07:37
Message-ID: 20220823170737.30d29668@karst
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

I've been able to work on this issue and isolate where in the code the oddity
is laying.

During ATExecAttachPartition(), AttachPartitionEnsureIndexes() look for existing
required index on the partition to attach. It creates missing index, or sets the
parent's index when a matching one exists on the partition. Good.

When a matching index is found, if the parent index enforce a constraint, the
function look for the similar constraint in the partition-to-be, and set the
constraint parent as well:

constraintOid = get_relation_idx_constraint_oid(RelationGetRelid(rel),
idx);

[...]

/*
* If this index is being created in the parent because of a
* constraint, then the child needs to have a constraint also,
* so look for one. If there is no such constraint, this
* index is no good, so keep looking.
*/
if (OidIsValid(constraintOid))
{
cldConstrOid = get_relation_idx_constraint_oid(
RelationGetRelid(attachrel),
cldIdxId);
/* no dice */
if (!OidIsValid(cldConstrOid))
continue;
}
/* bingo. */
IndexSetParentIndex(attachrelIdxRels[i], idx);
if (OidIsValid(constraintOid))
ConstraintSetParentConstraint(cldConstrOid, constraintOid,
RelationGetRelid(attachrel));

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

In the following little scenario, when looking at the constraint linked to
the PK unique index using the same index than get_relation_idx_constraint_oid
use, this is the self-FK that is actually returned first by
get_relation_idx_constraint_oid(), NOT the PK:

postgres=# DROP TABLE IF EXISTS parent, child1;

CREATE TABLE parent (
id bigint NOT NULL default 1,
no_part smallint NOT NULL,
id_abc bigint,
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 child1 CHECK ((no_part = 1))
);

-- force an indexscan as get_relation_idx_constraint_oid() use the unique
-- index on (conrelid, contypid, conname) to scan pg_cosntraint
set enable_seqscan TO off;
set enable_bitmapscan TO off;

SELECT conname
FROM pg_constraint
WHERE conrelid = 'parent'::regclass <=== parent
AND conindid = 'parent_pkey'::regclass; <=== PK index

DROP TABLE
CREATE TABLE
CREATE TABLE
SET
SET
conname
----------------------------
parent_id_abc_no_part_fkey <==== WOOPS!
parent_pkey
(2 rows)

In consequence, when attaching the partition, the PK of child1 is not marked as
partition of the parent's PK, which is wrong. WORST, the PK of child1 is
actually unexpectedly marked as a partition of the parent's **self-FK**:

postgres=# ALTER TABLE ONLY parent ATTACH PARTITION child1
FOR VALUES IN ('1');

SELECT oid, conname, conparentid, conrelid, confrelid
FROM pg_constraint
WHERE conrelid in ('parent'::regclass, 'child1'::regclass)
ORDER BY 1;

ALTER TABLE
oid | conname | conparentid | conrelid | confrelid
-------+-----------------------------+-------------+----------+-----------
16700 | parent_pkey | 0 | 16695 | 0
16701 | parent_id_abc_no_part_fkey | 0 | 16695 | 16695
16706 | child1 | 0 | 16702 | 0
16708 | **child1_pkey** | **16701** | 16702 | 0
16709 | parent_id_abc_no_part_fkey1 | 16701 | 16695 | 16702
16712 | parent_id_abc_no_part_fkey | 16701 | 16702 | 16695
(6 rows)

The expected result should probably be something like:

oid | conname | conparentid | conrelid | confrelid
-------+-----------------------------+-------------+----------+-----------
16700 | parent_pkey | 0 | 16695 | 0
...
16708 | child1_pkey | 16700 | 16702 | 0

I suppose this bug might exists in ATExecAttachPartitionIdx(),
DetachPartitionFinalize() and DefineIndex() where there's similar code and logic
using get_relation_idx_constraint_oid(). I didn't check for potential bugs there
though.

I'm not sure yet of how this bug should be fixed. Any comment?

Regards,

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-08-23 15:08:31 Re: SQL/JSON features for v15
Previous Message Robert Haas 2022-08-23 15:04:22 Re: logical decoding and replication of sequences