Allow clone of extensions in schemas other than the default

This removes the non-default schema exclusion code to allow
extensions to be transfered with `clone` without interruption.

We add tests to avoid regression.

docs/include/clone.rst

@@ -23,6 +23,7 @@
      --skip-ext-comments           Skip restoring COMMENT ON EXTENSION
      --skip-collations             Skip restoring collations
      --skip-vacuum                 Skip running VACUUM ANALYZE
+     --skip-ext-schemas            Skip creating schemas before extensions
      --requirements <filename>     List extensions requirements
      --filters <filename>          Use the filters defined in <filename>
      --fail-fast                   Abort early in case of error

src/bin/pgcopydb/catalog.c

@@ -4825,7 +4825,8 @@ catalog_iter_s_seq_finish(SourceSeqIterator *iter)
 bool
 catalog_prepare_filter(DatabaseCatalog *catalog,
 					   bool skipExtensions,
-					   bool skipCollations)
+					   bool skipCollations,
+					   bool skipExtSchemas)
 {
 	sqlite3 *db = catalog->db;
 
@@ -4838,11 +4839,6 @@ catalog_prepare_filter(DatabaseCatalog *catalog,
 	char *sql =
 		"insert into filter(oid, restore_list_name, kind) "
 
-		"     select oid, restore_list_name, 's_namespace' "
-		"       from s_namespace "
-
-		"  union all "
-
 		"     select oid, restore_list_name, 'table' "
 		"       from s_table "
 
@@ -4997,6 +4993,31 @@ catalog_prepare_filter(DatabaseCatalog *catalog,
 		}
 	}
 
+	/*
+	 * Implement --skip-ext-schemas by filtering any schemas during a pg_restore
+	 * to allow for precreated schemas to be used without error.
+	 */
+	if (skipExtSchemas)
+	{
+		char *s_schema_sql =
+			"insert or ignore into filter(oid, restore_list_name, kind) "
+			"    select oid, nspname, 's_namespace' "
+			"      from s_namespace ";
+
+		if (!catalog_sql_prepare(db, s_schema_sql, &query))
+		{
+			/* errors have already been logged */
+			return false;
+		}
+
+		/* now execute the query, which does not return any row */
+		if (!catalog_sql_execute_once(&query))
+		{
+			/* errors have already been logged */
+			return false;
+		}
+	}
+
 	return true;
 }
 

src/bin/pgcopydb/catalog.h

