Re: [BUG] parenting a PK constraint to a self-FK one (Was: Self FK oddity when attaching a partition) - Mailing list 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,