On 2019/01/25 2:18, Alvaro Herrera wrote: > On 2019-Jan-24, Amit Langote wrote: > >> A few hunks of the originally proposed patch are attached here as 0001, >> especially the part which fixes ATAddForeignKeyConstraint to pass the >> correct value of connoninherit to CreateConstraintEntry (which should be >> false for partitioned tables). With that change, many tests start failing >> because of the above bug. That patch also adds a test case like the one >> above, but it fails along with others due to the bug. Patch 0002 is the >> aforementioned simpler fix to make the errors (existing and the newly >> added) go away. > > Cool, thanks. I made a bunch of fixes -- I added an elog(ERROR) check > to ensure a constraint whose parent is being set does not already have a > parent; that seemed in line with the new asserts that check the > coninhcount. I also moved those asserts, changing the spirit of what > they checked. Also: I wasn't sure about stopping recursion for legacy > inheritance in ATExecDropConstraint() for non-check constraints, so I > changed that to occur in partitioned only. Also, stylistic fixes.
Thanks for the fixes and committing. > I was mildly surprised to realize that the my_fkey constraint on > fk_part_1_1 is gone after dropping fkey on its parent, since it was > declared locally when that table was created. However, it makes perfect > sense in retrospect, since we made it dependent on its parent. I'm not > terribly happy about this, but I don't quite see a way to make it better > that doesn't require much more code than is warranted. Fwiw, CHECK constraints behave that way too. OTOH, detaching a partition preserves all the constraints, even the ones that were never locally defined on the partition. >> These patches apply unchanged to the PG 11 branch. > > Yeah, only if you tried to compile, it would have complained about > table_close() ;-) Oops, sorry. I was really in a hurry that day as the dinnertime had passed. Thanks, Amit
