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