Date: Tue, 5 Feb 2019 18:28:29 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: 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: <20190205212829.GA20127@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <16136.1548790124@sss.pgh.pa.us>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On 2019-Jan-29, Tom Lane wrote:

> The closest I could get to solving it along the original lines
> went like this:
> 
>      * In addition, we support INTERNAL_AUTO dependencies, which alter the
>      * rules for this.  If the target has both INTERNAL and INTERNAL_AUTO
>      * dependencies, then it can be dropped if any one of those objects is
>      * dropped, but not unless at least one of them is.  In this situation we
>      * mustn't automatically transform a random deletion request into a
>      * parent-object deletion.  Instead, we proceed with deletion if we are
>      * recursing from any owning object, and otherwise set the object aside to
>      * recheck at the end.  If we don't later find a reason to delete one of
>      * the owning objects, we'll throw an error.
> 
> I think that's ugly as sin: INTERNAL means something different if there's
> another dependency beside it?

I agree it's pretty weird :-(

> A theoretically purer answer is to split INTERNAL_AUTO into two new
> dependency types, one linking to the real "owning" object (the parent
> index or trigger) and the other to the secondary owner (the partition
> table), while leaving regular INTERNAL as it stands.  I don't especially
> want to go that way, though: it seems overcomplicated from the standpoint
> of outside inspections of pg_depend, and it'd be very risky to back-patch
> into v11.  (For instance, if someone went backwards on their postmaster
> minor release, the older v11 version would not know what to do with the
> new dependency type.)

I don't put a lot of value in the idea of people going back in minor
releases, but I agree with the general principle of not making pg_depend
any harder to inspect than it already is.  Even so, I wouldn't dismiss a
solution along these lines, if it was clean enough; we could provide
better tools for pg_depend inspection (some sensible view, at least).
It might make sense to implement this in master only, if it turns out to
be better in some way.  I'm not volunteering right now, but maybe in the
future.

> It strikes me however that we can stick with the existing catalog contents
> by making this definition: of the INTERNAL_AUTO dependencies of a given
> object, the "true owner" to be reported in errors is the dependency
> that is of the same classId as that object.  If this doesn't uniquely
> identify one dependency, the report will mention a random one.  This is
> ugly as well, but it's certainly a lot more practical to cram into v11,
> and it seems like it would cover the current and likely future use-cases
> for partition-related objects.  When and if we find a use-case for which
> this doesn't work, we can think about extending the catalog representation
> to identify a unique owner in a less ad-hoc way.

This seems a great idea.  Do you want me to implement it?

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