From 198df7ca02a53f4b7841e2e7d2ac1e8f36ad1a12 Mon Sep 17 00:00:00 2001
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Tue, 25 Aug 2026 15:06:58 +0000
Subject: [PATCH v7 4/5] pg_combinebackup: Refuse mixed data checksum states in
 a backup chain

check_control_files() detected a chain whose backups were taken under
different data checksum states, warned, and proceeded.  The output
directory keeps the last backup's control file, so a full backup taken
with checksums off combined with an incremental taken after an offline
enable produces a cluster whose control file says "on" while most of
its blocks carry no checksums; it starts, and then every connection
dies on the first unchecksummed catalog page.  An offline enable
between two backups of a chain is all it takes, since it rewrites
every page without logging anything, so the incremental backup does
not re-ship the pages.

Turn the warning into an error, matching what pg_rewind does for the
equivalent combinations.  The check stays asymmetric on purpose: when
the last backup was taken with checksums off, stale checksums from an
earlier backup are never verified, and that chain remains usable.
---
 doc/src/sgml/ref/pg_combinebackup.sgml        |  19 +--
 src/bin/pg_combinebackup/pg_combinebackup.c   |  14 +-
 src/test/modules/test_checksums/meson.build   |   1 +
 .../t/023_combinebackup_mixed.pl              | 134 ++++++++++++++++++
 4 files changed, 155 insertions(+), 13 deletions(-)
 create mode 100644 src/test/modules/test_checksums/t/023_combinebackup_mixed.pl

diff --git a/doc/src/sgml/ref/pg_combinebackup.sgml b/doc/src/sgml/ref/pg_combinebackup.sgml
index 9a6d201e0b8..f5d7e177b36 100644
--- a/doc/src/sgml/ref/pg_combinebackup.sgml
+++ b/doc/src/sgml/ref/pg_combinebackup.sgml
@@ -306,18 +306,19 @@ PostgreSQL documentation
 
   <para>
    <literal>pg_combinebackup</literal> does not recompute page checksums when
-   writing the output directory. Therefore, if any of the backups used for
-   reconstruction were taken with checksums disabled, but the final backup was
-   taken with checksums enabled, the resulting directory may contain pages
-   with invalid checksums.
+   writing the output directory.  It therefore refuses a chain in which some
+   of the backups used for reconstruction were taken with checksums disabled
+   but the final backup was taken with checksums enabled: the resulting
+   directory would contain pages with invalid checksums, and the cluster
+   would fail checksum verification as soon as it reads them.  Take a new
+   full backup after enabling data checksums with
+   <xref linkend="app-pgchecksums"/>.
   </para>
 
   <para>
-   To avoid this problem, taking a new full backup after changing the checksum
-   state of the cluster using <xref linkend="app-pgchecksums"/> is
-   recommended. Otherwise, you can disable and then optionally reenable
-   checksums on the directory produced by <literal>pg_combinebackup</literal>
-   in order to correct the problem.
+   The reverse case is accepted: when the final backup was taken with
+   checksums disabled, stale checksums copied from an earlier backup are
+   never verified.
   </para>
  </refsect1>
 
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 86e2ee37c40..254a27b125b 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -670,13 +670,19 @@ check_control_files(int n_backups, char **backup_dirs)
 	pg_log_debug("system identifier is %" PRIu64, system_identifier);
 
 	/*
-	 * Warn the user if not all backups are in the same state with regards to
-	 * checksums.
+	 * Reject a chain whose backups were not all in the same state with
+	 * regards to checksums.  The loop above only flags that when the last
+	 * backup has checksums enabled: the blocks taken from the older backups
+	 * have no checksums, and pg_combinebackup does not recompute them, so the
+	 * combined cluster would fail verification as soon as it reads them.  The
+	 * other direction is harmless, since stale checksums are never verified
+	 * while checksums are disabled.
 	 */
 	if (data_checksum_mismatch)
 	{
-		pg_log_warning("only some backups have checksums enabled");
-		pg_log_warning_hint("Disable, and optionally reenable, checksums on the output directory to avoid failures.");
+		pg_log_error("only some backups have checksums enabled");
+		pg_log_error_hint("Take a new full backup after changing the data checksum state with pg_checksums.");
+		exit(1);
 	}
 
 	return system_identifier;
