Validate remote sequence data in sequencesync worker

[NikolayS]

Defect

get_and_validate_seq_info() in src/backend/replication/logical/sequencesync.c extracted most columns of the result set returned by the publisher's sequence-synchronization query with a bare Assert(!isnull):

• column 1 seqidx
• column 4 is_called
• column 5 page_lsn
• column 6 seqtypid
• columns 7-11 seqstart, seqincrement, seqmin, seqmax, seqcycle

Remote responses must be validated at runtime rather than assumed well-formed. An incompatible, broken, or malicious publisher returning NULL in any of these columns would crash assert-enabled builds; production builds would interpret the NULL datum as zero (feeding garbage such as a zero page_lsn or bogus sequence parameters into local sequence state via SetSequence() and UpdateSubscriptionRelState()), or dereference a null pointer on platforms where int64 is passed by reference.

Additionally, the echoed seqidx was used with list_nth() without verifying that it identifies a unique member of the current batch. A malformed response could therefore cause an out-of-bounds access, resulting in undefined behavior, associate returned state with the wrong local sequence, or corrupt the batch_missing_count accounting so that genuinely missing rows escape reporting.

This is the same defect class as commit 3cf5264 ("Check CREATE_REPLICATION_SLOT response shape in libpqwalreceiver"), one layer up: that commit hardened the replication-command response shape; this one hardens the SQL result values consumed by the sequencesync worker.

What changed

• Added a small helper slot_getattr_notnull() that raises ereport(ERROR, errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("invalid response from publisher"), errdetail(...)) naming the affected sequence and column, following the message style of libpqwalreceiver's existing response-shape checks.
• Converted all eight Assert(!isnull) value extractions to use that helper.
• Converted the seqidx NULL assert to a runtime error and added batch membership + uniqueness validation: copy_sequences() passes the batch base index, batch size, and a per-batch bool *batch_seen array (palloc0_array, batches are capped at 100) into get_and_validate_seq_info(), which rejects any index outside [batch_base, batch_base + batch_size) and any index seen more than once in the same batch — all before list_nth() or any seqinfo update. This is strictly stronger than a global range check: an out-of-batch or duplicate index would update the wrong LogicalRepSequenceInfo and corrupt batch_missing_count.
• Declared the seqidx result column as INT4OID (matching the %d int4 VALUES column the query actually returns) instead of INT8OID narrowed with DatumGetInt32(), so a malformed value is rejected by input parsing rather than silently truncated before the range check.

Not changed, deliberately:

• Column-count validation is already performed one layer down by libpqrcv_processTuples() (ERRCODE_PROTOCOL_VIOLATION, "invalid query response").
• Columns 2 (has_sequence_privilege) and 3 (last_value) keep their explicit, semantically meaningful NULL handling (concurrent drop / insufficient privilege, per 55f5186).
• The Assert(col == REMOTE_SEQ_COL_COUNT) sanity check is purely local bookkeeping and stays an assert.

Test evidence

Built with meson setup --buildtype=debug -Dcassert=true -Dtap_tests=enabled; full rebuild clean, and sequencesync.c compiles warning-free with -Werror. pgindent --diff on the file is empty.

Full setup + subscription meson suites with this exact code (post batch-validation and INT4OID changes):

Ok:                 41
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            1
Timeout:            0

The skip is 012_collation (ICU disabled in this build). 036_sequences passed (exit status 0, 7.08s); it exercises the modified code path end-to-end (initial sequence sync, REFRESH PUBLICATION, REFRESH SEQUENCES, mismatch/warning cases). Note the existing tests exercise only valid responses; the new error branches are untested, since injecting a malformed response would require a mock publisher: libpqrcv_connect() runs ALWAYS_SECURE_SEARCH_PATH_SQL, forcing an empty search_path on the publisher session, so the sync query's unqualified references resolve only in pg_catalog and cannot be shadowed from a TAP test.

Closes #62

🤖 Generated with Claude Code

https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP