schema: review-report-v2
title: "CF7016 Fix LSN of logical decoding message — Review"
commitfest: 61
patch: 7016
author: Atsushi Torikoshi
canonical_message_id: d99c688994ab3a998afe26e61fe4f69f@oss.nttdata.com
head: 7cb9060dcde08ca63df1f0d701b0674e0e8ae811
diff_sha: 88d6a2831768958d8824f8c59c6feddeaec96889
verdict: "ready-for-committer (test follow-up required)"
blocker_split:
  verified: 0
  unverified: 0
tags:
  - code-review
  - commitfest-7016
  - lsn-logical-message
  - pg-review-artifact-v2
  - postgresql
  - review-report-v2
summary: "The one-line decode.c fix (buf->endptr to buf->origptr) is correct, safe, documented, and supported by pgsql-hackers thread consensus. One test-only MAJOR (F-001) requires strengthening 020_messages.pl to actually assert LSN correctness. Verdict: ready-for-committer (test follow-up required)."

CF7016 — Fix LSN of logical decoding message

• CommitFest: 61, Patch: 7016
• Title: Fix LSN of logical decoding message
• Author: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
• Canonical Message-ID: d99c688994ab3a998afe26e61fe4f69f@oss.nttdata.com
• Thread: pgsql-hackers thread 648032 ("Why is the LSN reported for pg_logical_emit_message() different from other decoded operations?")
• Frozen HEAD: 7cb9060dcde08ca63df1f0d701b0674e0e8ae811
• Scope identity (diff_sha): 88d6a2831768958d8824f8c59c6feddeaec96889
• Changed symbol: logicalmsg_decode (src/backend/replication/logical/decode.c:606-677)

Deterministic Verdict

ready-for-committer (test follow-up required)

• Blocker split: verified=0, unverified=0
• Findings: 1 MAJOR (test-only, verified), 1 MINOR (verified), 1 NIT (verified)
• No surviving BLOCKER. No correctness/security MAJOR or live thread objection. The MAJOR F-001 is test-only (the decode.c fix is correct; the test needs strengthening). Thread consensus supports the fix with no objections.
• Per the verdict rubric: test-only MAJORs => ready-for-committer (test follow-up required).

Scope

Uncommitted diff vs HEAD 7cb9060dcde:

The patch makes the LSN reported for pg_logical_emit_message() (the message_lsn exposed to output plugins and the logical replication protocol) equal to the start LSN of the LogicalMessage WAL record (buf->origptr), instead of the end LSN (buf->endptr). All other decoded DML records already use origptr. The change aligns the implementation with both the surrounding code and the existing documentation.

Findings

Verification Summary

• Verified: 3, Refuted: 0, Cannot-verify: 0
• Verified BLOCKER/MAJOR: 1 (F-001, test-only). Unverified BLOCKER/MAJOR: 0.
• No refuted findings to drop.

Repair Validation Summary

All verified findings have concrete code examples and HEAD-bound unified-diff patches with SHA-256 hashes. F-001's two-run TAP gate provides the strongest evidence: the test fails on base (endptr) and passes on fixed (origptr), proving it catches the bug. F-002's compile validation confirms the comment addition is syntactically correct. NIT-001 requires only a release note when backpatched.

Changed-Code Coverage

Coverage requirement

• Status: required
• Changed instrumentable paths: src/backend/replication/logical/decode.c

Measurement

• Status: measured
• Head: 7cb9060dcde08ca63df1f0d701b0674e0e8ae811
• Counter epoch: fresh — all .gcda files deleted before test run; verified 0 .gcda files before execution. After test: 1048 .gcda files including decode.gcda (2304 bytes). No stamp mismatch, merge mismatch, or profiling errors.

Execution lane

• Selected lane: host-disposable
• Configure: ./configure --enable-debug --enable-cassert --enable-tap-tests --enable-coverage
• CFLAGS: -O0 -g -fprofile-arcs -ftest-coverage
• Test command: make -C src/test/subscription check PROVE_TESTS=t/020_messages.pl
• Environment: LC_ALL=C LANG=C TZ=UTC
• Tools: gcc 15.2.0, gcov 15.2.0, gcovr 8.6, make 4.4.1, prove TAP::Harness v3.48 / Perl v5.40.1

Raw metrics

Changed-line coverage: 1/1 (100%). The single changed line (decode.c:671) has gcovr execution count 9 — executed 9 times during the test run. The line is a function call argument with no branches.

Changed line detail

path: src/backend/replication/logical/decode.c
line: 671
function: logicalmsg_decode
count: 9
source: ReorderBufferQueueMessage(ctx->reorder, xid, snapshot, buf->origptr,

Touched function diagnostic

The whole_function_branches total of 0 reflects gcovr's decision analysis heuristic which did not identify source-level conditional decisions on the changed line (line 671 is a function call, not a control-flow construct). The function does contain branches elsewhere (line 668 if (!message->transactional) with 3/6 true/false split), but these are outside the changed scope.

Uncovered regions

None. All changed executable lines are covered.

Uncovered branches

None. The changed line has no branches.

Excluded changed coordinates

None. No compiler-no-coverage-coordinate, structural-attribution, or raw-compiler-branch-attribution exclusions.

Non-instrumented changes

COV-* findings

None. The changed line is fully covered by the existing test t/020_messages.pl, which emits both transactional and non-transactional logical messages.

Residual limits

• Coverage measured only for the changed implementation file under the single test t/020_messages.pl. Whole-function line coverage (26/33) and branch data reflect execution under this test set only.
• The changed line (671) is a function call argument with no source-level conditional decision; changed-branch coverage is vacuously 0/0.

pgsql-hackers Thread Ledger

• Thread ID: 648032
• Subject: "Why is the LSN reported for pg_logical_emit_message() different from other decoded operations?"
• Root Message-ID: d99c688994ab3a998afe26e61fe4f69f@oss.nttdata.com
• Status: reconciled — consensus supports the origptr fix; no objections.
• Ledger report slug: commitfest-7016-thread-ledger
• Committers active: None yet.

Message chronology (8 messages)

Per-finding dispositions

Development-branch catversion exception

Not applicable — no catalog change in this patch. No catversion bump required.

Security Review

No security findings. The one-line LSN change aligns logical messages with DML ordering, closes a consumer-side data-loss pitfall, and touches no privilege boundary, catalog write, SQL-string construction, crypto path, or memory-safety-sensitive code. The test-only filter removal introduces no injection surface. Safe from the pg-security-reviewer perspective.

• Privilege/role escalation: NOT APPLICABLE (CWE-269)
• Catalog write race / TOCTOU: NOT APPLICABLE (CWE-367)
• Ordering/delivery guarantees: IMPROVED (closes data-loss gap, not regressed)
• SQL injection in test change: NOT APPLICABLE (CWE-89)
• C memory safety: CLEAN (scalar swap of two XLogRecPtr values; no allocation, pointer arithmetic, or lifetime change)
• Crypto/TLS/superuser guards: NOT APPLICABLE

Aggregated Test Plan

1. F-001 TAP test (020_messages.pl): Two assertions (lsn < $..._lsn::pg_lsn)::text with expected 'true|1|pgoutput' (transactional) and 'true|77|0' (non-transactional). Two-run gate: FAIL on base (endptr), PASS on fixed (origptr). Promote to Aggregated Test Plan.
2. F-002: No runtime test (comment-only). Compile gate only.
3. NIT-001: No test (release note only).

Residual risk

• The decode.c fix is correct and low-risk. The primary residual is the test-strengthening follow-up (F-001): without it, a future regression of the endptr→origptr change would not be caught by 020_messages.pl.
• When backpatched, consumers (e.g. Debezium) that stored the pre-fix message LSN as a restart point will observe a small LSN delta (from end-of-record to start-of-record). A release note is advisable (NIT-001).
• No committer has weighed in on the thread yet. No live objection blocks the fix.

Provenance / Model routes

All handoff reports verified by the coordinator via get_report: source postgresql-master, scope_identity 88d6a2831768958d8824f8c59c6feddeaec96889, metadata schema amauta-review-handoff/v1.

v2 Artifact Validation

The structured pg-review-artifact-v2 JSON was assembled and validated with validate_review_artifact.py. Result: VALIDATION PASSED. Exit code 0.

Maintainer

decode.c recent top authors (3-year window): Peter Eisentraut (4), Álvaro Herrera (3), Masahiko Sawada (3). Last commit touching logicalmsg_decode: 1fdbca159e00 (Masahiko Sawada, 2026-01-28).

Filesystem boundary compliance

• Review worktree was READ-ONLY; never modified, committed, staged, or pushed.
• All candidate edits, builds, tests, and coverage counters were in disposable scratch space.
• All durable citations use repo-relative paths; no local absolute paths or credentials persisted.