[liang8283]

Summary

Adds an opt-in --heap-file-hash flag to gpbackup so that --incremental can skip heap tables whose data has not changed. Today every heap table is re-copied by every incremental backup; with this flag, a heap table is skipped when its on-disk file state, its last DDL timestamp and the definitions of the enum and composite types its columns use all match the previous backup.

Without the flag, behavior is unchanged: heap tables are always included, and AO and PAX tables keep using their existing change markers. The flag requires Greenplum 6 or later, or Cloudberry; on Greenplum 5 it logs a warning and no heap table is skipped.

--ao-file-hash from the first revision has been dropped: the aoseg content hash does not change on DELETE, so it is not a safe replacement for modcount (see the discussion below).

How it works

• What is hashed. A plpgsql helper, gp_toolkit.gpbackup_file_info(oid), runs on every segment via gp_dist_random. It resolves the relation's path with pg_relation_filepath() and stats every segment file of the main fork (path, path.1, path.2, ... until one is missing, the same way md.c walks them), so tables larger than RELSEG_SIZE are fully covered. The path itself is part of the hash, so a new relfilenode (TRUNCATE, VACUUM FULL, CLUSTER) is detected as a change.
• When it is hashed. Hashes are collected before the backup snapshot is taken, on a separate connection, right after a CHECKPOINT so that pg_stat_file sees flushed data. Any change committed after the snapshot moves the data files on by the next backup's CHECKPOINT, so the next incremental sees a different hash and includes the table. (Hashing after the snapshot would let changes the backup cannot see enter the hash.)
• In-progress writers. The data files also hold rows of transactions that are still open, and their COMMIT only updates the commit log. After reading the hashes and before the snapshot, gpbackup checks pg_locks and drops the hash of every table on which another transaction holds a write lock (RowExclusiveLock or stronger).
• Verification after the snapshot. A change committed after the first CHECKPOINT but before the snapshot may still sit in shared buffers, invisible to the hash but visible to the backup. Once the snapshot is taken, gpbackup issues a second CHECKPOINT, re-reads the file info of every hashed table, and drops the hash of any table whose files changed.
• DDL. Each heap entry also records the table's last DDL timestamp from pg_stat_last_operation, read inside the backup snapshot. A DDL that does not rewrite the table, such as DROP COLUMN, therefore still forces the table into the incremental.
• Columns. Restore loads a skipped table's old data file into the newest column list, and some changes alter that list or how stored values are dumped without touching the data files or the table's DDL timestamp: ALTER TYPE ... RENAME VALUE on an enum, ALTER TYPE ... ADD/DROP ATTRIBUTE on a composite type, adding a column to a table whose row type another table uses as a column type, or DROP DOMAIN ... CASCADE dropping a column. Each heap entry also records a hash of the table's columns (number, name, type, typmod, dropped) and of the definitions of the enum and composite types they use, directly or through domains, arrays, composite, range and multirange types, read inside the backup snapshot under a savepoint. If it cannot be read, no heap table is skipped.
• Values that name catalog objects. regclass and the other reg* types, and aclitem, store oids but are dumped as the names of the objects they reference, so renaming or dropping the referenced object changes what COPY writes, or makes the old value fail to load. Tables that use any of these types, directly or through domains, arrays, composite and range types, are never hashed and are always included.
• Hashes are keyed by oid until they are matched with the table's name inside the backup snapshot, so a rename between hashing and the snapshot cannot attach a hash to the wrong table.
• Unlogged tables are never hashed: a regular CHECKPOINT does not write their buffers, so their files may not move when they change.
• Partitions. Leaf partitions of partitioned heap tables are hashed like any other heap table.
• Failing safe. Whenever a hash cannot be trusted, no hash is recorded and the table is included in the next incremental:
  • either CHECKPOINT fails, or the lock check fails;
  • another transaction holds a write lock on the table;
  • the table's files changed between the first read and the post-snapshot verification;
  • a segment file was modified in the current second (pg_stat_file reports mtime in whole seconds, so a later write in the same second would not move it);
  • any segment cannot describe the relation, or the query fails.
