Re: Foreign keys and partitioned tables - Mailing list pgsql-hackers

Comments on the code:

@@ -7226,12 +7235,23 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab,
Relation rel,
     * numbers)
     */
    if (pkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+   {
+       /* fix recursion in ATExecValidateConstraint to enable this case */
+       if (fkconstraint->skip_validation && !fkconstraint->initially_valid)
+           ereport(ERROR,
+                   (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                    errmsg("cannot add NOT VALID foreign key to
relation \"%s\"",
+                           RelationGetRelationName(pkrel))));
+   }

Maybe this error message should be more along the lines of "is not
supported yet".  Also, this restriction should perhaps be mentioned in
the documentation additions that we have been discussing.


The first few hunks in ri_triggers.c (the ones that refer to the
pktable) are apparently for the postponed part of the feature, foreign
keys referencing partitioned tables.  So I think those hunks should be
dropped from this patch.

The removal of the ONLY for the foreign key queries also affects
inherited tables, in undesirable ways.  For example, consider this
script:

create table test1 (a int primary key);
insert into test1 values (1), (2), (3);

create table test2 (x int primary key, y int references test1 on delete
cascade);
insert into test2 values (11, 1), (22, 2), (33, 3);

create table test2a () inherits (test2);
insert into test2a values (111, 1), (222, 2);

delete from test1 where a = 1;

select * from test1 order by 1;
select * from test2 order by 1, 2;

With the patch, this will have deleted the (111, 1) record in test2a,
which it did not do before.

I think the trigger functions need to look up whether the table is a
partitioned table and decide whether the use ONLY based on that.
(This will probably also apply to the PK side, when that is
implemented.)


In pg_dump, maybe this can be refined:

+       /*
+        * For partitioned tables, it doesn't work to emit constraints
as not
+        * inherited.
+        */
+       if (tbinfo->relkind == RELKIND_PARTITIONED_TABLE)
+           only = "";
+       else
+           only = "ONLY ";

We use ONLY for foreign keys on inherited tables, but foreign keys are
not inherited anyway, so this is at best future-proofing.  We could
just drop the ONLY unconditionally.  Or at least explain this more.


Other than that, it looks OK.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services