Technical Analysis: Fix check_pub_rdt Bypass When Origin is Set in Same ALTER SUBSCRIPTION
Core Problem
This patch addresses a subtle logic bug in PostgreSQL's ALTER SUBSCRIPTION command processing where two options — retain_dead_tuples and origin — interact incorrectly when set in the same command.
Background: retain_dead_tuples and origin
The retain_dead_tuples feature (introduced in recent PostgreSQL versions) allows a subscription to signal to the publisher that it should not vacuum dead tuples that the subscriber might still need. When this option is enabled, PostgreSQL needs to verify that the publisher meets certain requirements (version compatibility, not in recovery, etc.) — this verification is gated by a flag called check_pub_rdt.
The origin option controls which origins of replicated data the subscription will accept (e.g., 'none' means only accept data originating from the publisher itself, not cascaded replication data).
The Bug
When a user issues:
ALTER SUBSCRIPTION SET (retain_dead_tuples = true, origin = 'none');
The internal processing of these options happens sequentially within the same command. The processing of retain_dead_tuples = true correctly sets check_pub_rdt = true, signaling that publisher validation checks must be performed. However, the origin option handler subsequently uses a simple assignment (check_pub_rdt = false) rather than a conditional one, unconditionally overwriting the flag regardless of its current state.
This means the critical publisher compatibility check is silently bypassed, potentially allowing retain_dead_tuples to be enabled against a publisher that doesn't support it (e.g., an older version or a standby in recovery). This could lead to incorrect behavior or errors at runtime rather than being caught at DDL time.
The Fix
The fix is minimal but important: changing the assignment operator from = to |=:
// Before (buggy):
check_pub_rdt = false; // origin handler unconditionally clears flag
// After (fixed):
check_pub_rdt |= false; // preserves existing true value if already set
With |=, if check_pub_rdt was already set to true by the retain_dead_tuples handler, the |= false operation preserves the true value. The flag can only be set, never cleared by a subsequent option in the same command.
Architectural Significance
This bug highlights a common pattern problem in multi-option DDL processing: option interdependence and processing order sensitivity. PostgreSQL's ALTER SUBSCRIPTION processes options in a loop, and each option handler must be careful not to clobber state set by previously-processed options. The |= pattern ensures monotonic flag accumulation — once a check is needed, no subsequent option processing can disable it.
This is particularly important for security/safety checks: the publisher validation for retain_dead_tuples exists to prevent data corruption or unexpected behavior, and a logic bug that silently bypasses it undermines the safety guarantees.
Patch Structure
- 0001: The actual code fix (likely a one-line change in
src/backend/commands/subscriptioncmds.c) - 0002: A tap test that reproduces the issue using a standby setup, demonstrating that without the fix, the publisher check is incorrectly skipped
The author notes the test is not fully polished but is provided for reviewer reproduction purposes.
Assessment
This is a straightforward correctness fix for a flag-handling bug. The fix is minimal and low-risk. The main discussion points for review would be:
- Whether there are other similar flag interactions in the same code path that could have the same issue
- Whether the tap test should be included (and polished) for regression coverage
- Whether this warrants backpatching to whatever version introduced
retain_dead_tuples