diff --git a/src/test/modules/test_checksums/meson.build b/src/test/modules/test_checksums/meson.build
index a4f25bac7a5..8b85a7848bb 100644
--- a/src/test/modules/test_checksums/meson.build
+++ b/src/test/modules/test_checksums/meson.build
@@ -46,6 +46,7 @@ tests += {
       't/020_cascade_divergence.pl',
       't/021_rewind_state.pl',
       't/022_rewind_standby_target.pl',
+      't/023_combinebackup_mixed.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_checksums/t/023_combinebackup_mixed.pl b/src/test/modules/test_checksums/t/023_combinebackup_mixed.pl
new file mode 100644
index 00000000000..ce9dee9413b
--- /dev/null
+++ b/src/test/modules/test_checksums/t/023_combinebackup_mixed.pl
@@ -0,0 +1,134 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that pg_combinebackup refuses a chain whose backups were taken under
+# different data checksum states when the final state has them enabled.
+#
+# The output directory keeps the last backup's control file, so a full
+# backup taken with checksums off plus an incremental taken after an
+# offline enable would produce a directory whose control file says "on"
+# while most of its blocks have no checksums.  An offline enable between
+# the two backups is enough to get there: it rewrites every page but logs
+# nothing, so the incremental backup does not re-ship the pages.
+#
+# The reverse order stays allowed: with the final backup taken with
+# checksums off, stale checksums from an earlier backup are never verified.
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+use FindBin;
+use lib $FindBin::RealBin;
+
+use DataChecksums::Utils;
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init(
+	no_data_checksums => 1,
+	has_archiving => 1,
+	allows_streaming => 1);
+$node->append_conf('postgresql.conf', 'summarize_wal = on');
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+$node->safe_psql('postgres',
+	"CREATE TABLE t1 AS SELECT generate_series(1,200000) AS a;");
+$node->safe_psql('postgres', 'CHECKPOINT;');
+
+test_checksum_state($node, 'off');
+
+$node->backup('full');
+
+# Offline enable: rewrites every page, logs nothing.
+$node->stop;
+system_or_bail('pg_checksums', '--enable', '--pgdata', $node->data_dir);
+$node->start;
+test_checksum_state($node, 'on');
+
+$node->safe_psql('postgres',
+	"CREATE TABLE t2 AS SELECT generate_series(1,1000) AS a;");
+$node->safe_psql('postgres', 'CHECKPOINT;');
+
+$node->command_ok(
+	[
+		'pg_basebackup',
+		'--pgdata' => $node->backup_dir . '/incr',
+		'--dbname' => $node->connstr('postgres'),
+		'--no-sync',
+		'--checkpoint' => 'fast',
+		'--incremental' => $node->backup_dir . '/full/backup_manifest',
+	],
+	'incremental backup with checksums on');
+
+# Combining the chain must be refused: the result would say "on" while the
+# blocks inherited from the full backup have no checksums.
+command_fails_like(
+	[
+		'pg_combinebackup',
+		$node->backup_dir . '/full',
+		$node->backup_dir . '/incr',
+		'--output' => $node->backup_dir . '/combined',
+	],
+	qr/only some backups have checksums enabled/,
+	'pg_combinebackup refuses a chain crossing an offline enable');
+
+# The other direction: full backup with checksums on, offline disable, then
+# an incremental.  The last backup wins, so the combined cluster comes up off.
+$node->backup('full2');
+
+$node->stop;
+system_or_bail('pg_checksums', '--disable', '--pgdata', $node->data_dir);
+$node->start;
+test_checksum_state($node, 'off');
+
+$node->safe_psql('postgres',
+	"CREATE TABLE t3 AS SELECT generate_series(1,1000) AS a;");
+$node->safe_psql('postgres', 'CHECKPOINT;');
+
+$node->command_ok(
+	[
+		'pg_basebackup',
+		'--pgdata' => $node->backup_dir . '/incr2',
+		'--dbname' => $node->connstr('postgres'),
+		'--no-sync',
+		'--checkpoint' => 'fast',
+		'--incremental' => $node->backup_dir . '/full2/backup_manifest',
+	],
+	'incremental backup with checksums off');
+
+my $restored = PostgreSQL::Test::Cluster->new('restored');
+$restored->init_from_backup(
+	$node, 'incr2',
+	combine_with_prior => ['full2'],
+	has_restoring => 1,
+	standby => 0);
+
+$restored->start;
+
+my ($rc, $stdout, $stderr) = $restored->psql('postgres',
+	"SELECT setting FROM pg_settings WHERE name = 'data_checksums';");
+is($rc, 0, 'combined backup accepts connections') or diag("stderr: $stderr");
+is($stdout, 'off', 'combined backup follows the last backup state');
+
+($rc, $stdout, $stderr) =
+  $restored->psql('postgres', 'SELECT count(*) FROM t3;');
+is($rc, 0, 'blocks from the incremental backup are readable')
+  or diag("stderr: $stderr");
+
+($rc, $stdout, $stderr) =
+  $restored->psql('postgres', 'SELECT count(*) FROM t1;');
+is($rc, 0, 'blocks inherited from the full backup are readable')
+  or diag("stderr: $stderr");
+
+my $log = PostgreSQL::Test::Utils::slurp_file($restored->logfile);
+unlike(
+	$log,
+	qr/page verification failed/,
+	'no checksum verification failures in the combined backup');
+
+$restored->stop('immediate');
+$node->stop;
+
+done_testing();
-- 
2.55.0

