Re: pgsql: Make heap TID a tiebreaker nbtree index column.

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

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;