From 85d8c3dd50f3042f74e3d4e8b0ab67b5b038375d Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Tue, 13 Nov 2018 18:14:23 -0800
Subject: [PATCH v9 1/7] Add pg_depend index scan tiebreaker column.

findDependentObjects() and other code that scans pg_depend evolved to
depend on pg_depend_depender_index and pg_depend_reference_index scan
order.  This is clear from running the regression tests with
"ignore_system_indexes=on": much of the test output changes for
regression tests that happen to have DROP diagnostic messages.  More
importantly, a small number of those test failures involve alternative
messages that are objectively useless or even harmful.  These
regressions all involve internal dependencies of one form or another.
For example, we can go from a HINT that suggests dropping a partitioning
parent table's trigger in order to drop the child's trigger to a HINT
that suggests simply dropping the child table.  This HINT is technically
still correct from the point of view of findDependentObjects(), and yet
it's clearly not the intended user-visible behavior.

Make dependency.c take responsibility for its dependency on scan order
by commenting on it directly.  Ensure that the behavior of
findDependentObjects() is deterministic in the event of duplicates by
adding a new per-backend sequentially assigned number column to
pg_depend.  Both indexes now use this new column as a final trailing
attribute, effectively making it a tiebreaker wherever processing order
happens to matter.  The new column is a per-backend sequentially
assigned number.  New values are assigned in decreasing order.  That
produces the behavior that's already expected from nbtree scans in the
event of duplicate index entries (nbtree usually leaves duplicate index
entries in reverse insertion order at present).  A similar new column is
not needed for pg_shdepend because the aforementioned harmful changes
only occur with cases involving internal dependencies.

The overall effect is to stabilize the behavior of DROP diagnostic
messages, making it possible to avoid the "\set VERBOSITY=terse" hack
that has been used to paper over test instability in the past.  We may
wish to go through the regression tests and remove existing instances of
the "\set VERBOSITY=terse" hack (see also: commit 8e753726), but that's
left for later.

An upcoming patch to make nbtree store duplicate entries in a
well-defined order by treating heap TID as a tiebreaker tuple attribute
more or less flips the order that duplicates appear (the order will
change from approximately descending to perfectly ascending).  That
would cause all sorts of problems for findDependentObjects() callers if
this groundwork was skipped.

Note that adding the new column has no appreciable storage overhead.
pg_depend indexes are made no larger, at least on 64-bit platforms,
because values can fit in a hole that was previously unused due to
alignment -- both pg_depend_depender_index and pg_depend_reference_index
continue to have 24 byte IndexTuples.  There is also no change in the
on-disk size of pg_depend heap relations on 64-bit platforms, for the
same reason.  The MAXALIGN()'d size of pg_depend heap tuples remains 56
bytes (including tuple header overhead).

Discussion: https://postgr.es/m/CAH2-Wzkypv1R+teZrr71U23J578NnTBt2X8+Y=Odr4pOdW1rXg@mail.gmail.com
Discussion: https://postgr.es/m/11852.1501610262%40sss.pgh.pa.us
---
 doc/src/sgml/catalogs.sgml                | 17 ++++++++-
 src/backend/catalog/dependency.c          | 10 ++++++
 src/backend/catalog/pg_depend.c           | 11 +++++-
 src/bin/initdb/initdb.c                   | 44 +++++++++++------------
 src/include/catalog/indexing.h            |  4 +--
 src/include/catalog/pg_depend.h           |  1 +
 src/test/regress/expected/alter_table.out |  2 +-
 src/test/regress/expected/misc_sanity.out |  4 +--
 8 files changed, 64 insertions(+), 29 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 18c38e42de..cef6e40cad 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2937,6 +2937,20 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
       </entry>
      </row>
 
+     <row>
+      <entry><structfield>depcreate</structfield></entry>
+      <entry><type>int4</type></entry>
+      <entry></entry>
+      <entry>
+       A per-backend sequentially assigned number for this dependency
+       relationship.  Used as a tiebreaker in the event of multiple
+       internal dependency relationships of otherwise equal
+       precedence.  Identifiers are assigned in descending order to
+       ensure that the most recently entered dependency is the one
+       referenced by <literal>HINT</literal> fields.
+      </entry>
+     </row>
+
     </tbody>
    </tgroup>
   </table>
