Subject: Re: Fixing findDependentObjects()'s dependency on scan order
 (regressions in DROP diagnostic messages)
To: Peter Geoghegan <pg@bowt.ie>,
 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: 
 <CAH2-Wzkypv1R+teZrr71U23J578NnTBt2X8+Y=Odr4pOdW1rXg@mail.gmail.com>
 <0a89eb40-ea89-9037-aad0-3b65c67cdcbd@postgrespro.ru>
 <CAH2-Wz=WRpJeej2xpd+FbzJXYAZZPiTgebTGv1P2NMcvgzvZGw@mail.gmail.com>
 <CAH2-WzmQ7XeKspNyA+K4iRMaVcqquq8G1qtvfK_grdKV-1Oa9w@mail.gmail.com>
From: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
Organization: Postgres Professional
Message-ID: <65ced322-84d2-777a-2fac-5f482c175431@postgrespro.ru>
Date: Thu, 6 Dec 2018 11:35:29 +0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: 
 <CAH2-WzmQ7XeKspNyA+K4iRMaVcqquq8G1qtvfK_grdKV-1Oa9w@mail.gmail.com>
Content-Type: multipart/mixed;
 boundary="------------2F243136085225DBA429D7A2"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------2F243136085225DBA429D7A2
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

On 14.11.2018 11:28, Peter Geoghegan wrote:
> We're already relying on the scan order being in reverse chronological
> order, so we might as well formalize the dependency. I don't think
> that it's possible to sort the pg_depend entries as a way of fixing
> the breakage while avoiding storing this extra information -- what is
> there to sort on that's there already? You'd have to infer a whole
> bunch of things about the object types associated with pg_depend
> entries to do that, and teach dependency.c about its callers. That
> seems pretty brittle to me.

This solution changes pg_depend relation for solve a problem, which 
exists only in regression tests.  Very rarely it can be in the 
partitioning cases. Or is it not?
I think this decision is some excessive.
May be you consider another approach:
1. Order of dependencies in 'DROP ... CASCADE' case is a problem of test 
tools, not DBMS. And here we can use 'verbose terse'.
2. Print all dependencies in findDependentObjects() on a drop error (see 
attachment as a prototype).

-- 
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company

--------------2F243136085225DBA429D7A2
Content-Type: text/x-patch;
 name="0001-Print-All-dependencies-on-error.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0001-Print-All-dependencies-on-error.patch"

From e6056363889a00699fcb6ef6ef8ce9bc3e007d7e Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>
Date: Thu, 6 Dec 2018 11:09:17 +0500
Subject: [PATCH] Print All dependencies on error

---
 src/backend/catalog/dependency.c | 59 ++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 7dfa3278a5..e84143af2a 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -595,7 +595,9 @@ findDependentObjects(const ObjectAddress *object,
 				 */
 				if (stack == NULL)
 				{
-					char	   *otherObjDesc;
+					char		*emsg = NULL;
+					int			emsgbufsize = 1024;
+					char		*depObjDesc;
 
 					if (pendingObjects &&
 						object_address_present(&otherObject, pendingObjects))
@@ -605,14 +607,57 @@ findDependentObjects(const ObjectAddress *object,
 						ReleaseDeletionLock(object);
 						return;
 					}
-					otherObjDesc = getObjectDescription(&otherObject);
+
+					do
+					{
+						ObjectAddress depObject;
+						Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(tup);
+
+						depObject.classId = foundDep->refclassid;
+						depObject.objectId = foundDep->refobjid;
+						depObject.objectSubId = foundDep->refobjsubid;
+
+						if (object_address_present(&depObject, pendingObjects))
+							continue;
+
+						if ((foundDep->deptype == DEPENDENCY_EXTENSION) &&
+							((flags & PERFORM_DELETION_SKIP_EXTENSIONS) ||
+							 (creating_extension &&
+							  depObject.classId == ExtensionRelationId &&
+							  depObject.objectId == CurrentExtensionObject))
+						   )
+							continue;
+
+						if ((foundDep->deptype != DEPENDENCY_INTERNAL) &&
+							(foundDep->deptype != DEPENDENCY_INTERNAL_AUTO)
+							)
+							continue;
+
+						depObjDesc = getObjectDescription(&depObject);
+						if (emsg == NULL)
+							emsg = palloc0(emsgbufsize);
+						else
+							strcat(emsg, " or ");
+
+						if (strlen(depObjDesc) > emsgbufsize - strlen(emsg))
+						{
+							char	*tmpstr;
+
+							emsgbufsize += strlen(depObjDesc) + 4;
+							tmpstr = palloc0(emsgbufsize);
+							memcpy(tmpstr, emsg, strlen(emsg));
+							pfree(emsg);
+							emsg = tmpstr;
+						}
+						strcat(emsg, depObjDesc);
+					} while (HeapTupleIsValid(tup = systable_getnext(scan)));
+
 					ereport(ERROR,
 							(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
-							 errmsg("cannot drop %s because %s requires it",
-									getObjectDescription(object),
-									otherObjDesc),
-							 errhint("You can drop %s instead.",
-									 otherObjDesc)));
+							 errmsg("cannot drop %s because another object requires it",
+									getObjectDescription(object)),
+							 errhint("You can try to drop one of the following: %s.",
+									 emsg)));
 				}
 
 				/*
-- 
2.17.1


--------------2F243136085225DBA429D7A2--