Date: Sat, 9 Feb 2019 13:50:10 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Langote <amitlangote09@gmail.com>, Peter Geoghegan <pg@bowt.ie>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Fixing findDependentObjects()'s dependency on scan order
 (regressions in DROP diagnostic messages)
Message-ID: <20190209165010.GA4298@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <5943.1549728638@sss.pgh.pa.us>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On 2019-Feb-09, Tom Lane wrote:

> Alvaro Herrera <alvherre@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?)  I'd do
that by adding a new function

long
deleteExactDependencyRecords(Oid classId, Oid objectId,
                             Oid refclassId, Oid refobjectId)

and calling that instead of deleteDependencyRecordsFor.

> Also, I suspect you might need a second CCI after the performDeletion
> call, in case the loop iterates?

Can do.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services