Fix pg_dump for hash partitioning on enum columns

doc/src/sgml/ref/pg_dump.sgml

@@ -863,16 +863,6 @@ PostgreSQL documentation
         and the two systems have different definitions of the collation used
         to sort the partitioning column.
        </para>
-
-       <para>
-        It is best not to use parallelism when restoring from an archive made
-        with this option, because <application>pg_restore</application> will
-        not know exactly which partition(s) a given archive data item will
-        load data into.  This could result in inefficiency due to lock
-        conflicts between parallel jobs, or perhaps even reload failures due
-        to foreign key constraints being set up before all the relevant data
-        is loaded.
-       </para>
       </listitem>
      </varlistentry>
 

doc/src/sgml/ref/pg_dumpall.sgml

@@ -395,10 +395,6 @@ PostgreSQL documentation
         and the two systems have different definitions of the collation used
         to sort the partitioning column.
        </para>
-
-       <!-- Currently, we don't need pg_dump's warning about parallelism here,
-        since parallel restore from a pg_dumpall script is impossible.
-       -->
       </listitem>
      </varlistentry>
 

src/bin/pg_dump/common.c

@@ -250,6 +250,9 @@ getSchemaData(Archive *fout, int *numTablesPtr)
 	pg_log_info("flagging inherited columns in subtables");
 	flagInhAttrs(fout->dopt, tblinfo, numTables);
 
+	pg_log_info("reading partitioning data");
+	getPartitioningInfo(fout);
+
 	pg_log_info("reading indexes");
 	getIndexes(fout, tblinfo, numTables);
 
@@ -302,7 +305,6 @@ static void
 flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
 			  InhInfo *inhinfo, int numInherits)
 {
-	DumpOptions *dopt = fout->dopt;
 	int			i,
 				j;
 
@@ -318,26 +320,18 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
 			continue;
 
 		/*
-		 * FIXME: In PostgreSQL, foreign tables can be inherited. But
-		 * pg_dump chokes on external tables, if an external table is
-		 * used as a partition, and a column has attislocal=false.
-		 */
-		if (tblinfo[i].relstorage == 'x' /* RELSTORAGE_EXTERNAL */)
-			continue;
-
-		/*
-		 * Normally, we don't bother computing anything for non-target tables,
-		 * but if load-via-partition-root is specified, we gather information
-		 * on every partition in the system so that getRootTableInfo can trace
-		 * from any given to leaf partition all the way up to the root.  (We
-		 * don't need to mark them as interesting for getTableAttrs, though.)
+		 * Normally, we don't bother computing anything for non-target tables.
+		 * However, we must find the parents of non-root partitioned tables in
+		 * any case, so that we can trace from leaf partitions up to the root
+		 * (in case a leaf is to be dumped but its parents are not).  We need
+		 * not mark such parents interesting for getTableAttrs, though.
 		 */
 		if (!tblinfo[i].dobj.dump)
 		{
 			mark_parents = false;
 
-			if (!dopt->load_via_partition_root ||
-				!tblinfo[i].ispartition)
+			if (!(tblinfo[i].relkind == RELKIND_PARTITIONED_TABLE &&
+				  tblinfo[i].ispartition))
 				find_parents = false;
 		}
 

src/bin/pg_dump/meson.build

@@ -0,0 +1,105 @@
+# Copyright (c) 2022-2023, PostgreSQL Global Development Group
+
+pg_dump_common_sources = files(
+  'compress_gzip.c',
+  'compress_io.c',
+  'compress_lz4.c',
+  'compress_none.c',
+  'dumputils.c',
+  'parallel.c',
+  'pg_backup_archiver.c',
+  'pg_backup_custom.c',
+  'pg_backup_db.c',
+  'pg_backup_directory.c',
+  'pg_backup_null.c',
+  'pg_backup_tar.c',
+  'pg_backup_utils.c',
+)
+
+pg_dump_common = static_library('libpgdump_common',
+  pg_dump_common_sources,
+  c_pch: pch_postgres_fe_h,
+  dependencies: [frontend_code, libpq, lz4, zlib],
+  kwargs: internal_lib_args,
+)
+
+
+pg_dump_sources = files(
+  'common.c',
+  'pg_dump.c',
+  'pg_dump_sort.c',
+)
+
+if host_system == 'windows'
+  pg_dump_sources += rc_bin_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'pg_dump',
+    '--FILEDESC', 'pg_dump - backup one PostgreSQL database',])
+endif
+
+pg_dump = executable('pg_dump',
+  pg_dump_sources,
+  link_with: [pg_dump_common],
+  dependencies: [frontend_code, libpq, zlib],
+  kwargs: default_bin_args,
+)
+bin_targets += pg_dump
+
+
+pg_dumpall_sources = files(
+  'pg_dumpall.c',
+)
+
+if host_system == 'windows'
+  pg_dumpall_sources += rc_bin_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'pg_dumpall',
+    '--FILEDESC', 'pg_dumpall - backup PostgreSQL databases'])
+endif
+
+pg_dumpall = executable('pg_dumpall',
+  pg_dumpall_sources,
+  link_with: [pg_dump_common],
+  dependencies: [frontend_code, libpq, zlib],
+  kwargs: default_bin_args,
+)
+bin_targets += pg_dumpall
+
+
+pg_restore_sources = files(
+  'pg_restore.c',
+)
+
+if host_system == 'windows'
+  pg_restore_sources += rc_bin_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'pg_restore',
+    '--FILEDESC', 'pg_restore - restore PostgreSQL databases'])
+endif
+
+pg_restore = executable('pg_restore',
+  pg_restore_sources,
+  link_with: [pg_dump_common],
+  dependencies: [frontend_code, libpq, zlib],
+  kwargs: default_bin_args,
+)
+bin_targets += pg_restore
+
+tests += {
+  'name': 'pg_dump',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'tap': {
+    'env': {
+      'GZIP_PROGRAM': gzip.path(),
+      'LZ4': program_lz4.found() ? program_lz4.path() : '',
+      'with_icu': icu.found() ? 'yes' : 'no',
+    },
+    'tests': [
+      't/001_basic.pl',
+      't/002_pg_dump.pl',
+      't/003_pg_dump_with_server.pl',
+      't/004_pg_dump_parallel.pl',
+      't/010_dump_connstr.pl',
+    ],
+  },
+}
+
+subdir('po', if_found: libintl)

