In my opinion, your patch detected three problems:
1. Unsteady order of query results/system messages ('DROP...CASCADE'
detects it).
2. Hide info about a child object dropping ('drop cascades to 62
other objects' detects it).
3. Possible non-informative messages about dependencies ('drop trigger
trg1' detects it)
Problem No. 1 will be amplified with new asynchronous operations,
background workers and distributing query execution. It is not problem
of DBMS. The solution is change the tests: include sorting of query
results, sorting of system messages before diff operation.
If steady order of messages is critical for users we can sort
targetObjects array in the begin of reportDependentObjects() routine by
classId, objectId and objectSubId fields.
Problem No. 2: we can suppress some output optimizations in
object_address_present_add_flags() routine and print all deleted objects.
Problem No. 3: I suppose we can go one of two ways:
a) print all depended objects returned by scan of DependDependerIndexId
relation, not only the first.
b) search a root of dependence and print only it.
On 06.11.2018 5:04, Peter Geoghegan wrote:
I've realized that my patch to make nbtree keys unique by treating
heap TID as a tie-breaker attribute must use ASC ordering, for reasons
that I won't go into here. Now that I'm not using DESC ordering, there
are changes to a small number of DROP...CASCADE messages that leave
users with something much less useful than what they'll see today --
see attached patch for full details. Some of these problematic cases
involve partitioning:
"""
create table trigpart (a int, b int) partition by range (a);
create table trigpart1 partition of trigpart for values from (0) to (1000);
create trigger trg1 after insert on trigpart for each row execute
procedure trigger_nothing();
...
drop trigger trg1 on trigpart1; -- fail
-ERROR: cannot drop trigger trg1 on table trigpart1 because trigger
trg1 on table trigpart requires it
-HINT: You can drop trigger trg1 on table trigpart instead.
+ERROR: cannot drop trigger trg1 on table trigpart1 because table
trigpart1 requires it
+HINT: You can drop table trigpart1 instead.
"""
As you can see, the original hint suggests "you need to drop the
object on the partition parent instead of its child", which is useful.
The new hint suggests "instead of dropping the trigger on the
partition child, maybe drop the child itself!", which is less than
useless. This is a problem that needs to be treated as a prerequisite
to committing the nbtree patch, so I'd like to get it out of the way
soon.
The high level issue is that findDependentObjects() relies on the scan
order of duplicates within the
DependDependerIndexId/pg_depend_depender_index index in a way that
nbtree doesn't actually guarantee, and never has guaranteed. As I've
shown, findDependentObjects()'s assumptions around where nbtree will
leave duplicates accidentally affects the quality of various
diagnostic messages. My example also breaks with
ignore_system_indexes=on, even on the master branch, so technically
this isn't a new problem.
I've looked into a way to fix findDependentObjects(). As far as I can
tell, I can fix issues by adding a kludgey special case along these
lines:
1 diff --git a/src/backend/catalog/dependency.c
b/src/backend/catalog/dependency.c
2 index 7dfa3278a5..7454d4e6f8 100644
3 --- a/src/backend/catalog/dependency.c
4 +++ b/src/backend/catalog/dependency.c
5 @@ -605,6 +605,15 @@ findDependentObjects(const ObjectAddress *object,
6 ReleaseDeletionLock(object);
7 return;
8 }
9 + /*
10 + * Assume that another pg_depend entry more suitably
11 + * represents dependency when an entry for a partition
12 + * child's index references a column of the partition
13 + * itself.
14 + */
15 + if (foundDep->deptype == DEPENDENCY_INTERNAL_AUTO &&
16 + otherObject.objectSubId != 0)
17 + break;
This is obviously brittle, but maybe it hints at a better approach.
Notably, it doesn't fix other similar issues, such as this:
--- a/contrib/earthdistance/expected/earthdistance.out
+++ b/contrib/earthdistance/expected/earthdistance.out
@@ -972,7 +972,7 @@ SELECT abs(cube_distance(ll_to_earth(-30,-90),
'(0)'::cube) / earth() - 1) <
drop extension cube; -- fail, earthdistance requires it
ERROR: cannot drop extension cube because other objects depend on it
-DETAIL: extension earthdistance depends on extension cube
+DETAIL: extension earthdistance depends on function cube_out(cube)
Can anyone think of a workable, scalable approach to fixing the
processing order of this findDependentObjects() pg_depend scan so that
we reliably get the user-visible behavior we already tacitly expect?
--
Peter Geoghegan
--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company