@@ -3054,7 +3068,8 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
        that the system itself depends on the referenced object, and so
        that object must never be deleted.  Entries of this type are
        created only by <command>initdb</command>.  The columns for the
-       dependent object contain zeroes.
+       dependent object and the <structfield>depcreate</structfield>
+       column contain zeroes.
       </para>
      </listitem>
     </varlistentry>
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 7dfa3278a5..d7889c2cd0 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -532,6 +532,11 @@ findDependentObjects(const ObjectAddress *object,
 	else
 		nkeys = 2;
 
+	/*
+	 * Note that we rely on DependDependerIndexId scan order to make
+	 * diagnostic messages deterministic.  (e.g., objsubid = 0 entries will be
+	 * processed before other entries for the same dependent object.)
+	 */
 	scan = systable_beginscan(*depRel, DependDependerIndexId, true,
 							  NULL, nkeys, key);
 
@@ -727,6 +732,11 @@ findDependentObjects(const ObjectAddress *object,
 	else
 		nkeys = 2;
 
+	/*
+	 * Note that we rely on DependReferenceIndexId scan order to make
+	 * diagnostic messages deterministic.  (e.g., refobjsubid = 0 entries will
+	 * be processed before other entries for the same referenced object.)
+	 */
 	scan = systable_beginscan(*depRel, DependReferenceIndexId, true,
 							  NULL, nkeys, key);
 
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 2ea05f350b..d7a32c05f4 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -29,6 +29,8 @@
 #include "utils/rel.h"
 #include "utils/tqual.h"
 
+/* Per-backend pg_depend tiebreaker value */
+static int32 depcreate = PG_INT32_MAX;
 
 static bool isObjectPinned(const ObjectAddress *object, Relation rel);
 
@@ -93,7 +95,10 @@ recordMultipleDependencies(const ObjectAddress *depender,
 		{
 			/*
 			 * Record the Dependency.  Note we don't bother to check for
-			 * duplicate dependencies; there's no harm in them.
+			 * duplicate dependencies; there's no harm in them.  Note that
+			 * depcreate ensures deterministic processing among dependencies
+			 * of otherwise equal precedence (e.g., among multiple entries of
+			 * the same refclassid + refobjid + refobjsubid).
 			 */
 			values[Anum_pg_depend_classid - 1] = ObjectIdGetDatum(depender->classId);
 			values[Anum_pg_depend_objid - 1] = ObjectIdGetDatum(depender->objectId);
@@ -104,6 +109,7 @@ recordMultipleDependencies(const ObjectAddress *depender,
 			values[Anum_pg_depend_refobjsubid - 1] = Int32GetDatum(referenced->objectSubId);
 
 			values[Anum_pg_depend_deptype - 1] = CharGetDatum((char) behavior);
+			values[Anum_pg_depend_depcreate - 1] = Int32GetDatum(depcreate--);
 
 			tup = heap_form_tuple(dependDesc->rd_att, values, nulls);
 
@@ -114,6 +120,9 @@ recordMultipleDependencies(const ObjectAddress *depender,
 			CatalogTupleInsertWithInfo(dependDesc, tup, indstate);
 
 			heap_freetuple(tup);
+			/* avoid signed underflow */
+			if (depcreate == PG_INT32_MIN)
+				depcreate = PG_INT32_MAX;
 		}
 	}
 
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 211a96380e..3679c5f8aa 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1619,55 +1619,55 @@ setup_depend(FILE *cmdfd)
 		"DELETE FROM pg_shdepend;\n\n",
 		"VACUUM pg_shdepend;\n\n",
 
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_class;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_proc;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_type;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_cast;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_constraint;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_conversion;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_attrdef;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_language;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_operator;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_opclass;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_opfamily;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_am;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_amop;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_amproc;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_rewrite;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_trigger;\n\n",
 
 		/*
 		 * restriction here to avoid pinning the public namespace
 		 */
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_namespace "
 		"    WHERE nspname LIKE 'pg%';\n\n",
 
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_ts_parser;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_ts_dict;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_ts_template;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_ts_config;\n\n",
-		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
+		"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p',0 "
 		" FROM pg_collation;\n\n",
 		"INSERT INTO pg_shdepend SELECT 0,0,0,0, tableoid,oid, 'p' "
 		" FROM pg_authid;\n\n",
diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h
index 2359b4c629..1ee7149a49 100644
--- a/src/include/catalog/indexing.h
+++ b/src/include/catalog/indexing.h
@@ -142,9 +142,9 @@ DECLARE_UNIQUE_INDEX(pg_database_datname_index, 2671, on pg_database using btree
 DECLARE_UNIQUE_INDEX(pg_database_oid_index, 2672, on pg_database using btree(oid oid_ops));
 #define DatabaseOidIndexId	2672
 
-DECLARE_INDEX(pg_depend_depender_index, 2673, on pg_depend using btree(classid oid_ops, objid oid_ops, objsubid int4_ops));
+DECLARE_INDEX(pg_depend_depender_index, 2673, on pg_depend using btree(classid oid_ops, objid oid_ops, objsubid int4_ops, depcreate int4_ops));
 #define DependDependerIndexId  2673
-DECLARE_INDEX(pg_depend_reference_index, 2674, on pg_depend using btree(refclassid oid_ops, refobjid oid_ops, refobjsubid int4_ops));
+DECLARE_INDEX(pg_depend_reference_index, 2674, on pg_depend using btree(refclassid oid_ops, refobjid oid_ops, refobjsubid int4_ops, depcreate int4_ops));
 #define DependReferenceIndexId	2674
 
 DECLARE_UNIQUE_INDEX(pg_description_o_c_o_index, 2675, on pg_description using btree(objoid oid_ops, classoid oid_ops, objsubid int4_ops));
diff --git a/src/include/catalog/pg_depend.h b/src/include/catalog/pg_depend.h
index 8f2d95210f..47cb387a6f 100644
--- a/src/include/catalog/pg_depend.h
+++ b/src/include/catalog/pg_depend.h
@@ -61,6 +61,7 @@ CATALOG(pg_depend,2608,DependRelationId)
 	 * field.  See DependencyType in catalog/dependency.h.
 	 */
 	char		deptype;		/* see codes in dependency.h */
+	int32		depcreate;		/* per-backend identifier; tiebreaker */
 } FormData_pg_depend;
 
 /* ----------------
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 38ede0a473..f9d4d38b3d 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2561,10 +2561,10 @@ DETAIL:  drop cascades to table alter2.t1
 drop cascades to view alter2.v1
 drop cascades to function alter2.plus1(integer)
 drop cascades to type alter2.posint
-drop cascades to operator family alter2.ctype_hash_ops for access method hash
 drop cascades to type alter2.ctype
 drop cascades to function alter2.same(alter2.ctype,alter2.ctype)
 drop cascades to operator alter2.=(alter2.ctype,alter2.ctype)
+drop cascades to operator family alter2.ctype_hash_ops for access method hash
 drop cascades to conversion alter2.ascii_to_utf8
 drop cascades to text search parser alter2.prs
 drop cascades to text search configuration alter2.cfg
diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out
index 1d4b000acf..6ee8a9424f 100644
--- a/src/test/regress/expected/misc_sanity.out
+++ b/src/test/regress/expected/misc_sanity.out
@@ -18,8 +18,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR
       deptype NOT IN ('a', 'e', 'i', 'n', 'p') OR
       (deptype != 'p' AND (classid = 0 OR objid = 0)) OR
       (deptype = 'p' AND (classid != 0 OR objid != 0 OR objsubid != 0));
- classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype 
----------+-------+----------+------------+----------+-------------+---------
+ classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype | depcreate 
+---------+-------+----------+------------+----------+-------------+---------+-----------
 (0 rows)
 
 -- **************** pg_shdepend ****************
-- 
2.17.1

