Hi Torikoshi-san, Sawada-san, I tested v3 on current master (8d87367b39e). It applies cleanly and builds with assertions, and contrib/test_decoding (16/16) and src/test/subscription/t/020_messages.pl pass.
I also checked the behavior with test_decoding. In a transaction with INSERT, pg_logical_emit_message(true, ...), INSERT and COMMIT, the message and the following INSERT were both reported at 0/017A1D08 before the patch. With v3 the message is reported at 0/017A1CC8, which is the start of the LogicalMessage record in pg_waldump, and the INSERT stays at 0/017A1D08. +1 to Sawada-san's two points (the comment about non-transactional messages, and the message_cb text in logicaldecoding.sgml). One more small idea: the existing tests pass both before and after the patch, so none of them covers this change. Maybe a small test in contrib/test_decoding could check that a transactional message and the next change get different LSNs. I did not test the non-transactional case or the back-branch patches. Thanks, João Em qua., 11 de nov. de 2026 às 19:14, Masahiko Sawada <[email protected]> escreveu: > > Hi, > > Sorry for the late reply. > > On Wed, Aug 12, 2026 at 12:16 AM torikoshia <[email protected]> > wrote: > > > > On 2026-08-11 09:04, Masahiko Sawada wrote: > > > > Thanks for the comment! > > > I'm still studying this patch but I want to clarify: was the problem > > > you faced caused by logical decoding showing different types of > > > changes(insert/update/delete/truncate/message) with the same LSN? > > > > Yes. The Debezium issue I encountered was caused by different types of > > changes being reported with the same LSN. > > > > More specifically, consider a transaction like this: > > > > begin; > > select pg_logical_emit_message(true, 'test1', 'xxx'); > > insert into t1 values (1, 'aaa'); > > commit; > > > > The problem occurred when Debezium received the logical message but > > crashed before receiving the INSERT. > > > > After a restart, Debezium uses the LSN of the last record it received > > to determine where to resume and which records have already been > > processed. Since the INSERT is reported with the same LSN as the > > logical message, it can be treated as already processed and skipped, > > resulting in data loss. > > > > > IIUC > > > logical decoding of MULTI_INSERT emits all INSERT changes with the > > > same LSN, so I think showing the same LSN multiple times is fine. > > > > So I agree that having the same LSN for multiple changes is not in > > itself a problem. The problem in this case is that changes originating > > from different WAL positions can be reported with the same LSN. > > > > > One possible reason why the message uses endptr instead of origptr is > > > for non-transactional messages; on receiver sides a non-transactional > > > message would be handled as a separate transaction, so it would be > > > useful to use endptr as the confirmed flush position. That said, it > > > doesn't apply for transactional messages. > > > > > While I agree that logical decoding uses origptr of logical decoding > > > messages as their LSN, I think having its endptr is also useful for > > > the above reason. For instance, I proposed to extend logical decoding > > > message handling[1] so that extension can define a function to handle > > > logical decoding messages on the subscriber. If we use origptr as the > > > flushed position, the same message is replicated again after the > > > server restart even if the subscriber has committed the message as a > > > separate transaction and sent an ack to the publisher. > > > > That makes sense. > > > > > So an alternative idea would be to use origptr for transactional > > > messages and endptr for non-transactional ones. With this idea, a > > > transactional message's LSN could coincide with the LSN reported for > > > the preceding COMMIT, or for a preceding non-transactional message. I > > > think that is acceptable for the same reason as MULTI_INSERT above: > > > what matters is that the LSN identifies the record the change came > > > from, not that it is unique. > > > > That approach seems reasonable to me. > > > > > > > I'm hesitant to backpatch it. What this issue tells us is that > > > consumers do look at the LSN of individual changes and possibly > > > persist it, and use it to decide where to resume and what has already > > > been processed. If we change the LSN we report in a minor release, a > > > position that a consumer recorded under the old behavior will be > > > interpreted under the new one after the upgrade. Whether that ends up > > > re-processing changes, skipping them, or failing to locate the resume > > > point depends on the consumer, and none of those seems like something > > > an operator should have to expect from a minor upgrade. > > > > > > There is no correctness problem within PostgreSQL here. So I think > > > this is pushed to master only, with the current behavior documented in > > > the back branches. Consumers hitting this can handle it on their side > > > in the meantime. > > > > This may be a safer approach. > > > > One thing that still concerns me is that consumer developers may > > assume that the LSN reported for a logical message identifies > > the WAL record containing that message, just as it does for other > > decoded changes. Even after consulting the documentation, one > > could reach that conclusion from this description: > > > > https://www.postgresql.org/docs/devel/protocol-logicalrep-message-formats.html > > > > > Message > > > .... > > > Int64 (XLogRecPtr) > > > The LSN of the logical decoding message. > > > > Also, as far as I imagine, the LSN of an individual decoded change is > > typically relevant mainly when a consumer has to determine its resume > > position after an unexpected interruption. Therefore, there may be > > consumers for which this behavior has not caused a visible problem > > during normal replication, and whose developers are not aware of this > > difference for logical messages. > > > > If that assumption is common, treating the documentation as defining > > the intended behavior and backpatching the change would make the fix > > easier for consumers: they could get the corrected behavior simply by > > upgrading PostgreSQL, without adding special handling for logical > > message LSNs only for PostgreSQL versions before 19. > > > > That said, I don't know whether there are actually many such consumers. > > It seems difficult to predict how changing the behavior in already > > released major versions might affect existing consumers. > > From that perspective, leaving the behavior unchanged in released > > branches seems safer. > > > > So I'm fine with changing this only in master and documenting the > > existing behavior in the back branches. > > Yeah, backpatching has another risk too: a consumer that has persisted > the LSNs of decoded messages would find them meaning something > different after a minor version update, and would have to behave > differently depending on the publisher's minor version. I don't think > we can expect consumers to handle that. > > > For the released branches, though, I think it would be useful not only > > to update the documentation, but also to make the issue clearly visible > > in the release notes, since some consumers may need to review or adjust > > their logics. > > Agreed. I'll describe the behavior difference in the commit message so > that it's picked up when the release notes are written. > > Regarding the patch: > > - ReorderBufferQueueMessage(ctx->reorder, xid, snapshot, buf->endptr, > + /* > + * Non-transactional messages are processed as separate transactions on > + * the receiver, so use endptr as the confirmed flush position for them. > + */ > + ReorderBufferQueueMessage(ctx->reorder, xid, snapshot, > + message->transactional ? buf->origptr : buf->endptr, > message->transactional, > > I think this needs more comments: what we assume on the consumer side > (a non-transactional message is applied and confirmed as a unit of its > own, like a commit), and what happens if we used origptr for a > non-transactional message instead; a consumer that reports the > message's LSN as its flush position can get the message decoded again > after a restart, since decoding resumes from the confirmed position > inclusive. It's the same reason why the commit message carries both > commit_lsn and end_lsn. > > Also, logicaldecoding.sgml says "The lsn has WAL location of the > message" for message_cb, which needs the same update, in the > back-branch patches too. > > Regards, > > -- > Masahiko Sawada > Amazon Web Services: https://aws.amazon.com > > > >
