[BUG] Incorrect historic snapshot may be serialized to disk during fast-forwarding
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t52723psql -h localhost -U postgresBuilt from patchset v10 (message #10), September 09, 2026 at 02:38 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t52723_10 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t52723_10 && git checkout t52723_10Patchset v10 (message #10) is on t52723_10
Hi,
When working on another historic snapshot's bug in [1]/messages/by-id/tencent_21E152AD504A814C071EDF41A4DD7BA84D06@qq.com, I find the $subject.
Here is a test case, but we need to add some log in SnapBuildSerialize() first:
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 6e18baa33cb..6d13b2d811b 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1523,6 +1523,19 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
/* consistent snapshots have no next phase */
Assert(builder->next_phase_at == InvalidTransactionId);
+ StringInfoData logbuf;
+ initStringInfo(&logbuf);
+ appendStringInfo(&logbuf, "SnapBuildSerialize: lsn: %X/%08X xmin: %u, xmax: %u, committed: ",
+ LSN_FORMAT_ARGS(lsn), builder->xmin, builder->xmax);
+ for (size_t i = 0; i < builder->committed.xcnt; i++)
+ {
+ if (i > 0)
+ appendStringInfoString(&logbuf, ", ");
+ appendStringInfo(&logbuf, "%u", builder->committed.xip[i]);
+ }
+ elog(LOG, "%s", logbuf.data);
+ pfree(logbuf.data);
+
/*
* We identify snapshots by the LSN they are valid for. We don't need to
* include timelines in the name as each LSN maps to exactly one timeline1) create table t (id int) with (user_catalog_table = true);
2) select pg_create_logical_replication_slot('s1', 'test_decoding');
3) select pg_create_logical_replication_slot('s2', 'test_decoding');
4) insert into t values (1);
5) select pg_replication_slot_advance('s1', pg_current_wal_lsn());
6) select pg_logical_slot_get_changes('s2', pg_current_wal_lsn(), null);
Then we will find some log like this:
LOG: SnapBuildSerialize: lsn: 0/017D1318 xmin: 768, xmax: 768, committed:
STATEMENT: select pg_replication_slot_advance('s1', pg_current_wal_lsn());
LOG: SnapBuildSerialize: lsn: 0/017D1318 xmin: 768, xmax: 769, committed: 768
STATEMENT: select pg_logical_slot_get_changes('s2', pg_current_wal_lsn(), null);
At the same lsn, we get two different historic snapshots, and the first one (which is incorrect) is serialized to disk.
The main reason is that we don't handle XLOG_HEAP2_NEW_CID during fast-forwarding, so we don't consider the insert as having a catalog change.
Attach a patch to fix it.
Looking forward to your reply.
[1]: /messages/by-id/tencent_21E152AD504A814C071EDF41A4DD7BA84D06@qq.com
/messages/by-id/tencent_21E152AD504A814C071EDF41A4DD7BA84D06@qq.com
--
Regards,
ChangAo Chen
Attachments:
v1-0001-Handle-XLOG_HEAP2_NEW_CID-in-heap2_decode-even-if.patchapplication/octet-stream; charset=utf-8; name=v1-0001-Handle-XLOG_HEAP2_NEW_CID-in-heap2_decode-even-if.patchDownload+4-3
Hi ChangAo,
Thanks for your analyze and report.
At 2025-11-23 00:07:59, "cca5507" <cca5507@qq.com> wrote:
The main reason is that we don't handle XLOG_HEAP2_NEW_CID during fast-forwarding, so we don't consider the insert as having a catalog change.
Yeah. Refers to code below:
```
case XLOG_HEAP2_NEW_CID:
if (!ctx->fast_forward)
{
xl_heap_new_cid *xlrec;
xlrec = (xl_heap_new_cid *) XLogRecGetData(buf->record);
SnapBuildProcessNewCid(builder, xid, buf->origptr, xlrec);
break;
}
```
It is clear that transaction is skipped set has catalog change (in SnapBuildProcessNewCid) duraing fast forward.
However, in practice this does not cause any actual issues, because for the "real" system catalogs (those not
specified via user_catalog_table and thus critical for decoding), any HEAP2_NEW_CID changes are always accompanied
by some invalid messages. As a result, they are ultimately marked as catalog changes.
Still, from a code perspective, I would prefer fixing this behavior for logical consistency across the snapshot
builder, even if it doesn’t manifest as a runtime problem today. But we can not provide a test case.
Attach a patch to fix it.
The patch in attachment is better for me. What do you think?
Bset regards,
Haiyang Li
Attachments:
v2-0001-Handle-XLOG_HEAP2_NEW_CID-in-fast-forward.patchapplication/octet-stream; name=v2-0001-Handle-XLOG_HEAP2_NEW_CID-in-fast-forward.patch; x-cm-securityLevel=0Download+9-6
Hi Haiyang,
The patch in attachment is better for me. What do you think?
The v2-0001 LGTM.
A small suggestion:
We should move the 'break' out of the 'if', because we don't want it fall through to XLOG_HEAP2_REWRITE if we are fast-forwarding.
--
Regards,
ChangAo Chen
Hi ChangAo,
At 2025-11-23 13:31:49, "cca5507" <cca5507@qq.com> wrote:
The patch in attachment is better for me. What do you think?
The v2-0001 LGTM.
A small suggestion:
We should move the 'break' out of the 'if', because we don't want it fall through to XLOG_HEAP2_REWRITE if we are fast-forwarding.
Fair. The patch updated is provided in attachment.
Best regards,
Haiyang Li
Attachments:
v3-0001-Handle-XLOG_HEAP2_NEW_CID-in-fast-forward.patchapplication/octet-stream; name=v3-0001-Handle-XLOG_HEAP2_NEW_CID-in-fast-forward.patch; x-cm-securityLevel=0Download+10-8
Hi,
I add some commit message to the patch and create a CF entry:
https://commitfest.postgresql.org/patch/6304/
--
Regards,
ChangAo Chen
On Sat, Nov 22, 2025 at 10:33 PM ocean_li_996 <ocean_li_996@163.com> wrote:
Hi ChangAo,
At 2025-11-23 13:31:49, "cca5507" <cca5507@qq.com> wrote:
The patch in attachment is better for me. What do you think?
The v2-0001 LGTM.
A small suggestion:
We should move the 'break' out of the 'if', because we don't want it fall through to XLOG_HEAP2_REWRITE if we are fast-forwarding.
Fair. The patch updated is provided in attachment.
While I agree with your analysis, I'm not sure what actual problems it
could lead to in practice. Have you had a chance to reproduce this
behavior by using DDLs instead of a user-catalog table? IIUC the
problem can occur if a transaction makes catalog changes and writes
only NEW_CID WAL records without INVALIDATION WAL records. However,
I'm not sure there are such transactions in practice. IIUC it would
not be a problem in terms of logical decoding even if we don't include
their XIDs to the snapshot if they change only user-catalog tables. It
might be more future proof to mark transactions as catalog-changed
even when fast-forwarding a NEW_CID record, as you proposed, but I'd
like to confirm the actual problems first.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Hi,
While I agree with your analysis, I'm not sure what actual problems it
could lead to in practice. Have you had a chance to reproduce this
behavior by using DDLs instead of a user-catalog table?
I'm not sure if this can be reproduced by using DDLs, but I will try.
IIUC the problem can occur if a transaction makes catalog changes and writes
only NEW_CID WAL records without INVALIDATION WAL records.
+1
However, I'm not sure there are such transactions in practice.
Maybe currently not, but what about future, I'm not sure yet.
IIUC it would not be a problem in terms of logical decoding even if we
don't include their XIDs to the snapshot if they change only user-catalog
tables. It might be more future proof to mark transactions as catalog-changed
even when fast-forwarding a NEW_CID record, as you proposed, but I'd
like to confirm the actual problems first.
Our current design of historic snapshot is to track all catalogs even if only a part
of them are useful for logical decoding. So I think this bug breaks our design even
if it's maybe ok in practice.
--
Regards,
ChangAo Chen
Hi,
The commit message of 'user_catalog_table' says:
```
Replication solutions built around the logical decoding machinery
will likely need to set this operation for their configuration
tables; it might also be needed by extensions which perform table
access in their output functions.
```
So if there is an extension using the incorrect snapshot to scan user
catalog tables, it will get the wrong result, right?
--
Regards,
ChangAo Chen
Hi Masahiko, ChangAo,
Masahiko Sawada <sawada.mshk@gmail.com> 2025-12-30 01:10:01 wrote:
It might be more future proof to mark transactions as catalog-changed
even when fast-forwarding a NEW_CID record, as you proposed, but I'd
like to confirm the actual problems first.
Yeah, I haven’t identified any actual issue in practice. My observations are
purely based on code-level logic analysis.
cca5507 <cca5507@qq.com> Sat, 17 Jan 2026 23:56:17 +0800 wrote:
The commit message of 'user_catalog_table' says:
```
Replication solutions built around the logical decoding machinery
will likely need to set this operation for their configuration
tables; it might also be needed by extensions which perform table
access in their output functions.
```So if there is an extension using the incorrect snapshot to scan user
catalog tables, it will get the wrong result, right?
I agree with your analysis. The introduction of user_catalog_table is intended to
ensure that the historical MVCC in logical replication can observe the historical
state of this table. Consequently, transactions on this table should be tracked by
the historical snapshot — even while in the fast forward state.
Regards
Haiyang Li
Hi ChangAo, Haiyang, and Masahiko,
I reviewed v4 at the CommitFest branch d4014e6ed86. On the unpatched
parent, I reproduced both the incorrect serialized snapshot
and the resulting failure in an output plugin's catalog lookup. I verified
that v4 fixes both cases.
Following Masahiko's question about a DDL-only reproducer, I inspected the
WAL generated in my test environment by:
* CREATE TABLE
* ALTER TABLE ADD COLUMN
* CREATE INDEX
* COMMENT ON TABLE
* GRANT on a table
* DROP TABLE
Each tested case generated both XLOG_HEAP2_NEW_CID and
XLOG_XACT_INVALIDATIONS. The latter already causes
ReorderBufferXidSetCatalogChanges() to be called, including during
fast-forwarding. Therefore, these transactions were tracked as
catalog-changing even without v4, and I could not reproduce the bug with
these representative DDL statements.
I also attached a test-only patch that fast-forwards over a NEW_CID without
XLOG_XACT_INVALIDATIONS and verifies that the transaction's XID is retained
in the serialized snapshot. The test fails without v4 and passes with it.
The patch applies on top of v4-0001 and can be folded into a future
revision if the approach is acceptable.
Regards,
Yuya Shinde
On Mon, Jan 26, 2026 at 11:19 PM ocean_li_996 <ocean_li_996@163.com> wrote:
Show quoted text
Hi Masahiko, ChangAo,
Masahiko Sawada <sawada.mshk@gmail.com> 2025-12-30 01:10:01 wrote:
It might be more future proof to mark transactions as catalog-changed
even when fast-forwarding a NEW_CID record, as you proposed, but I'd
like to confirm the actual problems first.Yeah, I haven’t identified any actual issue in practice. My observations
are
purely based on code-level logic analysis.cca5507 <cca5507@qq.com> Sat, 17 Jan 2026 23:56:17 +0800 wrote:
The commit message of 'user_catalog_table' says:
```
Replication solutions built around the logical decoding machinery
will likely need to set this operation for their configuration
tables; it might also be needed by extensions which perform table
access in their output functions.
```So if there is an extension using the incorrect snapshot to scan user
catalog tables, it will get the wrong result, right?I agree with your analysis. The introduction of user_catalog_table is
intended to
ensure that the historical MVCC in logical replication can observe the
historical
state of this table. Consequently, transactions on this table should be
tracked by
the historical snapshot — even while in the fast forward state.Regards
Haiyang Li