• Decision. FilterTablesForIncremental skips a table that is neither AO nor PAX only when the previous restore plan has data for it (a table that belonged to an extension in the previous backup has none), both the previous and the current TOC carry a hash for it, and the hash, the DDL timestamp and the column hash all match.

Files changed

File	Change
toc/toc.go	Add HeapEntry{FileHashMD5, LastDDLTimestamp, ColumnsMD5} and IncrementalEntries.Heap (omitempty).
options/flag.go	Add --heap-file-hash.
backup/backup.go	Collect heap file hashes in DoSetup before the connection pool takes the snapshot, and verify them after it.
backup/queries_incremental.go	CollectHeapFileHashes, getTablesWithActiveWriters, VerifyHeapFileHashes, ensureFileStatFunction, getHeapTables, getFileHashesForTables (batched), getHeapIncrementalEntries, getHeapLastDDLTimestamps, getHeapColumnHashes.
backup/wrappers.go	backupIncrementalMetadata writes heap entries (hash, DDL timestamp, column hash) into the TOC.
backup/incremental.go	FilterTablesForIncremental skips heap tables via heapTableUnchanged, only if the previous restore plan has data for them.
backup/validate.go	--heap-file-hash requires --leaf-partition-data.
backup/incremental_test.go	Unit tests for the heap branches: unchanged, hash changed, DDL changed, column types changed, hash missing in either backup.

Backward compatibility

• Without the flag, the TOC and the incremental decision are identical to before.
• TOC additions are omitempty; older TOCs load fine, and older gprestore ignores the new fields.
• An incremental based on a backup taken without the flag includes every heap table (no previous hash).
• gp_toolkit.gpbackup_file_info is created only when the flag is used, for the duration of each hashing step, and dropped afterwards, so a stale body is never reused and nothing is left in gp_toolkit. The (text, text) signature from the first revision of this PR is dropped.

Operational notes

• The flag issues a cluster-wide CHECKPOINT at the start of each backup, plus a one-second wait so files flushed by it are not treated as "modified in the current second", and a second CHECKPOINT right after the snapshot is taken (usually cheap, since little is dirty by then).
• Hashes are collected for all heap tables in the schemas being backed up; --include-table / --exclude-table are applied afterwards.

How tested

• make unit: all suites pass, including the new FilterTablesForIncremental heap tests.
• End to end on a Cloudberry cluster: full backup with --heap-file-hash, then mutations, then an incremental, then a restore of the incremental. Every table's row count matches the source, and:

Scenario	Result
Append to a heap table >1GB (first segment file unchanged, only .1 grows)	Included
Insert committed while the full backup waits for its table lock (after the snapshot)	Included; restore returns both rows
Uncommitted INSERT / UPDATE / DELETE during the full backup, committed after it	Included; restore matches the source
Heap + AO insert committed in one transaction during the wait after the CHECKPOINT	Included; restore returns the new row in both tables
ALTER TABLE ... DROP COLUMN	Included; restore succeeds with the new column list
ALTER TYPE ... RENAME VALUE on an enum used directly, via a domain array, or via a composite type	Included; restore returns the new label
ALTER TYPE ... ADD ATTRIBUTE / DROP ATTRIBUTE ... CASCADE on a composite type used directly, nested in another composite, or via a domain array	Included; restore matches the source
ALTER TABLE ... ADD COLUMN on a table whose row type another heap table uses as a column type	Both tables included; restore matches the source
Partitioned heap table, one leaf modified	Modified leaf included, sibling leaves skipped
DROP DOMAIN ... CASCADE dropping a column of a heap table	Included; restore succeeds
Heap table removed from an extension after the full backup (ALTER EXTENSION ... DROP TABLE)	Included; restore returns its rows
Insert into an existing page of an UNLOGGED heap table	Included (unlogged tables are never skipped)
Heap tables storing regclass (directly, in a composite, in a domain array), regproc and aclitem values; referenced table and function renamed, or role renamed	Included; restore matches the source
Chain full → incremental → incremental	Unchanged table skipped twice and restored from the full backup
Unmodified heap table	Skipped

[talmacschen-arch]

Thanks for this — thoughtful PR and the overall design is sound: with both flags off the path is logically identical to the previous FilterTablesForIncremental, the TOC additions are all omitempty so backward compatibility holds, using a dedicated dbconn to avoid transaction-abort propagation is the right call, and excluding modcount from the AO content hash to dodge GP5's cross-partition modcount propagation addresses a real pain point.

I checked the two issues below against the Cloudberry kernel source and reproduced both on a live cluster, so they are not speculation — details are in the inline comments:

• [correctness] --heap-file-hash can silently miss changes on heap tables larger than 1GB — inline on gpbackup_file_info.
• [gap] --heap-file-hash does not cover leaf partitions of partitioned heap tables — inline on getHeapTables.

Two more points:

Test coverage: the new decision logic in FilterTablesForIncremental (AO-hash / heap-hash branches: prev vs current equal → skip, differ → include, either empty → include, Heap == nil → include) has no unit test — the test change only moves the existing flags-off call into JustBeforeEach. These branches are pure map logic over constructed TOCs and can be unit-tested without a live DB; worth adding since this is the heart of the feature.

nits:

• ensureFileStatFunction only CREATE OR REPLACEs when the function is absent; an older same-signature (oid) body would not be refreshed.
• The plpgsql EXCEPTION WHEN OTHERS THEN RETURN '' swallows every error (including permission failures) — safe-by-degradation, but hides real errors when debugging.
• CHECKPOINT is a cluster-wide heavy operation; frequent incrementals add I/O — worth a note in the docs.

Nice work overall — physical-state hashing for change detection is the right direction; both issues are about which physical state is being hashed.

[talmacschen-arch]

Requesting changes on the strength of the correctness issue in my detailed review above (F1).

On heap tables larger than 1GB, --heap-file-hash stats only the first 1GB segment (pg_relation_filepath never returns the .N tail segments — confirmed in md.c/relpath.c), so append-only changes that land in tail segments are silently missed and dropped from the incremental. I reproduced this on a live cluster: after appending 0.5GB to a >1GB table, the main segment's size|mtime was byte-for-byte unchanged while the data went to .1. That is a data-loss path for the feature.

Please either hash all segments of the relation (walk path, path.1, path.2, … like md.c does) or gate/document the flag for tables over 1GB before merge. F2 (leaf partitions excluded) and the missing unit tests are non-blocking. Happy to re-review once segment handling is addressed — the overall approach is good.

[talmacschen-arch]

@liang8283 heads-up — I've reviewed this PR and left a Request Changes.

The one blocker is a correctness issue (F1): on heap tables larger than 1GB, --heap-file-hash stats only the first 1GB segment, because pg_relation_filepath never returns the .N tail segments (confirmed in md.c / relpath.c). I reproduced it on a live cluster — after appending 0.5GB to a >1GB table, the main segment's size|mtime was byte-for-byte unchanged while the new data went to .1, so an append-mostly large table would be silently skipped from the incremental. A fix suggestion (walk all segments like md.c does) is in the inline comment.

F2 (leaf partitions of partitioned heap tables are excluded via NOT IN inhrelid) and the missing unit tests for the new hash branches are non-blocking. The overall approach is solid — happy to re-review once the segment handling is sorted out. Thanks!

[woblerr]

Preferably splitting --heap-file-hash and --ao-file-hash into two separate PRs. They are independent features, so each can be reviewed and merged on its own.

The --heap-file-hash functionality looks like the most useful change.

[liang8283]

Thanks both for the careful reviews and the reproductions. I pushed fbd2786, which addresses all of them; details are in the inline replies, and the PR description is updated.

