From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages)
Date: Sun, 10 Feb 2019 11:12:58 -0500
Message-ID: <27642.1549815178@sss.pgh.pa.us> (raw)
In-Reply-To: <CAH2-WzkOqS0bGswVwy2jySuia1SG2sa8rsCzHTxogUwKg7a8PQ@mail.gmail.com>
References: <20190205212829.GA20127@alvherre.pgsql>
<4572.1549404528@sss.pgh.pa.us>
<CAH2-WzmRtQLJdC7KSdtwxQ65Yk+_Vi89eVqKJvPOx=Xc7WLLKw@mail.gmail.com>
<20444.1549423295@sss.pgh.pa.us>
<26049.1549516498@sss.pgh.pa.us>
<18885.1549642539@sss.pgh.pa.us>
<CAH2-WzkOqS0bGswVwy2jySuia1SG2sa8rsCzHTxogUwKg7a8PQ@mail.gmail.com>
Peter Geoghegan <pg@bowt.ie> writes:
> On Fri, Feb 8, 2019 at 8:15 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> * The other such issue is a pre-existing bug, which maybe we ought to
>> back-patch, though I can't recall seeing any field reports that seem
>> to match it: after recursing to an internal/extension dependency,
>> we need to ensure that whatever objflags were passed down to our level
>> get applied to the targetObjects entry for the current object.
> Hmm. This seems very subtle to me. Perhaps the comment you've added
> above the new object_address_present_add_flags() call in
> findDependentObjects() ought to explain the "current object gets
> marked with objflags" issue first, while only then mentioning the
> cross-check. The interface that object_address_present_add_flags()
> presents seems kind of odd to me, though I don't doubt that it makes
> sense in the wider context of the code.
How about this comment text?
/*
* The current target object should have been added to
* targetObjects while processing the owning object; but it
* probably got only the flag bits associated with the
* dependency we're looking at. We need to add the objflags
* that were passed to this recursion level, too, else we may
* get a bogus failure in reportDependentObjects (if, for
* example, we were called due to a partition dependency).
*
* If somehow the current object didn't get scheduled for
* deletion, bleat. (That would imply that somebody deleted
* this dependency record before the recursion got to it.)
* Another idea would be to reacquire lock on the current
* object and resume trying to delete it, but it seems not
* worth dealing with the race conditions inherent in that.
*/
>> [ invent separate primary and secondary partition dependencies? ]
> I lean towards changing these on HEAD, ...
Me too.
> ... now that it's clear that
> something very different will be needed for v11.
Just to be be clear, my inclination is to do nothing about this in v11.
It's not apparent to me that any fix is possible given the v11 dependency
data, at least not without downsides that'd likely outweigh the upsides.
We've not seen field complaints about these problems.
> That said, I still don't think that partition_dependency_matches() is
> all that bad, since the state is still right there in the pg_depend
> entry. The main drawback of that overall approach is that it obscures
> a legitimate distinction about dependencies that could be made more
> apparent to somebody looking through raw pg_depend entries.
The thing I don't like about it is that it's not very hard to foresee
cases where the approach will fail to uniquely resolve the primary
dependency. Making the original creator of the dependencies specify
which object is the primary owner seems a lot more future-proof.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pg@bowt.ie, alvherre@2ndquadrant.com
Subject: Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages)
In-Reply-To: <27642.1549815178@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox