Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1h8A1v-0006w0-3T for pgsql-committers@arkaria.postgresql.org; Sun, 24 Mar 2019 20:47:43 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1h8A1t-0005EC-9c for pgsql-committers@arkaria.postgresql.org; Sun, 24 Mar 2019 20:47:41 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1h8A1t-0005E5-3i for pgsql-committers@lists.postgresql.org; Sun, 24 Mar 2019 20:47:41 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1h8A1q-0001mX-Ds for pgsql-committers@lists.postgresql.org; Sun, 24 Mar 2019 20:47:40 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id x2OKlXvr007823; Sun, 24 Mar 2019 16:47:34 -0400 From: Tom Lane To: Peter Geoghegan cc: pgsql-committers Subject: Re: pgsql: Make heap TID a tiebreaker nbtree index column. In-reply-to: References: <13999.1553105283@sss.pgh.pa.us> <24277.1553114810@sss.pgh.pa.us> <16667.1553116435@sss.pgh.pa.us> <17604.1553118255@sss.pgh.pa.us> <17845.1553118645@sss.pgh.pa.us> <8748.1553119737@sss.pgh.pa.us> <21983.1553138224@sss.pgh.pa.us> <5446.1553272377@sss.pgh.pa.us> <6226.1553274783@sss.pgh.pa.us> <4642.1553276304@sss.pgh.pa.us> <22259.1553441524@sss.pgh.pa.us> <27112.1553448806@sss.p! gh.pa.us> <28074.1553450544@sss.pgh.pa.us> Comments: In-reply-to Peter Geoghegan message dated "Sun, 24 Mar 2019 11:27:27 -0700" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <7738.1553460394.0@sss.pgh.pa.us> Date: Sun, 24 Mar 2019 16:47:33 -0400 Message-ID: <7822.1553460453@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <7738.1553460394.1@sss.pgh.pa.us> Peter Geoghegan writes: > On Sun, Mar 24, 2019 at 11:02 AM Tom Lane 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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="stabilize-drop-role-output-too.patch"; charset="us-ascii" Content-ID: <7738.1553460394.2@sss.pgh.pa.us> Content-Description: stabilize-drop-role-output-too.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_sh= depend.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 *nli= st2); 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 =3D (const ShDepObjectInfo *) a; + const ShDepObjectInfo *objb =3D (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) wi= ll + * come first. + */ + if ((unsigned int) obja->object.objectSubId < (unsigned int) objb->obje= ct.objectSubId) + return -1; + if ((unsigned int) obja->object.objectSubId > (unsigned int) objb->obje= ct.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 =3D 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 orde= r + * in which other databases are reported, though. */ #define MAX_REPORTED_DEPS 100 = + allocedobjects =3D 128; /* arbitrary initial array size */ + objects =3D (ShDepObjectInfo *) + palloc(allocedobjects * sizeof(ShDepObjectInfo)); + numobjects =3D 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 =3D=3D 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 =3D=3D 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 =3D=3D MyDatabaseId || ! sdepForm->dbid =3D=3D InvalidOid) { ! if (numobjects >=3D allocedobjects) { ! allocedobjects *=3D 2; ! objects =3D (ShDepObjectInfo *) ! repalloc(objects, ! allocedobjects * sizeof(ShDepObjectInfo)); } ! objects[numobjects].object =3D object; ! objects[numobjects].deptype =3D sdepForm->deptype; ! objects[numobjects].objtype =3D (sdepForm->dbid =3D=3D 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 =3D 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 =3D=3D 0) diff --git a/src/test/regress/expected/dependency.out b/src/test/regress/e= xpected/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_d= ep_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_d= ep_user1 in schema deptest DROP OWNED BY regress_dep_user1; DROP USER regress_dep_user1; ------- =_aaaaaaaaaa0--