• @talmacschen-arch: F1 (tail segments) and F2 (leaf partitions) are fixed. On the other points:
  • Unit tests now cover the heap branches of FilterTablesForIncremental: unchanged, hash changed, DDL changed, and hash missing in either backup.
  • gpbackup_file_info is recreated on every run, so a stale body is never reused.
  • The helper now only treats a missing segment file as "end of relation"; any other error surfaces as a warning, and the table is included rather than silently hashed.
  • The CHECKPOINT cost is now stated in the flag help and the PR description.
• @woblerr: both issues (DDL without a rewrite, and changes committed between snapshot and hash) are fixed, and your reproductions pass. As you suggested, I split the feature: this PR now contains only --heap-file-hash, and --ao-file-hash will come as a separate PR.

Could you take another look when you have a moment?

[liang8283]

A follow-up on --ao-file-hash: I said earlier that it would come as a separate PR, but I'm dropping it instead. Looking at it again before the split, I found it is not safe, and on Cloudberry it brings no benefit over the existing modcount check:

• DELETE is invisible to the aoseg hash. Deleting rows from an AO/AOCS table only updates the visimap; segno, eof, tupcount and vpinfo in pg_aoseg / pg_aocsseg are unchanged, while modcount does increase. The hash replaced the modcount + DDL-timestamp comparison, so an AO table that only saw DELETEs would be skipped, and a restore would bring the deleted rows back. Dropping the DDL timestamp would also reintroduce the DROP COLUMN issue raised above, this time for AO tables.
• No partition-granularity gain on Cloudberry. The motivation was GP5, where a parent's modcount propagates to sibling partitions. On Cloudberry each leaf partition's modcount is already independent: inserting into one leaf leaves its siblings' modcount unchanged, so the current check already works per leaf.
• The GP6 AOCS branch read columns that pg_aocsseg does not have (column_num, physical_segno, eof_uncompressed), so that query would always fail on GP6.

Making it correct would mean adding modcount (or the visimap) back into the comparison, which leaves nothing to gain over today's behavior. So this PR stays --heap-file-hash only, and no AO follow-up is planned.

[woblerr]

@liang8283
There is another issue. Please, check it out.

The full backup hashes the heap file after CHECKPOINT flushes an uncommitted insert, but COPY cannot see that row. Committing the transaction later leaves the hash unchanged, so the next incremental skips the table.

In psql, create a heap table with one committed row, then leave the second insert uncommitted.

CREATE TABLE t(id int) USING heap DISTRIBUTED BY (id);
INSERT INTO t VALUES (1);
BEGIN;
INSERT INTO t VALUES (2);

In another terminal, run a full backup with --leaf-partition-data --heap-file-hash. Once it finishes, run COMMIT; in the original psql session.

Run an incremental from that full backup with --heap-file-hash and restore it. The restore succeeds, but SELECT id FROM t ORDER BY id returns only 1.

Repeating the incremental from the same full backup with --heap-file-hash disabled restores both 1 and 2.

[liang8283]

@woblerr Thanks, confirmed and fixed in b60ef8f. The heap files also contain rows of transactions that are still in progress, and their COMMIT only updates the commit log, so the hash never changed after the commit.

The fix: after reading the file hashes, and before the backup snapshot is taken, gpbackup now checks pg_locks and drops the hash of every table on which another transaction holds a lock taken by data-modifying commands (RowExclusiveLock and stronger). That lock is held until the writer's transaction ends, so the ordering covers both cases:

• the writer commits before the snapshot: its rows are visible to the backup;
• the writer is still open: it still holds the lock at the check, so the table gets no hash and the next incremental includes it.

If the lock check itself fails, no heap hashes are recorded at all.

With your exact steps (uncommitted insert during the full backup, committed after it), the incremental now includes t and the restore returns both 1 and 2. I also ran the same sequence with an uncommitted UPDATE and an uncommitted DELETE (which take ExclusiveLock with the global deadlock detector off): before this fix both were skipped by the incremental and restored the pre-commit values; now the table is included and the restore matches the source. The earlier scenarios (tail segments, snapshot race, DROP COLUMN, leaf partitions) still behave as before.

[woblerr]

@woblerr Thanks, confirmed and fixed in b60ef8f. The heap files also contain rows of transactions that are still in progress, and their COMMIT only updates the commit log, so the hash never changed after the commit.

The fix: after reading the file hashes, and before the backup snapshot is taken, gpbackup now checks pg_locks and drops the hash of every table on which another transaction holds a lock taken by data-modifying commands (RowExclusiveLock and stronger). That lock is held until the writer's transaction ends, so the ordering covers both cases:

* the writer commits before the snapshot: its rows are visible to the backup;

* the writer is still open: it still holds the lock at the check, so the table gets no hash and the next incremental includes it.

If the lock check itself fails, no heap hashes are recorded at all.

With your exact steps (uncommitted insert during the full backup, committed after it), the incremental now includes t and the restore returns both 1 and 2. I also ran the same sequence with an uncommitted UPDATE and an uncommitted DELETE (which take ExclusiveLock with the global deadlock detector off): before this fix both were skipped by the incremental and restored the pre-commit values; now the table is included and the restore matches the source. The earlier scenarios (tail segments, snapshot race, DROP COLUMN, leaf partitions) still behave as before.

Thanks, the original case is fixed.
During the one-second wait after the incremental’s CHECKPOINT, I inserted id = 2 into a heap table and an AO table in the same transaction and committed.
By the time gpbackup checked pg_locks, the locks were gone and the heap file hash still matched the full backup. Restore succeeded, but only the AO table contained id = 2.

Could you check it out, please.

[liang8283]

@woblerr Thanks, both confirmed and fixed in 9ef673a.

Commit during the wait after CHECKPOINT. A row written after the CHECKPOINT can stay in shared buffers, so the file hash read a moment later does not see it, and once the writer commits before the lock check there is no lock left to catch it either. The hash is now verified after the backup snapshot is taken: gpbackup issues a second CHECKPOINT, re-reads the file info of every hashed table, and drops the hash of any table whose files changed since the first read. Any change committed before the snapshot is flushed by that second CHECKPOINT, so it can no longer hide behind a matching hash. The pre-snapshot lock check stays, since it covers rows written before the first CHECKPOINT and committed after the snapshot.

With your sequence (heap and AO insert committed in one transaction during the one-second wait), the incremental now includes the heap table and the restore returns the new row in both tables. To reproduce it reliably the row has to land in an existing page; a new row that extends the file moves its mtime immediately and was already caught.

ALTER TYPE ... RENAME VALUE. Enum values are stored by oid, so renaming a label changes the dumped text without touching the data files or the table's DDL timestamp. Each heap entry now also records a hash of the labels of the enum types its columns use, directly or through domains, arrays, composite types and range types, read inside the backup snapshot. A heap table is skipped only if the file hash, the DDL timestamp and this enum-label hash all match. With your scenario the table is included and restored as queued; I also checked a column of a domain-over-enum array and a composite type with an enum attribute, and an unrelated heap table in the same database is still skipped.

The earlier scenarios (uncommitted writes, tail segments, the lock-wait race, DROP COLUMN, leaf partitions) still behave as before.

[woblerr]

@liang8283 Thanks for fixing the previous issues.
Changing the structure of a composite type still leaves the heap file hash, the table’s DDL timestamp and the enum-label hash unchanged, so the incremental skips the table.
Restore then loads records from the full backup into the updated type and fails with malformed record literal. An incremental from the same full backup without --heap-file-hash restores successfully.

Create a test database and table.

CREATE TYPE pair AS (a int, b int);
CREATE TABLE t(id int, v pair)
  USING heap WITH (autovacuum_enabled=false)
  DISTRIBUTED BY (id);

INSERT INTO t VALUES (1, ROW(10,20)::pair);
SELECT * FROM t;

Take a full backup and note its printed timestamp.

Add an attribute to the composite type.

ALTER TYPE pair ADD ATTRIBUTE c int CASCADE;
SELECT * FROM t;
-- (10,20,)

Take an incremental using the full backup’s timestamp.
Restore using the incremental’s timestamp:

ERROR: malformed record literal: "(10,20)"

