Re: Fixing findDependentObjects()'s dependency on scan order(regressions in DROP diagnostic messages) - Mailing list pgsql-hackers

On Fri, Jan 18, 2019 at 3:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I think the tiebreaker idea is just a hack, because it'd only be stable
> to the extent that the added tiebreaker values are stable, and they
> wouldn't be very much so if the counter state is only kept per-backend.
>
> Attached is a draft patch to sort objects before the recursion step
> in findDependentObjects.  I found that sorting by descending OID is
> really the right thing; if we sort by increasing OID then we get a
> whole lot more diffs in the DROP CASCADE output.  As shown, there
> are just a few such diffs, and many of them seem to be for the better
> anyway.

This reminds me of the output that I saw back when my patch used DESC
heap TID order. I agree that those regression test changes are
improvements. I think that they're caused by the existing nbtree
code's preference for storing duplicates on the first leaf page it
could go on that is found to be empty. It stores duplicates in
approximately descending order, but now you're artificially forcing
perfectly descending order -- one or two things that were not in the
expected (reverse) chronological order, by accident, now appear in
order.

(I've not verified this explanation, though.)

> I did not do anything here about reverting the various hacks to
> suppress DROP CASCADE printouts in specific regression tests.
> I'm not very sure whether doing so would be useful or not.

I doubt it myself. We can always change it later on.

> Testing this under ignore_system_indexes, it fixes most of the
> cases where the output changes, but there are still two categories
> it doesn't fix:
>
> * Objects-to-drop output from DROP ROLE is still unstable.  I suppose
> this would be fixed by also doing sorting in that code path, but
> I've not looked into it.

The nbtree patch is not dependent on doing better here, since the
order here is merely unstable, without leading to incorrect diagnostic
messages. I can just change the regress output mechanically. That
said, I cannot be sure that the instability won't become more of an
annoyance with heap TID being treated as a tie-breaker attribute
within nbtree. It's probably fine to reserve the option to do better
here, and do so if and when it becomes clear that it matters. I defer
to you.

> * There is still instability in which object you get told to drop
> when attempting to drop an index partition or trigger, as a consequence
> of there being two possible DEPENDENCY_INTERNAL_AUTO targets.  I still
> feel that the right fix there involves changing the design for what
> dependency types we store, but I've not worked on it yet.

I thought that your ALTER OBJECT DEPENDS ON EXTENSION example made the
case for fixing that directly inarguable. I'm slightly surprised that
you're not fully convinced of this already. Have I missed some
subtlety?

Thanks
-- 
Peter Geoghegan