@@ -380,7 +380,8 @@ bool catalog_s_seq_fetch(SQLiteQuery *query);
  */
 bool catalog_prepare_filter(DatabaseCatalog *catalog,
 							bool skipExtensions,
-							bool skipCollations);
+							bool skipCollations,
+							bool skipExtSchemas);
 
 typedef struct CatalogFilter
 {

src/bin/pgcopydb/cli_clone_follow.c

@@ -46,6 +46,7 @@
 	"  --skip-ext-comments           Skip restoring COMMENT ON EXTENSION\n" \
 	"  --skip-collations             Skip restoring collations\n" \
 	"  --skip-vacuum                 Skip running VACUUM ANALYZE\n" \
+	"  --skip-ext-schemas            Skip creating schemas before extensions\n" \
 	"  --requirements <filename>     List extensions requirements\n" \
 	"  --filters <filename>          Use the filters defined in <filename>\n" \
 	"  --fail-fast                   Abort early in case of error\n" \

src/bin/pgcopydb/cli_common.c

@@ -783,6 +783,7 @@ cli_copy_db_getopts(int argc, char **argv)
 		{ "skip-ext-comments", no_argument, NULL, 'M' },
 		{ "skip-collations", no_argument, NULL, 'l' },
 		{ "skip-vacuum", no_argument, NULL, 'U' },
+		{ "skip-ext-schemas", no_argument, NULL, 'n' },
 		{ "filter", required_argument, NULL, 'F' },
 		{ "filters", required_argument, NULL, 'F' },
 		{ "requirements", required_argument, NULL, 'Q' },
@@ -995,7 +996,14 @@ cli_copy_db_getopts(int argc, char **argv)
 			case 'M':
 			{
 				options.skipCommentOnExtension = true;
-				log_trace("--skip-extensions");
+				log_trace("--skip-ext-comments");
+				break;
+			}
+
+			case 'n':
+			{
+				options.skipExtSchemas = true;
+				log_trace("--skip-ext-schemas");
 				break;
 			}
 

src/bin/pgcopydb/cli_common.h

@@ -48,6 +48,7 @@ typedef struct CopyDBOptions
 	bool skipCommentOnExtension;
 	bool skipCollations;
 	bool skipVacuum;
+	bool skipExtSchemas;
 	bool noRolesPasswords;
 	bool failFast;
 

src/bin/pgcopydb/copydb.c

@@ -511,6 +511,7 @@ copydb_init_specs(CopyDataSpec *specs,
 		.roles = options->roles,
 		.skipLargeObjects = options->skipLargeObjects,
 		.skipExtensions = options->skipExtensions,
+		.skipExtSchemas = options->skipExtSchemas,
 		.skipCommentOnExtension = options->skipCommentOnExtension,
 		.skipCollations = options->skipCollations,
 		.skipVacuum = options->skipVacuum,

src/bin/pgcopydb/copydb.h

@@ -224,6 +224,7 @@ typedef struct CopyDataSpec
 	bool skipCommentOnExtension;
 	bool skipCollations;
 	bool skipVacuum;
+	bool skipExtSchemas;
 	bool noRolesPasswords;
 
 	bool restart;

src/bin/pgcopydb/copydb_schema.c

@@ -1007,7 +1007,8 @@ copydb_fetch_filtered_oids(CopyDataSpec *specs, PGSQL *pgsql)
 
 			if (!catalog_prepare_filter(filtersDB,
 										specs->skipExtensions,
-										specs->skipCollations))
+										specs->skipCollations,
+										specs->skipExtSchemas))
 			{
 				log_error("Failed to prepare filtering hash-table, "
 						  "see above for details");
@@ -1173,7 +1174,8 @@ copydb_fetch_filtered_oids(CopyDataSpec *specs, PGSQL *pgsql)
 
 		if (!catalog_prepare_filter(filtersDB,
 									specs->skipExtensions,
-									specs->skipCollations))
+									specs->skipCollations,
+									specs->skipExtSchemas))
 		{
 			log_error("Failed to prepare filtering hash-table, "
 					  "see above for details");

src/bin/pgcopydb/pgcmd.c

@@ -451,50 +451,6 @@ pg_dump_db(PostgresPaths *pgPaths,
 	char *extNamespaces[PG_CMD_MAX_ARG];
 	int extNamespaceCount = 0;
 
-	/* now --exclude-schema for extension's own schemas */
-	DumpExtensionNamespaceContext context = {
-		.extNamespaces = extNamespaces,
-		.extNamespaceCount = &extNamespaceCount,
-	};
-
-	if (!catalog_begin(filtersDB, false))
-	{
-		/* errors have already been logged */
-		return false;
-	}
-
-	if (!catalog_iter_s_extension(filtersDB,
-								  &context,
-								  &pg_dump_db_extension_namespace_hook))
-	{
-		log_error("Failed to prepare pg_dump command line arguments, "
-				  "see above for details");
-		return false;
-	}
-
-	if (!catalog_commit(filtersDB))
-	{
-		/* errors have already been logged */
-		return false;
-	}
-
-	for (int i = 0; i < extNamespaceCount; i++)
-	{
-		/* check that we still have room for --exclude-schema args */
-		if (PG_CMD_MAX_ARG < (argsIndex + 2))
-		{
-			log_error("Failed to call pg_dump, "
-					  "too many schema are excluded: "
-					  "argsIndex %d > %d",
-					  argsIndex + 2,
-					  PG_CMD_MAX_ARG);
-			return false;
-		}
-
-		args[argsIndex++] = "--exclude-schema";
-		args[argsIndex++] = extNamespaces[i];
-	}
-
 	args[argsIndex++] = "--file";
 	args[argsIndex++] = (char *) filename;
 	args[argsIndex++] = (char *) connStrings->safeSourcePGURI.pguri;

tests/pagila/copydb.sh

@@ -30,24 +30,42 @@ EOF
 PAGILA_SOURCE_PGURI="postgres://pagila:0wn3d@source/pagila"
 PAGILA_TARGET_PGURI="postgres://pagila:0wn3d@target/pagila"
 
+# Create extensions in the default and alternative schemas to always
+# ensure these copy as expected.
 psql -d ${PAGILA_SOURCE_PGURI} <<EOF
 create extension ltree;
 create extension hstore;
+create schema foo;
+create extension intarray schema foo cascade;
 EOF
 
 grep -v "OWNER TO postgres" /usr/src/pagila/pagila-schema.sql > /tmp/pagila-schema.sql
 
 psql -o /tmp/s.out -d ${PAGILA_SOURCE_PGURI} -1 -f /tmp/pagila-schema.sql
 psql -o /tmp/d.out -d ${PAGILA_SOURCE_PGURI} -1 -f /usr/src/pagila/pagila-data.sql
 
-pgcopydb clone --skip-ext-comments --notice \
-         --source ${PAGILA_SOURCE_PGURI} \
-         --target ${PAGILA_TARGET_PGURI}
+function compare_schema() {
+    pgcopydb compare schema \
+             --source ${PAGILA_SOURCE_PGURI} \
+             --target ${PAGILA_TARGET_PGURI}
+}
+
+function compare_data() {
+    pgcopydb compare data \
+             --source ${PAGILA_SOURCE_PGURI} \
+             --target ${PAGILA_TARGET_PGURI}
+}
 
-pgcopydb compare schema \
+pgcopydb clone --skip-ext-comments --notice \
          --source ${PAGILA_SOURCE_PGURI} \
          --target ${PAGILA_TARGET_PGURI}
+compare_schema
+compare_data
 
-pgcopydb compare data \
+# Repeat the process with --drop-if-exists to ensure it is idempotent
+pgcopydb clone --skip-ext-comments --notice \
          --source ${PAGILA_SOURCE_PGURI} \
-         --target ${PAGILA_TARGET_PGURI}
+         --target ${PAGILA_TARGET_PGURI} \
+         --drop-if-exists
+compare_schema
+compare_data