Handle extension members when first setting object dump flags in pg_dump.

pg_dump's original approach to handling extension member objects was to
run around and clear (or set) their dump flags rather late in its data
collection process.  Unfortunately, quite a lot of code expects those flags
to be valid before that; which was an entirely reasonable expectation
before we added extensions.  In particular, this explains Karsten Hilbert's
recent report of pg_upgrade failing on a database in which an extension
has been installed into the pg_catalog schema.  Its objects are initially
marked as not-to-be-dumped on the strength of their schema, and later we
change them to must-dump because we're doing a binary upgrade of their
extension; but we've already skipped essential tasks like making associated
DO_SHELL_TYPE objects.

To fix, collect extension membership data first, and incorporate it in the
initial setting of the dump flags, so that those are once again correct
from the get-go.  This has the undesirable side effect of slightly
lengthening the time taken before pg_dump acquires table locks, but testing
suggests that the increase in that window is not very much.

Along the way, get rid of ugly special-case logic for deciding whether
to dump procedural languages, FDWs, and foreign servers; dump decisions
for those are now correct up-front, too.

In 9.3 and up, this also fixes erroneous logic about when to dump event
triggers (basically, they were *always* dumped before).  In 9.5 and up,
transform objects had that problem too.

Since this problem came in with extensions, back-patch to all supported
versions.

1 files changed, 128 insertions, 30 deletions

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 72100e0024c..aea259b5e4f 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -40,30 +40,30 @@ static int	numCatalogIds = 0;
 
 /*
  * These variables are static to avoid the notational cruft of having to pass
- * them into findTableByOid() and friends.  For each of these arrays, we
- * build a sorted-by-OID index array immediately after it's built, and then
- * we use binary search in findTableByOid() and friends.  (qsort'ing the base
- * arrays themselves would be simpler, but it doesn't work because pg_dump.c
- * may have already established pointers between items.)
- */
-static TableInfo *tblinfo;
-static TypeInfo *typinfo;
-static FuncInfo *funinfo;
-static OprInfo *oprinfo;
-static NamespaceInfo *nspinfo;
-static int	numTables;
-static int	numTypes;
-static int	numFuncs;
-static int	numOperators;
-static int	numCollations;
-static int	numNamespaces;
+ * them into findTableByOid() and friends.  For each of these arrays, we build
+ * a sorted-by-OID index array immediately after the objects are fetched,
+ * and then we use binary search in findTableByOid() and friends.  (qsort'ing
+ * the object arrays themselves would be simpler, but it doesn't work because
+ * pg_dump.c may have already established pointers between items.)
+ */
 static DumpableObject **tblinfoindex;
 static DumpableObject **typinfoindex;
 static DumpableObject **funinfoindex;
 static DumpableObject **oprinfoindex;
 static DumpableObject **collinfoindex;
 static DumpableObject **nspinfoindex;
+static DumpableObject **extinfoindex;
+static int	numTables;
+static int	numTypes;
+static int	numFuncs;
+static int	numOperators;
+static int	numCollations;
+static int	numNamespaces;
+static int	numExtensions;
 
+/* This is an array of object identities, not actual DumpableObjects */
+static ExtensionMemberId *extmembers;
+static int	numextmembers;
 
 static void flagInhTables(TableInfo *tbinfo, int numTables,
 			  InhInfo *inhinfo, int numInherits);
@@ -71,6 +71,7 @@ static void flagInhAttrs(TableInfo *tblinfo, int numTables);
 static DumpableObject **buildIndexArray(void *objArray, int numObjs,
 				Size objSize);
 static int	DOCatalogIdCompare(const void *p1, const void *p2);
+static int	ExtensionMemberIdCompare(const void *p1, const void *p2);
 static void findParentsByOid(TableInfo *self,
 				 InhInfo *inhinfo, int numInherits);
 static int	strInArray(const char *pattern, char **arr, int arr_size);
