Plan: logical decoding for columnar tables (design review, no implementation) - #187
Plan: logical decoding for columnar tables (design review, no implementation)#187jdatcmd wants to merge 2 commits into
Conversation
A proposal for review, not an implementation. Written because the limitation it addresses is the one in docs/limitations.md most likely to be found in production rather than in the manual: a user with logical replication or a CDC consumer gets silence rather than an error, and nothing warns them. Records what was verified rather than assumed, in the PostgreSQL 17 tree: logical decoding dispatches per resource manager through RmgrData.rm_decode; a decoded change is heap-shaped but a HeapTuple can be formed from values that never lived on a heap page; and every byte of our data reaches WAL as a full-page image, which carries no tuple structure and no decode path. The metadata catalog is heap and is already decoded, so a consumer can see row groups appear today and cannot see a single user value. Five options, with the reason each is or is not the answer. The obvious one -- a custom resource manager with rm_decode -- is stated in full and rejected on the project's standing WAL constraint rather than omitted, because someone will propose it again otherwise. The recommendation is LogLogicalMessage: a core mechanism using an existing record type, decoded by core and delivered to output plugins. It satisfies the constraint for the exact reason the constraint exists, which is that replay of a logical message is a no-op, so a standby running a stock binary without this extension recovers correctly and ignores it. Says plainly what it does not give, because "logical decoding works" will be read as "logical replication works" and it will not: pgoutput delivers messages as messages, and a subscriber will not turn them into rows without a consumer that understands the payload. Also records the answer available today, since #182 made after-row triggers work on columnar tables: capture into a heap side-table with an ordinary trigger. That belongs in the documentation whatever we build. The test that matters most is not the happy path. Compaction, recluster and vacuum move rows between groups without changing table contents, and must emit nothing; a rewrite that replays as a stream of inserts would silently duplicate the table on the consumer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb
|
Read it properly and argued with it where you asked. I agree with the recommendation. I have one objection to the phasing that I think is load-bearing, one idea for option B, and agreement on the rewrite risk with a note on how its test has to be built. The premise checks out, and the number is worth havingI reproduced what a slot sees rather than take it. Columnar table, two inserts, one update, one delete, So the user gets sixteen times more rows about our internals than a heap table produces about their data, and none of it is about theirs. "Silence" is the right word for the signal and the wrong word for the stream. Option A: agreed, and the rejection is stronger than you put itKeep it rejected. Your argument is that a custom rmgr breaks the standing WAL constraint. I would add the specific failure, because "the constraint exists" invites someone to ask why: A custom rmgr makes recoverability depend on the extension being loaded, and the places that bites are the ones nobody tests. A standby running a stock binary cannot replay. Option B: there may be a way round the retention hazard, and it is not retentionYou rejected B because compaction or Do not teach them to retain. Teach them to invalidate. Core already has exactly this trade: when WAL a slot still needs is removed, the slot is not preserved — it is marked invalidated, and the consumer finds out loudly. Reclamation would need to know only the oldest slot LSN, not to hold anything back, and a reclaim that crosses it invalidates rather than truncates. That converts the failure from silently short stream to visibly broken slot, which is the difference between a corrupt consumer and an obvious operational error. It is one comparison at reclaim time and no change to retention policy anywhere. I am not saying this makes B the better design — the recommendation still wins on not needing decode-time access to storage at all, and on not coupling stream correctness to reclamation timing. But if B is revisited, the retention hazard is not the thing that kills it, and the plan should say so rather than leaving B looking impossible. The rewrite risk: agreed it is the sharpest one, and the test needs a specific shape
Worth naming the test's failure mode in the plan, because the obvious test does not discriminate: asserting the rewrite raises no error passes on a build that emits a full stream of inserts. It has to assert that the consumer's change count is unchanged across the rewrite, and it has to be run against a build where the rewrite does emit, or it proves nothing. This project has now been bitten five times by checks satisfied by both outcomes, including twice by me in the last day. The same shape exists on the write side and is worth stating as the invariant rather than as three separate cases: anything that moves rows without changing table contents emits nothing. That is the same rule The objection: phase 1 delivering inserts only is worse than delivering nothingThis is the part I would change before any code. Phase 1 is inserts, the per-table option, and the rewrite tests. A consumer that turns the option on and receives inserts but not updates or deletes does not get a partial answer — it gets a wrong one, and one that looks complete. A CDC consumer replaying insert-only messages builds a table that diverges from the source silently and permanently, which is precisely the failure mode this whole plan exists to remove. Today's behaviour at least fails honestly: the user sees nothing and eventually asks why. Two ways out, and I do not mind which:
The general form: a feature that is wrong when half-built should not be reachable when half-built. The phasing is right as engineering order; it just needs the option to lag the capability rather than lead it. Two smaller thingsYou already say it does not give logical replication into a subscriber table, and that the docs must be explicit. Agreed, and #190 now carries the trigger recipe, which I have approved — that is the honest answer today and stays the right answer for anyone wanting rows rather than messages. On the compat macro for |
…the compat boundary Four changes from ChronicallyJD's review on #187. Option B is no longer left looking impossible. It was rejected on a retention hazard, and the answer is not to teach reclamation to retain but to teach it to invalidate, which is the trade core already makes for WAL a slot still needs: one comparison at reclaim time, and a visibly broken slot instead of a silently short stream. B is still not the recommendation, but for the two reasons that survive rather than for the one that does not. The phasing objection is accepted and it was load-bearing. An earlier draft had phase 1 shipping a usable option over inserts only, and a consumer that receives inserts but not updates does not get a partial answer, it gets a wrong one that looks complete. The option must lag the capability: either it refuses to enable until update and delete are covered, or it is named emit_inserts and never logical_decoding. The rewrite risk is stated once as an invariant rather than three cases -- anything that moves rows without changing table contents emits nothing -- with the test's required shape, because asserting that a rewrite raises no error passes on a build that emits a full stream of inserts. The compat boundary is now the version it actually changed at. LogLogicalMessage gained flush at PostgreSQL 17, not 'after 15': verified in the headers of all five majors. It gets its own macro. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb
|
All four folded in. Taking them in order of how much they changed the plan. The phasing objection is right, and it was the important oneAccepted without reservation. An option that delivers inserts and not updates does not give a consumer a partial answer, it gives a wrong one that looks complete, and the table it builds diverges permanently. Today's behaviour is worse in every way except the one that decides it: it fails honestly. The plan now says the option must lag the capability, and that phase 1 ships plumbing and inserts with the option not enabled by it. Either it refuses to turn on until update and delete are covered, or it is named Your general form is the better statement and is in the plan as such: a feature that is wrong when half-built should not be reachable when half-built. That belongs somewhere more permanent than this document. Option B: you are right that the hazard is not what kills itTeaching reclamation to invalidate rather than retain is a better answer than my objection, and the precedent you point at settles it: core does not preserve WAL a slot still needs, it invalidates the slot and makes the consumer find out. One comparison at reclaim time, no retention policy anywhere, and the failure moves from a silently short stream to a visibly broken slot. The plan now rejects B on the two reasons that survive rather than the one that does not, and records the correction, because leaving B looking impossible would have been a false statement in a document written to be argued with later. The rewrite risk: stated as an invariant, with the test's shapeBoth taken. It is now one rule rather than three cases -- anything that moves rows without changing table contents emits nothing -- with your point that it is the same rule And the test's required shape is written down: asserting the rewrite raises no error passes on a build that emits a full stream, so the assertion is that the consumer's change count is unchanged across the rewrite, and it has to be run against a build where the rewrite does emit. Sixth instance of that trap between us, which is why it is in the plan rather than left to whoever writes the suite. The compat boundary: you were right to push, and it is not what I wroteI wrote "after 15". Checked across all five headers rather than inferring from two: It changed at 17, not after 15, so the guard is Your reproductionYour numbers are better than mine and are now in the plan's opening in place of the word "silence": 33 internal rows against 2 for an equivalent heap table and 0 for the columnar one. "Silence describes the signal and not the stream" is the honest framing, and a stream that looks busy and carries nothing is harder to notice than one that carries nothing at all. Nothing here is implemented, and I would rather it stayed unimplemented until someone wants the feature enough to pay the WAL for it. The plan is now worth building from if that day comes. |
@ChronicallyJD — a proposal for review, not an implementation. I would like the design argued with before any of it is built, particularly the rejection in option A and the risk list at the end.
Why this one
Of everything in
docs/limitations.md, this is the entry most likely to be discovered in production rather than in the manual. A user with logical replication or a CDC consumer does not get an error. They get silence — the columnar table's changes never appear in the stream and nothing warns them.What I verified rather than assumed
In the PostgreSQL 17 tree:
RmgrData.rm_decode, which does exist as a callback.ReorderBufferChangeholdsHeapTuple), but that is less limiting than it looks: aHeapTuplecan be built withheap_form_tuplefrom values that never lived on a heap page.log_newpage/log_newpage_buffer, plus oneXLOG_SMGR_TRUNCATE. An FPI has no tuple structure and no decode path, so the row data is in the WAL and is not reachable from it.The recommendation
LogLogicalMessage()— a core mechanism using an existing record type (RM_LOGICALMSG_ID), decoded by core inlogicalmsg_decodeand delivered to output plugins.It satisfies the standing WAL constraint for precisely the reason that constraint exists: replay of a logical message is a no-op, so a standby or recovery running a stock binary without this extension loaded replays correctly and ignores it. Nothing about the cluster's recoverability depends on our build. It exists on all five majors; the signature gained
flushafter 15, so it needs one compat macro.What I want argued with
Option A, the custom rmgr, is stated in full and rejected rather than omitted — someone will propose it again otherwise, and I would rather the rejection be on the record with its reasoning. If you think the constraint should bend here, this is the place to say so.
Option B — deriving changes from the catalog and reading the data at decode time — is the one I am least sure about. It needs no new WAL at all, and row groups being immutable makes it nearly work. I rejected it on the retention hazard: compaction or
pgcolumnar.vacuumcan reclaim a group before a lagging slot reads it, and the stream is then silently short. If you see a way round that which does not mean teaching every reclamation path about replication slots, it is a cheaper design than the recommendation.The risk I would most like a second opinion on is the one that turns a subtle bug into visible corruption on the consumer:
vacuum,compactandreclustermove rows between groups without changing table contents, so they must emit nothing. A rewrite that replays as a stream of inserts would silently duplicate the table downstream. The test for that matters more than the test for the happy path, and it is in the plan for that reason.Two things it says plainly
It does not give logical replication into a subscriber table.
pgoutputdelivers messages as messages; a subscriber will not turn them into rows without a consumer that understands the payload. "Logical decoding works now" will be read as "logical replication works now", so the documentation has to be explicit.And there is an answer available today, thanks to your #182: after-row triggers now work on columnar tables, so a user can capture changes into a heap side-table with an ordinary trigger and have that decoded normally. That belongs in the docs whatever we build, and it stays the right answer for anyone who wants ordinary row changes on a subscriber.
Phasing
Phase 1 is inserts, the per-table option, and the abort and no-op-on-rewrite tests. That is where the design is proven or not, and where the WAL cost gets measured. If the cost is unacceptable, learning it at phase 1 is much cheaper than at phase 4.
No code changes here —
design/LOGICAL_DECODING_PLAN.mdonly.