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
From 04c8c6ff940c387b164425fe0e0a5ffdbe2a5854 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Mon, 5 Nov 2018 10:27:16 -0800
Subject: [PATCH 8/8] Questionable/tentative ASC regress output fixes
---
contrib/earthdistance/expected/earthdistance.out | 2 +-
src/test/regress/expected/create_view.out | 2 +-
src/test/regress/expected/event_trigger.out | 16 ++++++++--------
src/test/regress/expected/indexing.out | 12 ++++++------
src/test/regress/expected/triggers.out | 12 ++++++------
5 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/contrib/earthdistance/expected/earthdistance.out b/contrib/earthdistance/expected/earthdistance.out
index 26a843c3fa..4395e619de 100644
--- 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)
HINT: Use DROP ... CASCADE to drop the dependent objects too.
drop extension earthdistance;
drop type cube; -- fail, extension cube requires it
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 141fc6da62..8abcd7b3d9 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -1711,4 +1711,4 @@ select pg_get_ruledef(oid, true) from pg_rewrite
DROP SCHEMA temp_view_test CASCADE;
NOTICE: drop cascades to 27 other objects
DROP SCHEMA testviewschm2 CASCADE;
-NOTICE: drop cascades to 62 other objects
+NOTICE: drop cascades to 63 other objects
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 0755931db8..ec4f6c300f 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -406,19 +406,19 @@ DROP INDEX evttrig.one_idx;
NOTICE: NORMAL: orig=t normal=f istemp=f type=index identity=evttrig.one_idx name={evttrig,one_idx} args={}
DROP SCHEMA evttrig CASCADE;
NOTICE: drop cascades to 3 other objects
-DETAIL: drop cascades to table evttrig.one
+DETAIL: drop cascades to table evttrig.parted
drop cascades to table evttrig.two
-drop cascades to table evttrig.parted
+drop cascades to table evttrig.one
NOTICE: NORMAL: orig=t normal=f istemp=f type=schema identity=evttrig name={evttrig} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.parted name={evttrig,parted} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_20 name={evttrig,part_10_20} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_15_20 name={evttrig,part_15_20} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_15 name={evttrig,part_10_15} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_1_10 name={evttrig,part_1_10} args={}
+NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.two name={evttrig,two} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.one name={evttrig,one} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=sequence identity=evttrig.one_col_a_seq name={evttrig,one_col_a_seq} args={}
NOTICE: NORMAL: orig=f normal=t istemp=f type=default value identity=for evttrig.one.col_a name={evttrig,one,col_a} args={}
-NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.two name={evttrig,two} args={}
-NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.parted name={evttrig,parted} args={}
-NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_1_10 name={evttrig,part_1_10} args={}
-NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_20 name={evttrig,part_10_20} args={}
-NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_10_15 name={evttrig,part_10_15} args={}
-NOTICE: NORMAL: orig=f normal=t istemp=f type=table identity=evttrig.part_15_20 name={evttrig,part_15_20} args={}
DROP TABLE a_temp_tbl;
NOTICE: NORMAL: orig=t normal=f istemp=t type=table identity=pg_temp.a_temp_tbl name={pg_temp,a_temp_tbl} args={}
DROP EVENT TRIGGER regress_event_trigger_report_dropped;
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index ca27346f18..17e8d3f136 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -148,8 +148,8 @@ create table idxpart (a int) partition by range (a);
create index on idxpart (a);
create table idxpart1 partition of idxpart for values from (0) to (10);
drop index idxpart1_a_idx; -- no way
-ERROR: cannot drop index idxpart1_a_idx because index idxpart_a_idx requires it
-HINT: You can drop index idxpart_a_idx instead.
+ERROR: cannot drop index idxpart1_a_idx because column a of table idxpart1 requires it
+HINT: You can drop column a of table idxpart1 instead.
drop index idxpart_a_idx; -- both indexes go away
select relname, relkind from pg_class
where relname like 'idxpart%' order by relname;
@@ -998,11 +998,11 @@ select indrelid::regclass, indexrelid::regclass, inhparent::regclass, indisvalid
(3 rows)
drop index idxpart0_pkey; -- fail
-ERROR: cannot drop index idxpart0_pkey because index idxpart_pkey requires it
-HINT: You can drop index idxpart_pkey instead.
+ERROR: cannot drop index idxpart0_pkey because constraint idxpart0_pkey on table idxpart0 requires it
+HINT: You can drop constraint idxpart0_pkey on table idxpart0 instead.
drop index idxpart1_pkey; -- fail
-ERROR: cannot drop index idxpart1_pkey because index idxpart_pkey requires it
-HINT: You can drop index idxpart_pkey instead.
+ERROR: cannot drop index idxpart1_pkey because constraint idxpart1_pkey on table idxpart1 requires it
+HINT: You can drop constraint idxpart1_pkey on table idxpart1 instead.
alter table idxpart0 drop constraint idxpart0_pkey; -- fail
ERROR: cannot drop inherited constraint "idxpart0_pkey" of relation "idxpart0"
alter table idxpart1 drop constraint idxpart1_pkey; -- fail
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 70b7c6eead..ae04a89622 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1896,14 +1896,14 @@ select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
(4 rows)
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.
drop trigger trg1 on trigpart2; -- fail
-ERROR: cannot drop trigger trg1 on table trigpart2 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 trigpart2 because table trigpart2 requires it
+HINT: You can drop table trigpart2 instead.
drop trigger trg1 on trigpart3; -- fail
-ERROR: cannot drop trigger trg1 on table trigpart3 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 trigpart3 because table trigpart3 requires it
+HINT: You can drop table trigpart3 instead.
drop table trigpart2; -- ok, trigger should be gone in that partition
select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
--
2.17.1
