On Sun, Feb 10, 2019 at 1:50 AM Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > On 2019-Feb-09, Tom Lane wrote: > > Alvaro Herrera <alvhe...@2ndquadrant.com> writes: > > > On 2019-Feb-09, Tom Lane wrote: > > >> No, that's still the back end of the deletion machinery, and in > > >> particular > > >> it would fail to clean pg_depend entries for the trigger. Going in by > > >> the > > >> front door would use performDeletion(). (See deleteOneObject() to get > > >> an idea of what's being possibly missed out here.) > > > > > This patch I think does the right thing. > > > > (squint ...) Don't much like the undocumented deleteDependencyRecordsFor > > call; that looks like it's redundant with what deleteOneObject will do. > > I think you're doing it to get rid of the INTERNAL dependency so that > > deletion won't recurse across that, but why is that a good idea? Needs > > a comment at least. > > Yeah, it's deleting the INTERNAL dependency, because otherwise the > trigger deletion is (correctly) forbidden, since the constraint depends > on it. Perhaps it'd be good to have it be more targetted: make sure it > only deletes that dependency row and not any others that the trigger > might have (though I don't have it shouldn't have any. How could it?)
Reading Tom's reply to my email, I wondered if performDeletion won't do more than what the code is already doing (except calling the right trigger deletion function which the current code doesn't), because the trigger in question is an internal trigger without any dependencies (the function it invokes are pinned by the system)? Thanks, Amit
