Peter Geoghegan <p...@bowt.ie> writes:
> On Sun, Mar 24, 2019 at 11:02 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
>> I think I can probably get that done today, but if you don't want to
>> wait, feel free to put in the detail-suppression for now.
> I'll monitor the situation, and proceed with a stopgap
> detail-suppression fix this evening if, for whatever reason, it seems
> necessary.
It turns out we can't use the ObjectAddresses infrastructure because
it doesn't store enough information. So that means an extra qsort
comparator function, which is slightly annoying, but it's still not
a huge amount of code.
This flips one expected result to another order from what it was.
(I experimented with OID-descending sort, but that flips two
other expected results.)
If no objections, I'll push shortly.
regards, tom lane
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 63a7f48..5b74040 100644
*** a/src/backend/catalog/pg_shdepend.c
--- b/src/backend/catalog/pg_shdepend.c
*************** typedef enum
*** 74,79 ****
--- 74,86 ----
REMOTE_OBJECT
} SharedDependencyObjectType;
+ typedef struct
+ {
+ ObjectAddress object;
+ char deptype;
+ SharedDependencyObjectType objtype;
+ } ShDepObjectInfo;
+
static void getOidListDiff(Oid *list1, int *nlist1, Oid *list2, int *nlist2);
static Oid classIdGetDbId(Oid classId);
static void shdepChangeDep(Relation sdepRel,
*************** typedef struct
*** 497,502 ****
--- 504,548 ----
} remoteDep;
/*
+ * qsort comparator for ShDepObjectInfo items
+ */
+ static int
+ shared_dependency_comparator(const void *a, const void *b)
+ {
+ const ShDepObjectInfo *obja = (const ShDepObjectInfo *) a;
+ const ShDepObjectInfo *objb = (const ShDepObjectInfo *) b;
+
+ /*
+ * Primary sort key is OID ascending.
+ */
+ if (obja->object.objectId < objb->object.objectId)
+ return -1;
+ if (obja->object.objectId > objb->object.objectId)
+ return 1;
+
+ /*
+ * Next sort on catalog ID, in case identical OIDs appear in different
+ * catalogs. Sort direction is pretty arbitrary here.
+ */
+ if (obja->object.classId < objb->object.classId)
+ return -1;
+ if (obja->object.classId > objb->object.classId)
+ return 1;
+
+ /*
+ * Last, sort on object subId.
+ *
+ * We sort the subId as an unsigned int so that 0 (the whole object) will
+ * come first.
+ */
+ if ((unsigned int) obja->object.objectSubId < (unsigned int) objb->object.objectSubId)
+ return -1;
+ if ((unsigned int) obja->object.objectSubId > (unsigned int) objb->object.objectSubId)
+ return 1;
+ return 0;
+ }
+
+ /*
* checkSharedDependencies
*
* Check whether there are shared dependency entries for a given shared
*************** checkSharedDependencies(Oid classId, Oid
*** 531,536 ****
--- 577,585 ----
List *remDeps = NIL;
ListCell *cell;
ObjectAddress object;
+ ShDepObjectInfo *objects;
+ int numobjects;
+ int allocedobjects;
StringInfoData descs;
StringInfoData alldescs;
*************** checkSharedDependencies(Oid classId, Oid
*** 538,546 ****
--- 587,603 ----
* We limit the number of dependencies reported to the client to
* MAX_REPORTED_DEPS, since client software may not deal well with
* enormous error strings. The server log always gets a full report.
+ *
+ * For stability of regression test results, we sort local and shared
+ * objects by OID before reporting them. We don't worry about the order
+ * in which other databases are reported, though.
*/
#define MAX_REPORTED_DEPS 100
+ allocedobjects = 128; /* arbitrary initial array size */
+ objects = (ShDepObjectInfo *)
+ palloc(allocedobjects * sizeof(ShDepObjectInfo));
+ numobjects = 0;
initStringInfo(&descs);
initStringInfo(&alldescs);
*************** checkSharedDependencies(Oid classId, Oid
*** 580,615 ****
/*
* If it's a dependency local to this database or it's a shared
! * object, describe it.
*
* If it's a remote dependency, keep track of it so we can report the
* number of them later.
*/
! if (sdepForm->dbid == MyDatabaseId)
! {
! if (numReportedDeps < MAX_REPORTED_DEPS)
! {
! numReportedDeps++;
! storeObjectDescription(&descs, LOCAL_OBJECT, &object,
! sdepForm->deptype, 0);
! }
! else
! numNotReportedDeps++;
! storeObjectDescription(&alldescs, LOCAL_OBJECT, &object,
! sdepForm->deptype, 0);
! }
! else if (sdepForm->dbid == InvalidOid)
{
! if (numReportedDeps < MAX_REPORTED_DEPS)
{
! numReportedDeps++;
! storeObjectDescription(&descs, SHARED_OBJECT, &object,
! sdepForm->deptype, 0);
}
! else
! numNotReportedDeps++;
! storeObjectDescription(&alldescs, SHARED_OBJECT, &object,
! sdepForm->deptype, 0);
}
else
{
--- 637,662 ----
/*
* If it's a dependency local to this database or it's a shared
! * object, add it to the objects array.
*
* If it's a remote dependency, keep track of it so we can report the
* number of them later.
*/
! if (sdepForm->dbid == MyDatabaseId ||
! sdepForm->dbid == InvalidOid)
{
! if (numobjects >= allocedobjects)
{
! allocedobjects *= 2;
! objects = (ShDepObjectInfo *)
! repalloc(objects,
! allocedobjects * sizeof(ShDepObjectInfo));
}
! objects[numobjects].object = object;
! objects[numobjects].deptype = sdepForm->deptype;
! objects[numobjects].objtype = (sdepForm->dbid == MyDatabaseId) ?
! LOCAL_OBJECT : SHARED_OBJECT;
! numobjects++;
}
else
{
*************** checkSharedDependencies(Oid classId, Oid
*** 648,653 ****
--- 695,727 ----
table_close(sdepRel, AccessShareLock);
/*
+ * Sort and report local and shared objects.
+ */
+ if (numobjects > 1)
+ qsort((void *) objects, numobjects,
+ sizeof(ShDepObjectInfo), shared_dependency_comparator);
+
+ for (int i = 0; i < numobjects; i++)
+ {
+ if (numReportedDeps < MAX_REPORTED_DEPS)
+ {
+ numReportedDeps++;
+ storeObjectDescription(&descs,
+ objects[i].objtype,
+ &objects[i].object,
+ objects[i].deptype,
+ 0);
+ }
+ else
+ numNotReportedDeps++;
+ storeObjectDescription(&alldescs,
+ objects[i].objtype,
+ &objects[i].object,
+ objects[i].deptype,
+ 0);
+ }
+
+ /*
* Summarize dependencies in remote databases.
*/
foreach(cell, remDeps)
*************** checkSharedDependencies(Oid classId, Oid
*** 670,675 ****
--- 744,750 ----
SHARED_DEPENDENCY_INVALID, dep->count);
}
+ pfree(objects);
list_free_deep(remDeps);
if (descs.len == 0)
diff --git a/src/test/regress/expected/dependency.out b/src/test/regress/expected/dependency.out
index 8d31110..2f04b71 100644
*** a/src/test/regress/expected/dependency.out
--- b/src/test/regress/expected/dependency.out
*************** FROM pg_type JOIN pg_class c ON typrelid
*** 128,135 ****
-- doesn't work: grant still exists
DROP USER regress_dep_user1;
ERROR: role "regress_dep_user1" cannot be dropped because some objects depend on it
! DETAIL: privileges for table deptest1
! privileges for database regression
owner of default privileges on new relations belonging to role regress_dep_user1 in schema deptest
DROP OWNED BY regress_dep_user1;
DROP USER regress_dep_user1;
--- 128,135 ----
-- doesn't work: grant still exists
DROP USER regress_dep_user1;
ERROR: role "regress_dep_user1" cannot be dropped because some objects depend on it
! DETAIL: privileges for database regression
! privileges for table deptest1
owner of default privileges on new relations belonging to role regress_dep_user1 in schema deptest
DROP OWNED BY regress_dep_user1;
DROP USER regress_dep_user1;