@@ -83,10 +84,14 @@ static int	strInArray(const char *pattern, char **arr, int arr_size);
 TableInfo *
 getSchemaData(int *numTablesPtr)
 {
+	TableInfo  *tblinfo;
+	TypeInfo   *typinfo;
+	FuncInfo   *funinfo;
+	OprInfo    *oprinfo;
+	CollInfo   *collinfo;
+	NamespaceInfo *nspinfo;
 	ExtensionInfo *extinfo;
 	InhInfo    *inhinfo;
-	CollInfo   *collinfo;
-	int			numExtensions;
 	int			numAggregates;
 	int			numInherits;
 	int			numRules;
@@ -103,6 +108,20 @@ getSchemaData(int *numTablesPtr)
 	int			numForeignServers;
 	int			numDefaultACLs;
 
+	/*
+	 * We must read extensions and extension membership info first, because
+	 * extension membership needs to be consultable during decisions about
+	 * whether other objects are to be dumped.
+	 */
+	if (g_verbose)
+		write_msg(NULL, "reading extensions\n");
+	extinfo = getExtensions(&numExtensions);
+	extinfoindex = buildIndexArray(extinfo, numExtensions, sizeof(ExtensionInfo));
+
+	if (g_verbose)
+		write_msg(NULL, "identifying extension members\n");
+	getExtensionMembership(extinfo, numExtensions);
+
 	if (g_verbose)
 		write_msg(NULL, "reading schemas\n");
 	nspinfo = getNamespaces(&numNamespaces);
@@ -123,10 +142,6 @@ getSchemaData(int *numTablesPtr)
 	getOwnedSeqs(tblinfo, numTables);
 
 	if (g_verbose)
-		write_msg(NULL, "reading extensions\n");
-	extinfo = getExtensions(&numExtensions);
-
-	if (g_verbose)
 		write_msg(NULL, "reading user-defined functions\n");
 	funinfo = getFuncs(&numFuncs);
 	funinfoindex = buildIndexArray(funinfo, numFuncs, sizeof(FuncInfo));
@@ -204,14 +219,10 @@ getSchemaData(int *numTablesPtr)
 		write_msg(NULL, "reading table inheritance information\n");
 	inhinfo = getInherits(&numInherits);
 
-	/*
-	 * Identify extension member objects and mark them as not to be dumped.
-	 * This must happen after reading all objects that can be direct members
-	 * of extensions, but before we begin to process table subsidiary objects.
-	 */
+	/* Identify extension configuration tables that should be dumped */
 	if (g_verbose)
-		write_msg(NULL, "finding extension members\n");
-	getExtensionMembership(extinfo, numExtensions);
+		write_msg(NULL, "finding extension tables\n");
+	processExtensionTables(extinfo, numExtensions);
 
 	/* Link tables to parents, mark parents of target tables interesting */
 	if (g_verbose)
@@ -748,6 +759,93 @@ findNamespaceByOid(Oid oid)
 	return (NamespaceInfo *) findObjectByOid(oid, nspinfoindex, numNamespaces);
 }
 
+/*
+ * findExtensionByOid
+ *	  finds the entry (in extinfo) of the extension with the given oid
+ *	  returns NULL if not found
+ */
+ExtensionInfo *
+findExtensionByOid(Oid oid)
+{
+	return (ExtensionInfo *) findObjectByOid(oid, extinfoindex, numExtensions);
+}
+
+
+/*
+ * setExtensionMembership
+ *	  accept and save data about which objects belong to extensions
+ */
+void
+setExtensionMembership(ExtensionMemberId *extmems, int nextmems)
+{
+	/* Sort array in preparation for binary searches */
+	if (nextmems > 1)
+		qsort((void *) extmems, nextmems, sizeof(ExtensionMemberId),
+			  ExtensionMemberIdCompare);
+	/* And save */
+	extmembers = extmems;
+	numextmembers = nextmems;
+}
+
+/*
+ * findOwningExtension
+ *	  return owning extension for specified catalog ID, or NULL if none
+ */
+ExtensionInfo *
+findOwningExtension(CatalogId catalogId)
+{
+	ExtensionMemberId *low;
+	ExtensionMemberId *high;
+
+	/*
+	 * We could use bsearch() here, but the notational cruft of calling
+	 * bsearch is nearly as bad as doing it ourselves; and the generalized
+	 * bsearch function is noticeably slower as well.
+	 */
+	if (numextmembers <= 0)
+		return NULL;
+	low = extmembers;
+	high = extmembers + (numextmembers - 1);
+	while (low <= high)
+	{
+		ExtensionMemberId *middle;
+		int			difference;
+
+		middle = low + (high - low) / 2;
+		/* comparison must match ExtensionMemberIdCompare, below */
+		difference = oidcmp(middle->catId.oid, catalogId.oid);
+		if (difference == 0)
+			difference = oidcmp(middle->catId.tableoid, catalogId.tableoid);
+		if (difference == 0)
+			return middle->ext;
+		else if (difference < 0)
+			low = middle + 1;
+		else
+			high = middle - 1;
+	}
+	return NULL;
+}
+
+/*
+ * qsort comparator for ExtensionMemberIds
+ */
+static int
+ExtensionMemberIdCompare(const void *p1, const void *p2)
+{
+	const ExtensionMemberId *obj1 = (const ExtensionMemberId *) p1;
+	const ExtensionMemberId *obj2 = (const ExtensionMemberId *) p2;
+	int			cmpval;
+
+	/*
+	 * Compare OID first since it's usually unique, whereas there will only be
+	 * a few distinct values of tableoid.
+	 */
+	cmpval = oidcmp(obj1->catId.oid, obj2->catId.oid);
+	if (cmpval == 0)
+		cmpval = oidcmp(obj1->catId.tableoid, obj2->catId.tableoid);
+	return cmpval;
+}
+
 
 /*
  * findParentsByOid