src/bin/pg_dump/pg_backup_archiver.c

@@ -93,6 +93,7 @@ static RestorePass _tocEntryRestorePass(TocEntry *te);
 static bool _tocEntryIsACL(TocEntry *te);
 static void _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te);
 static void _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te);
+static bool is_load_via_partition_root(TocEntry *te);
 static void buildTocEntryArrays(ArchiveHandle *AH);
 static void _moveBefore(TocEntry *pos, TocEntry *te);
 static int	_discoverArchiveFormat(ArchiveHandle *AH);
@@ -892,6 +893,8 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
 				}
 				else
 				{
+					bool		use_truncate;
+
 					_disableTriggersIfNecessary(AH, te);
 
 					/* Select owner and schema as necessary */
@@ -903,13 +906,24 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
 
 					/*
 					 * In parallel restore, if we created the table earlier in
-					 * the run then we wrap the COPY in a transaction and
-					 * precede it with a TRUNCATE.  If archiving is not on
-					 * this prevents WAL-logging the COPY.  This obtains a
-					 * speedup similar to that from using single_txn mode in
-					 * non-parallel restores.
+					 * this run (so that we know it is empty) and we are not
+					 * restoring a load-via-partition-root data item then we
+					 * wrap the COPY in a transaction and precede it with a
+					 * TRUNCATE.  If wal_level is set to minimal this prevents
+					 * WAL-logging the COPY.  This obtains a speedup similar
+					 * to that from using single_txn mode in non-parallel
+					 * restores.
+					 *
+					 * We mustn't do this for load-via-partition-root cases
+					 * because some data might get moved across partition
+					 * boundaries, risking deadlock and/or loss of previously
+					 * loaded data.  (We assume that all partitions of a
+					 * partitioned table will be treated the same way.)
 					 */
-					if (is_parallel && te->created)
+					use_truncate = is_parallel && te->created &&
+						!is_load_via_partition_root(te);
+
+					if (use_truncate)
 					{
 						/*
 						 * Parallel restore is always talking directly to a
@@ -950,7 +964,7 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
 					AH->outputKind = OUTPUT_SQLCMDS;
 
 					/* close out the transaction started above */
-					if (is_parallel && te->created)
+					if (use_truncate)
 						CommitTransaction(&AH->public);
 
 					_enableTriggersIfNecessary(AH, te);
@@ -1048,6 +1062,43 @@ _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te)
 			 fmtQualifiedId(te->namespace, te->tag));
 }
 
+/*
+ * Detect whether a TABLE DATA TOC item is performing "load via partition
+ * root", that is the target table is an ancestor partition rather than the
+ * table the TOC item is nominally for.
+ *
+ * In newer archive files this can be detected by checking for a special
+ * comment placed in te->defn.  In older files we have to fall back to seeing
+ * if the COPY statement targets the named table or some other one.  This
+ * will not work for data dumped as INSERT commands, so we could give a false
+ * negative in that case; fortunately, that's a rarely-used option.
+ */
+static bool
+is_load_via_partition_root(TocEntry *te)
+{
+	if (te->defn &&
+		strncmp(te->defn, "-- load via partition root ", 27) == 0)
+		return true;
+	if (te->copyStmt && *te->copyStmt)
+	{
+		PQExpBuffer copyStmt = createPQExpBuffer();
+		bool		result;
+
+		/*
+		 * Build the initial part of the COPY as it would appear if the
+		 * nominal target table is the actual target.  If we see anything
+		 * else, it must be a load-via-partition-root case.
+		 */
+		appendPQExpBuffer(copyStmt, "COPY %s ",
+						  fmtQualifiedId(te->namespace, te->tag));
+		result = strncmp(te->copyStmt, copyStmt->data, copyStmt->len) != 0;
+		destroyPQExpBuffer(copyStmt);
+		return result;
+	}
+	/* Assume it's not load-via-partition-root */
+	return false;
+}
+
 /*
  * This is a routine that is part of the dumper interface, hence the 'Archive*' parameter.
  */
@@ -3020,8 +3071,12 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
 			res = res & ~REQ_DATA;
 	}
 
-	/* If there's no definition command, there's no schema component */
-	if (!te->defn || !te->defn[0])
+	/*
+	 * If there's no definition command, there's no schema component.  Treat
+	 * "load via partition root" comments as not schema.
+	 */
+	if (!te->defn || !te->defn[0] ||
+		strncmp(te->defn, "-- load via partition root ", 27) == 0)
 		res = res & ~REQ_SCHEMA;
 
 	/*