From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
Cc: pgsql-committers <pgsql-committers@lists.postgresql.org>
Subject: Re: pgsql: Make heap TID a tiebreaker nbtree index column.
Date: Sun, 24 Mar 2019 16:47:33 -0400
Message-ID: <7822.1553460453@sss.pgh.pa.us> (raw)
In-Reply-To: <CAH2-Wz=yNk=Ue1G8ySED3qsaSUD_g6EQ0KNeZZA808=5EYProA@mail.gmail.com>
References: <E1h6eep-0001Mw-Vd@gemulon.postgresql.org>
<CAH2-WzmFdW-e9+uG5BqzuW7YWBdL9=Vrwgmt5p6vndCeKJ5Z4w@mail.gmail.com>
<13999.1553105283@sss.pgh.pa.us>
<CAH2-Wznu+i765yrRcpKC0m9N2LEFWg=stjF73U7XmakSXWDDAg@mail.gmail.com>
<CAH2-WzkCQ_MtKeOpzozj7QhhgP1unXsK8o9DMAFvDqQFEPpkYQ@mail.gmail.com>
<24277.1553114810@sss.pgh.pa.us>
<16667.1553116435@sss.pgh.pa.us>
<17604.1553118255@sss.pgh.pa.us>
<CAH2-WzmfxXs3bSX6T3DP1wTEWbyhkTiyPZ6o9t2DmcHbBtsCNA@mail.gmail.com>
<17845.1553118645@sss.pgh.pa.us>
<8748.1553119737@sss.pgh.pa.us>
<CAH2-Wz=U8tcaP1wx4t2n8-grz-9ar2fPJeYu9MExSyOu_NVmiA@mail.gmail.com>
<CAH2-Wzm0nQyV3x06_XEuH62tda8X6op-4r29uj8VA0XambsUpw@mail.gmail.com>
<21983.1553138224@sss.pgh.pa.us>
<CAH2-WzmMakut=D8=TnmzhGtB+ELbb6k84LYNW28C8MZ2D-ZoJw@mail.gmail.com>
<5446.1553272377@sss.pgh.pa.us>
<6226.1553274783@sss.pgh.pa.us>
<CAH2-Wzny8NVzgvx=n6nBE9xNEf-W5o=ri2Dq3kspYmgDHtrHDw@mail.gmail.com>
<4642.1553276304@sss.pgh.pa.us>
<22259.1553441524@sss.pgh.pa.us>
<27112.1553448806@sss.p! gh.pa.us>
<CAH2-WzkYwR_KDMyWgyxu9H3-7w4AV=w7ehHuh0TL6J8xR-aa7g@mail.gmail.com>
<28074.1553450544@sss.pgh.pa.us>
<CAH2-Wz=yNk=Ue1G8ySED3qsaSUD_g6EQ0KNeZZA808=5EYProA@mail.gmail.com>
Peter Geoghegan <pg@bowt.ie> writes:
> On Sun, Mar 24, 2019 at 11:02 AM Tom Lane <tgl@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
Attachments:
[text/x-diff] stabilize-drop-role-output-too.patch (6.9K, ../7822.1553460453@sss.pgh.pa.us/2-stabilize-drop-role-output-too.patch)
download | inline diff:diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.cindex 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.outindex 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;
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-committers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pg@bowt.ie, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Make heap TID a tiebreaker nbtree index column.
In-Reply-To: <7822.1553460453@sss.pgh.pa.us>
* 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