Re: partitioned tables referenced by FKs
От | Alvaro Herrera |
---|---|
Тема | Re: partitioned tables referenced by FKs |
Дата | |
Msg-id | 20190318220223.GA15196@alvherre.pgsql обсуждение исходный текст |
Ответ на | Re: partitioned tables referenced by FKs (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>) |
Ответы |
Re: partitioned tables referenced by FKs
Re: partitioned tables referenced by FKs Re: partitioned tables referenced by FKs Re: partitioned tables referenced by FKs |
Список | pgsql-hackers |
On 2019-Feb-28, Amit Langote wrote: > I'd like to hear your thoughts on some suggestions to alter the structure > of the reorganized code around foreign key addition/cloning. With this > patch adding support for foreign keys to reference partitioned tables, the > code now has to consider various cases due to the possibility of having > partitioned tables on both sides of a foreign key, which is reflected in > the complexity of the new code. I spent a few hours studying this and my conclusion is the opposite of yours: we should make addFkRecurseReferencing the recursive one, and CloneFkReferencing a non-recursive caller of that. So we end up with both addFkRecurseReferenced and addFkRecurseReferencing as recursive routines, and CloneFkReferenced and CloneFkReferencing being non-recursive callers of those. With this structure change there is one more call to CreateConstraintEntry than before, and now there are two calls of tryAttachPartitionForeignKey instead of one; I think with this new structure things are much simpler. I also changed CloneForeignKeyConstraints's API: instead of returning a list of cloned constraint giving its caller the responsibility of adding FK checks to phase 3, we now give CloneForeignKeyConstraints the 'wqueue' list, so that it can add the FK checks itself. It seems much cleaner this way. > Also, it seems a bit confusing that there is a CreateConstraintEntry call > in addFkRecurseReferenced() which is creating a constraint on the > *referencing* relation, which I think should be in > ATAddForeignKeyConstraint, the caller. I know we need to create a copy of > the constraint to reference the partitions of the referenced table, but we > might as well put it in CloneFkReferenced and reverse who calls who -- > make addFkRecurseReferenced() call CloneFkReferenced and have the code to > create the cloned constraint and action triggers in the latter. That will > make the code to handle different sides of foreign key look similar, and > imho, easier to follow. Well, if you think about it, *all* the constraints created by all these routines are in the referencing relations. The only question here is *why* we create those tuples; in the case of the ...Referenced routines, it's because of the partitions in the referenced side; in the case of the ...Referencing routines, it's because of partitions in the referencing side. I think changing it the way you suggest would be even more confusing. As discussed in the other subthread, I'm not making any effort to reuse an existing constraint defined in a partition of the referenced side; as far as I can tell that's a nonsensical transformation. A pretty silly bug remains here. Watch: create table pk (a int primary key) partition by list (a); create table pk1 partition of pk for values in (1); create table fk (a int references pk); insert into pk values (1); insert into fk values (1); drop table pk cascade; Note that the DROP of the main PK table is pretty aggressive: since it cascades, you want it to drop the constraint on the FK side. This would work without a problem if 'pk' was a non-partitioned table, but in this case it fails: alvherre=# drop table pk cascade; NOTICE: drop cascades to constraint fk_a_fkey on table fk ERROR: removing partition "pk1" violates foreign key constraint "fk_a_fkey1" DETALLE: Key (a)=(1) still referenced from table "fk". The reason is that the "pre drop check" that's done before allow a drop of the partition doesn't realize that the constraint is also being dropped (and so the check ought to be skipped). If you were to do just "DROP TABLE pk1" then this complaint would be correct, since it would leave the constraint in an invalid state. But here, it's bogus and annoying. You can DELETE the matching values from table FK and then the DROP works. Here's another problem caused by the same misbehavior: alvherre=# drop table pk, fk; ERROR: removing partition "pk1" violates foreign key constraint "fk_a_fkey1" DETALLE: Key (a)=(1) still referenced from table "fk". Note here we want to get rid of table 'fk' completely. If you split it up in a DROP of fk, followed by a DROP of pk, it works. I'm not yet sure what's the best way to attack this. Maybe the "pre-drop check" needs a completely different approach. The simplest approach is to prohibit a table drop or detach for any partitioned table that's referenced by a foreign key, but that seems obnoxious and inconvenient. I still haven't put back the code in "#if 0". FWIW I think we should add an index on pg_constraint.confrelid now. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
В списке pgsql-hackers по дате отправления: