From: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
To: Peter Geoghegan <pg@bowt.ie>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
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: Thu, 6 Dec 2018 11:35:29 +0500
Message-ID: <65ced322-84d2-777a-2fac-5f482c175431@postgrespro.ru> (raw)
In-Reply-To: <CAH2-WzmQ7XeKspNyA+K4iRMaVcqquq8G1qtvfK_grdKV-1Oa9w@mail.gmail.com>
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>
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
Attachments:
[text/x-patch] 0001-Print-All-dependencies-on-error.patch (2.8K, ../65ced322-84d2-777a-2fac-5f482c175431@postgrespro.ru/2-0001-Print-All-dependencies-on-error.patch)
download | inline diff:
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.cindex 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
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: a.lepikhov@postgrespro.ru, pg@bowt.ie, peter.eisentraut@2ndquadrant.com, alvherre@2ndquadrant.com
Subject: Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages)
In-Reply-To: <65ced322-84d2-777a-2fac-5f482c175431@postgrespro.ru>
* 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