[liang8283]

@woblerr Thanks, confirmed. Fixed in e90efe1. The enum fix only covered one case of a wider problem: restore loads a skipped table's old data file into the newest column list, and some changes alter that list or how stored values are dumped without touching the data files or the table's DDL timestamp. Besides ALTER TYPE ... ADD/DROP ATTRIBUTE, the same happens when a column is added to a table whose row type another table uses as a column type, and when DROP DOMAIN ... CASCADE drops a column.

The enum-label hash is replaced by a hash of the table's own columns (number, name, type, typmod, dropped) and of the definitions of every enum and composite type they use, directly or through domains, arrays, nested composite, range and multirange types, read inside the backup snapshot. The table is skipped only if this hash matches too. The query runs under a savepoint; if it fails, no heap table is skipped.

While going over the whole feature again I also fixed a few related cases:

• A table is now skipped only if the previous restore plan has data for it. A table that belonged to an extension in the previous backup (whose data is not backed up) and was then removed from it with ALTER EXTENSION ... DROP TABLE was skipped and restored empty.
• Unlogged tables are never hashed: a regular CHECKPOINT does not write their buffers, so their files may not move when they change.
• Hashes are keyed by oid until they are matched with the table's name inside the snapshot.
• gp_toolkit.gpbackup_file_info is dropped after each hashing step instead of being left in gp_toolkit.

With your steps the incremental now includes t, and the restore returns (10,20,). I also checked DROP ATTRIBUTE ... CASCADE, a composite nested in another composite, an array of a domain over the composite, the table row-type case, DROP DOMAIN ... CASCADE, the extension case, an unlogged table, RENAME COLUMN on a partitioned and on an inheritance parent, a typed table, and a full → incremental → incremental chain: each affected table is included and restored to match the source, and unrelated heap tables are still skipped. The earlier scenarios still behave as before.

[woblerr]

There is one more issue with regclass columns. Could you take a look?

With a heap table storing 'public.target'::regclass, taking a full backup and then running ALTER TABLE public.target RENAME TO renamed leaves the referencing table’s file hash, DDL timestamp and column hash unchanged, so the incremental skips it.
Restore then fails with relation "public.target" does not exist when loading the old reference from the full backup. An incremental from the same full backup without --heap-file-hash restores the reference successfully.

And branch conflicts need to be resolved.

[liang8283]

@woblerr Thanks, confirmed. Fixed in 016d651, which also merges main to resolve the conflicts.

regclass stores an oid but is dumped as the name of the relation it references, so renaming or dropping that relation changes what COPY writes, without touching the referencing table's files, its DDL timestamp or its column hash. The same holds for the other reg* types (regproc, regprocedure, regoper, regoperator, regtype, regconfig, regdictionary, regnamespace, regrole, regcollation) and for aclitem, which is dumped with role names. Tracking every referenced object's name per stored value is not practical, so heap tables that use any of these types, directly or through domains, arrays, composite and range types, are now never hashed and are always included in the incremental, as without --heap-file-hash.

Two smaller changes in the same commit:

• The column hash query now types its string literals explicitly ('a'::text || ...), the same fix as 8da3023 on main, so unknown || oid cannot become ambiguous on PostgreSQL 14 based servers.
• --heap-file-hash now requires Greenplum 6 or later (or Cloudberry), since the column hash needs a recursive query; on Greenplum 5 it logs a warning and no heap table is skipped.

With your steps the incremental now includes the referencing table, and the restore returns renamed. I also checked a regclass inside a composite and in a domain array, a regproc after ALTER FUNCTION ... RENAME, and an aclitem after ALTER ROLE ... RENAME: each table is included and restored to match the source, and an unrelated heap table is still skipped. The earlier scenarios still behave as before.

[woblerr]

@liang8283 Huge work done, thanks!
The last issue is missing license header in backup/queries_incremental_internal_test.go.

Could you take a look?

[liang8283]

@woblerr Thanks! Added the license header in 9339cd6.

[woblerr]

LGTM