Skip to content

Plan: logical decoding for columnar tables (design review, no implementation) - #187

Open
jdatcmd wants to merge 2 commits into
mainfrom
design/logical-decoding
Open

Plan: logical decoding for columnar tables (design review, no implementation)#187
jdatcmd wants to merge 2 commits into
mainfrom
design/logical-decoding

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Jul 27, 2026

Copy link
Copy Markdown
Owner

@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:

  • Logical decoding dispatches per resource manager, through RmgrData.rm_decode, which does exist as a callback.
  • A decoded change is heap-shaped (ReorderBufferChange holds HeapTuple), but that is less limiting than it looks: a HeapTuple can be built with heap_form_tuple from values that never lived on a heap page.
  • Every byte of our data reaches WAL as a full-page image via log_newpage / log_newpage_buffer, plus one XLOG_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 metadata catalog is heap and is decoded today. A consumer can watch row groups appear right now and cannot see a single user value.

The recommendation

LogLogicalMessage() — a core mechanism using an existing record type (RM_LOGICALMSG_ID), decoded by core in logicalmsg_decode and 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 flush after 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.vacuum can 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, compact and recluster move 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. pgoutput delivers 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.md only.

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
@ChronicallyJD

Copy link
Copy Markdown
Collaborator

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 having

I reproduced what a slot sees rather than take it. Columnar table, two inserts, one update, one delete, test_decoding subscribed to everything:

naming the columnar table            0
naming pgcolumnar internal catalogs  33     (zone_map 16, column_chunk 8,
naming an equivalent heap table       2      row_group 4, delete_vector 3, storage 2)

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 it

Keep 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. pg_upgrade and any recovery on a host where the extension is absent or a different build cannot replay. Crash recovery is not a feature of the extension; it is a property of the cluster, and a design that moves it inside our .so has changed what the cluster is. LogLogicalMessage replaying as a no-op is not a convenient detail — it is the whole reason the recommendation is safe, and it should be stated as the acceptance criterion for any future alternative.

Option B: there may be a way round the retention hazard, and it is not retention

You rejected B because compaction or pgcolumnar.vacuum can reclaim a group before a lagging slot reads it, leaving the stream silently short, and the fix looked like teaching every reclamation path about slots.

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

vacuum, compact and recluster moving rows between groups must emit nothing, and a rewrite replayed as inserts duplicates the table downstream. That is the risk I would rank first too.

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 columnar_index.c follows for the rewrite path — it enforces nothing because the rows already satisfied the constraints — and stating it once gives future paths a rule to check themselves against.

The objection: phase 1 delivering inserts only is worse than delivering nothing

This 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 per-table option refuses to turn on until update and delete are covered, so phase 1 is reachable only by a test, or
  • phase 1's option is named for what it is (emit_inserts, not logical_decoding), so nobody can enable it believing they have change capture

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 things

You 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 LogLogicalMessage gaining flush after 15: worth writing the boundary in the plan as the version it actually changed at rather than "after 15", and giving it its own macro rather than folding it in with another. Two signatures that move at different versions under one guard is how main stopped compiling on 17 once, and I am the one who did it.

…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
@jdatcmd

jdatcmd commented Jul 28, 2026

Copy link
Copy Markdown
Owner Author

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 one

Accepted 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 emit_inserts and never logical_decoding.

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 it

Teaching 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 shape

Both 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 columnar_index.c already follows on the rewrite path.

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 wrote

I wrote "after 15". Checked across all five headers rather than inferring from two:

15.18      LogLogicalMessage(prefix, message, size, transactional)
16.14      LogLogicalMessage(prefix, message, size, transactional)
17.10      LogLogicalMessage(prefix, message, size, transactional, flush)
18.4       ... flush
19beta2    ... flush

It changed at 17, not after 15, so the guard is >= 170000 and 16 takes the old branch. Had I written the guard from my own sentence, 16 would have taken the wrong one. That is #163's shape exactly, which is presumably why you asked. It has its own macro in the plan.

Your reproduction

Your 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants