Logical replication row filter loses unchanged toasted columns

Dear Kato-san,

I found a bug in the row filter's UPDATE to INSERT transformation. An
unchanged column that is stored out-of-line silently becomes NULL on
the subscriber, unless it is part of the replica identity.

I could also reproduce the failure with your reproducer.

ALTER TABLE t ALTER COLUMN body SET STORAGE EXTERNAL;
CREATE PUBLICATION p FOR TABLE t WHERE (id = 7);
INSERT INTO t VALUES (3, repeat('a', 5000));

Note that I could reproduce without setting the storage parameter to EXTERNAL.
E.g., we can put string which has lower compression rate, like below.

```
CREATE EXTENSION pgcrypto;
INSERT INTO t SELECT 3, string_agg(encode(gen_random_bytes(1000), 'hex'), '') FROM generate_series(1, 5);
```

We cannot simply fill the value in. As Petr put it when
LOGICALREP_COLUMN_UNCHANGED was being discussed [1], such values "are
not written to WAL nor accessible via historic snapshot", so the
output plugin never sees them.

The historic snapshot seems to aim reading old version of catalogs, but toasted tables
are not.

I see three ways to deal with this.

Option B would have performance regressions not only for logical replication but
also for normal workloads. We need to generate for narrower cases, e.g., check
the filtering rule and generate WAL for unchanged toasted columns, if we want to
choose this. I'm not sure it's possible or how engineering would be needed though.

So, I prefer to 1) implement option A for all branches first, and 2) investigate
option B separately. Regarding the C, it can be chosen if the option A needs lots
of codes.

The only way forward is
pg_replication_slot_advance() or recreating the subscription.

Can we also use ALTER SUBSCRIPTION SKIP?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hi Kato-san,

I see three ways to deal with this.

Option A: detect the missing value in pgoutput_row_filter() and raise
an error naming the table and the column, trading silent data loss for
a loud failure. The catch is that the error is not recoverable. Once
the change is in WAL, neither ALTER TABLE ... REPLICA IDENTITY FULL
nor dropping the row filter helps, because decoding uses the historic
catalog snapshot from the time of the change. The only way forward is
pg_replication_slot_advance() or recreating the subscription.

If the subscriber apply worker raises the error instead, replication stops
on that
one transaction and ALTER SUBSCRIPTION ... SKIP steps over exactly it.
That has been available since 15, so it covers every affected branch.
The user can also disable the subscription, repair the row by hand and
re-enable it. The failure is just as loud, but recovery is ordinary
rather than drastic. Additionally, if the subscriber was using "NOT NULL"
constraint on such a column, the issue will be handled similarly anyways.

The apply worker has enough context for a useful message: it knows the
remote relation and the column name, and can hint at REPLICA IDENTITY
FULL. It knows less about the cause than the publisher does, but the
hint can cover that.

One detail if this route is taken: the check should not go into
slot_store_data() itself. That function is also used for old tuples in
apply_handle_update() and apply_handle_delete(), and in
apply_handle_update_internal() and apply_handle_tuple_routing() it
materialises an UPDATE's new tuple purely for conflict reporting
(CT_UPDATE_MISSING and CT_UPDATE_ORIGIN_DIFFERS). In that last case an
unchanged out-of-line column is perfectly legitimate, so a check there
would fire on ordinary updates whose target row happens to be missing
locally.

apply_handle_insert(), immediately after slot_store_data(), looks like
the right place, and appears to be the only one needed: a partitioned
target routes the already-stored slot through apply_handle_tuple_routing()
with CMD_INSERT, so one check covers both cases.

Option B: when a table belongs to a publication with a row filter,

make heap_update() log the whole old tuple, as it already does for
REPLICA IDENTITY FULL. The existing copy loop then finds the value,
the INSERT is complete, and no error is needed. This is the real fix,
but every UPDATE of such a table writes the unchanged out-of-line
value to WAL even when no transformation happens, which can be a large
regression. It also needs a new field in PublicationDesc, so I do not
think it can be back-patched.

The concern with option B is that every UPDATE of a row-filtered table
would write the unchanged out-of-line value even when no transformation
happens. I think that can be narrowed considerably: the extra logging
can be done based on HeapTupleHasExternal(), which is a single infomask bit
test requiring no deforming. ExtractReplicaIdentity() already uses
exactly that gate for REPLICA IDENTITY FULL.

The cost then falls on every UPDATE of a row-filtered table whose row
currently holds out-of-line values, rather than on every UPDATE of such
a table. Where the wide column is usually NULL or stays inline this is
close to free, and where every row is toasted the cost is real -- but
that is precisely the case where the current behaviour loses data.

What makes B attractive is that it needs no change to the output plugin,
the protocol or the subscriber. Once the old tuple carries the
flattened values, the copy loop already in pgoutput_row_filter() finds
them and the INSERT goes out complete. It is the same mechanism that
makes REPLICA IDENTITY FULL work today.

I agree it cannot be back-patched, for the reasons given: it needs to
know at heap_update() time that the table is published with a row
filter, and it would introduce a WAL volume regression in a minor
release.

Option C: document the restriction and leave the behavior alone. This
is the only option that changes nothing on the back branches, but the
value keeps disappearing without any warning.

I lean towards A because losing data silently seems worse than

stopping, but the unrecoverable error bothers me. Which approach do
you prefer, and should the fix be back-patched?

Rather than choosing among the three, would this combination work?

- back branches (15 and up): the subscriber-side error described
above, together with a documentation note in the UPDATE
transformation section stating that a column which is stored
out-of-line, unchanged, and outside the replica identity cannot be
carried through the transformation, and that REPLICA IDENTITY FULL
avoids it.

- master: option B, so the INSERT is complete and no error is needed.

Option C then becomes the documentation half of the first item rather
than a standalone choice.

One thing worth being explicit about: back-patching an error changes
behaviour in a minor release. I still think it is the right trade, since the
alternative is undetectable data loss, and where the column is NOT NULL
replication already fails today—just with a constraint violation that points
at the symptom rather than the cause.

Thanks,
---
Nikhil Sontakke
PlanetScale

Option B: when a table belongs to a publication with a row filter,

make heap_update() log the whole old tuple, as it already does for
REPLICA IDENTITY FULL. The existing copy loop then finds the value,
the INSERT is complete, and no error is needed. This is the real fix,
but every UPDATE of such a table writes the unchanged out-of-line
value to WAL even when no transformation happens, which can be a large
regression. It also needs a new field in PublicationDesc, so I do not
think it can be back-patched.

The concern with option B is that every UPDATE of a row-filtered table
would write the unchanged out-of-line value even when no transformation
happens. I think that can be narrowed considerably: the extra logging
can be done based on HeapTupleHasExternal(), which is a single infomask bit
test requiring no deforming. ExtractReplicaIdentity() already uses
exactly that gate for REPLICA IDENTITY FULL.

The cost then falls on every UPDATE of a row-filtered table whose row
currently holds out-of-line values, rather than on every UPDATE of such
a table. Where the wide column is usually NULL or stays inline this is
close to free, and where every row is toasted the cost is real -- but
that is precisely the case where the current behaviour loses data.

What makes B attractive is that it needs no change to the output plugin,
the protocol or the subscriber. Once the old tuple carries the
flattened values, the copy loop already in pgoutput_row_filter() finds
them and the INSERT goes out complete. It is the same mechanism that
makes REPLICA IDENTITY FULL work today.

I took a close look at option B and it might not be so attractive (what
ever is? :-))
and might need invasive changes.

I agree it cannot be back-patched, for the reasons given: it needs to
know at heap_update() time that the table is published with a row
filter, and it would introduce a WAL volume regression in a minor
release.

Option C: document the restriction and leave the behavior alone. This
is the only option that changes nothing on the back branches, but the
value keeps disappearing without any warning.

I lean towards A because losing data silently seems worse than

stopping, but the unrecoverable error bothers me. Which approach do
you prefer, and should the fix be back-patched?

Rather than choosing among the three, would this combination work?

- back branches (15 and up): the subscriber-side error described
above, together with a documentation note in the UPDATE
transformation section stating that a column which is stored
out-of-line, unchanged, and outside the replica identity cannot be
carried through the transformation, and that REPLICA IDENTITY FULL
avoids it.

But this backpatching reason still holds good as far as I can see!

Regards,
Nikhil

On Wednesday, August 12, 2026 4:05 PM Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:

I found a bug in the row filter's UPDATE to INSERT transformation. An
unchanged column that is stored out-of-line silently becomes NULL on
the subscriber, unless it is part of the replica identity.

I could also reproduce the failure with your reproducer.

ALTER TABLE t ALTER COLUMN body SET STORAGE EXTERNAL; CREATE
PUBLICATION p FOR TABLE t WHERE (id = 7); INSERT INTO t VALUES (3,
repeat('a', 5000));

Note that I could reproduce without setting the storage parameter to
EXTERNAL.
E.g., we can put string which has lower compression rate, like below.

```
CREATE EXTENSION pgcrypto;
INSERT INTO t SELECT 3, string_agg(encode(gen_random_bytes(1000), 'hex'),
'') FROM generate_series(1, 5); ```

I see three ways to deal with this.

Option B would have performance regressions not only for logical replication
but also for normal workloads. We need to generate for narrower cases, e.g.,
check the filtering rule and generate WAL for unchanged toasted columns, if
we want to choose this. I'm not sure it's possible or how engineering would be
needed though.

So, I prefer to 1) implement option A for all branches first, and 2) investigate
option B separately. Regarding the C, it can be chosen if the option A needs
lots of codes.

I feel catching such an unchanged toasted column issue during INSERT in the
apply worker could be better. The apply worker provides native context about the
transaction and the action being replayed, making the error easier to diagnose.
Erroring in the apply worker also gives users more flexibility to resolve the
issue, for example, they could define a temporary trigger on the target table to
skip the conflict or fill in the toasted value manually (after querying it from
the publisher), or skip the whole transaction using ALTER SUB SKIP, or disable
the subscription for now and analyze the issue and resolve later.

I also thought of reporting ERROR during DML, like putting this in
CheckCmdReplicaIdentity, but we can't tell at that point whether the tuple
being updated has a toasted column. And we also could not simply error out in
CheckCmdReplicaIdentity for any column that could be toasted, because lots of
columns types (text, varchar ...) is default with EXTENDED storage that can
potentially be toasted, so that would be too broad.

The only feasible place for DML error is at a lower level, like heap_update,
where we can error out if an unchanged toasted column (that is not part of the
replica identity) is being updated on a table published with a row filter.
However, this might overkill in cases where the row filter would not convert the
update to an INSERT, and I'm not sure we want to evaluate the row filter
expression during DML to do more detailed check. So I think it's not a great
idea to do it in DML.

Beyond this specific row filter issue, I'm thinking about a more general
problem: when an unchanged toasted column is not logged in the new tuple, the
subscriber cannot perform proper conflict resolution. For example, if an update
hits an update_exists conflict and the user wants to keep the remote change
(converting the UPDATE to an INSERT), without the unchanged toasted value, the
resolution lacks the data needed for the INSERT. I think we may need to support
logging additional columns beyond the replica identity in the future to address
this.

BTW, other CDC solutions also suffer from the lack of unchanged toasted
columns - I've seen several blogs mention this as a limitation [1]https://www.morling.dev/blog/backfilling-postgres-toast-columns-debezium-change-events/[2]https://clickhouse.com/docs/integrations/clickpipes/postgres/toast.

So, I think after applying the fix to report an ERROR, it would be worth
implementing an additional WAL logging feature on top of it, allowing users to specify
which columns should be logged in WAL for logical decoding and replication. I
can see several use cases for this: 1) Allowing users to add non-RI columns to
publication row filter expressions. 2) Helping with conflict resolution. 3)
Giving subscriber replication workers more column data for analysis, such as
detecting whether two transactions modify the same subscriber-only unique index
and enabling parallel apply if not. 4) Making it easier for other CDC solutions
to handle missing values.

[1]: https://www.morling.dev/blog/backfilling-postgres-toast-columns-debezium-change-events/
[2]: https://clickhouse.com/docs/integrations/clickpipes/postgres/toast

Best Regards,
Zhijie Hou

Hi Zhijie,

I feel catching such an unchanged toasted column issue during INSERT in the
apply worker could be better. The apply worker provides native context
about the
transaction and the action being replayed, making the error easier to
diagnose.
Erroring in the apply worker also gives users more flexibility to resolve
the
issue,

This is what I was arriving at myself as well. FWIW, here is a patch
that does precisely that along with test case changes.

I think this should go in now, so that a change which cannot be applied
completely stops being silently turned into a NULL on the subscriber.
For the same reason I think it should be back-patched to 15, which is
where row filters were added and so is the first affected branch.

Any option along the lines of the option B that Shinya-san outlined
changes what the publisher writes to WAL, so for master it will need a
commitfest cycle regardless. I would rather not have the silent data
loss wait for that.

Thanks,
Nikhil
---
Nikhil Sontakke
PlanetScale

On Thu, Aug 13, 2026 at 11:12 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

I feel catching such an unchanged toasted column issue during INSERT in the
apply worker could be better. The apply worker provides native context about the
transaction and the action being replayed, making the error easier to diagnose.
Erroring in the apply worker also gives users more flexibility to resolve the
issue, for example, they could define a temporary trigger on the target table to
skip the conflict or fill in the toasted value manually (after querying it from
the publisher), or skip the whole transaction using ALTER SUB SKIP, or disable
the subscription for now and analyze the issue and resolve later.

Sounds like a reasonable approach to fix the problem.

I also thought of reporting ERROR during DML, like putting this in
CheckCmdReplicaIdentity, but we can't tell at that point whether the tuple
being updated has a toasted column. And we also could not simply error out in
CheckCmdReplicaIdentity for any column that could be toasted, because lots of
columns types (text, varchar ...) is default with EXTENDED storage that can
potentially be toasted, so that would be too broad.

The only feasible place for DML error is at a lower level, like heap_update,
where we can error out if an unchanged toasted column (that is not part of the
replica identity) is being updated on a table published with a row filter.
However, this might overkill in cases where the row filter would not convert the
update to an INSERT, and I'm not sure we want to evaluate the row filter
expression during DML to do more detailed check. So I think it's not a great
idea to do it in DML.

Beyond this specific row filter issue, I'm thinking about a more general
problem: when an unchanged toasted column is not logged in the new tuple, the
subscriber cannot perform proper conflict resolution. For example, if an update
hits an update_exists conflict and the user wants to keep the remote change
(converting the UPDATE to an INSERT), without the unchanged toasted value, the
resolution lacks the data needed for the INSERT. I think we may need to support
logging additional columns beyond the replica identity in the future to address
this.

BTW, other CDC solutions also suffer from the lack of unchanged toasted
columns - I've seen several blogs mention this as a limitation [1][2].

So, I think after applying the fix to report an ERROR, it would be worth
implementing an additional WAL logging feature on top of it, allowing users to specify
which columns should be logged in WAL for logical decoding and replication. I
can see several use cases for this: 1) Allowing users to add non-RI columns to
publication row filter expressions. 2) Helping with conflict resolution. 3)
Giving subscriber replication workers more column data for analysis, such as
detecting whether two transactions modify the same subscriber-only unique index
and enabling parallel apply if not. 4) Making it easier for other CDC solutions
to handle missing values.

The only way to support these currently is to use REPLICA IDENTITY
FULL which could be costly. One idea is to have INCLUDE-like syntax
similar to what we have for CREATE INDEX to include columns for WAL
logging unchanged toast columns. However, we can do that as a
HEAD-only improvement in a separate thread.

--
With Regards,
Amit Kapila.

On Wed, 12 Aug 2026 at 05:23, Shinya Kato <shinya11.kato@gmail.com> wrote:

I see three ways to deal with this.

Option A: detect the missing value in pgoutput_row_filter() and raise
an error naming the table and the column, trading silent data loss for
a loud failure. [...]

Option B: when a table belongs to a publication with a row filter,
make heap_update() log the whole old tuple, as it already does for
REPLICA IDENTITY FULL. [...]

Option C: document the restriction and leave the behavior alone. [...]

Or, an option D: Forbid the creation (and use) of filtered publication
table definitions for tables which contain a non-identity
varlena-typed column (i.e. the type's typlen is -1).

Publishing varlena identity columns is safe, and users can just avoid
including varlena columns when they add a filter; we should not allow
users to create publications of which we know ahead of time that the
data stream is likely to break on our side.

I think this option D can be backported, but would require some
pg_upgrade checks.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

On Thu, Aug 13, 2026 at 9:50 AM Matthias van de Meent <
boekewurm+postgres@gmail.com> wrote:

Or, an option D: Forbid the creation (and use) of filtered publication
table definitions for tables which contain a non-identity varlena-typed
column (i.e. the type's typlen is -1).

I think it's too late for that: option A seems better until we get a proper
fix.

we should not allow users to create publications of which we know ahead of

time that the
data stream is likely to break on our side.

Maybe instead* we issue a warning on creation (again, until we get a real
fix, which I think is doable and probably needed for more than just this
use case)

* To be clear, I'm recommending the warning (call it option E) in addition
to option A

Cheers,
Greg

On Thu, 13 Aug 2026 at 17:19, Greg Sabino Mullane <htamfids@gmail.com> wrote:

On Thu, Aug 13, 2026 at 9:50 AM Matthias van de Meent <boekewurm+postgres@gmail.com> wrote:

Or, an option D: Forbid the creation (and use) of filtered publication table definitions for tables which contain a non-identity varlena-typed column (i.e. the type's typlen is -1).

I think it's too late for that: option A seems better until we get a proper fix.

What do you mean by "too late for that"?

If you mean "there are already systems with publications with filters
on tables with published varlena non-identity columns" then you're
right that those systems exist, but that shouldn't preclude us from
starting to raise errors when the user wants to create a new (or start
using an existing) publication that we can assume to be broken; We've
disabled and removed inherently broken features before, why shouldn't
we do that here?

I don't think there is a more proper fix than this option D.
Publications are downstream of the LR decoder (plugins can use
publications, but , and including publication information in the
decision-making process upstream of that (such as, in tableam's update
handler) we would have to break through several layers of
abstractions. And, after all of that, it'd still leave decoders which
don't use the publication tables with broken data, because those don't
necessarily track the data they export through pg_publication et al.

we should not allow users to create publications of which we know ahead of time that the
data stream is likely to break on our side.

Maybe instead* we issue a warning on creation (again, until we get a real fix, which I think is doable and probably needed for more than just this use case)

I don't see how we can get a "real" fix. Detoasting and WAL-logging
all external columns "because a logical plugin may need to see this
column if it has filtering" would explode the amount of WAL used in
updates; it'd be indistinguishable from REPLICATION IDENTITY FULL.

* To be clear, I'm recommending the warning (call it option E) in addition to option A

The warning would have to say something along the lines of "Hey, your
table has varlena columns and a filter, be aware your column data may
be lost. Oh, don't worry, we'll detect it when we lost your data and
stop the replication stream.", and if the message says something along
those lines then we'd better just plainly disallow such publications
by throwing errors, so that we don't leave a known and documented way
to lose data or halt publications when the user is using PG normally
in all possible ways. "Sorry, TOAST happened" is not a good argument
to structurally halt replication slots.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

On Thu, Aug 13, 2026 at 7:20 PM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:

On Wed, 12 Aug 2026 at 05:23, Shinya Kato <shinya11.kato@gmail.com> wrote:

I see three ways to deal with this.

Option A: detect the missing value in pgoutput_row_filter() and raise
an error naming the table and the column, trading silent data loss for
a loud failure. [...]

Option B: when a table belongs to a publication with a row filter,
make heap_update() log the whole old tuple, as it already does for
REPLICA IDENTITY FULL. [...]

Option C: document the restriction and leave the behavior alone. [...]

Or, an option D: Forbid the creation (and use) of filtered publication
table definitions for tables which contain a non-identity
varlena-typed column (i.e. the type's typlen is -1).

I think even if we want to block operations that can create such a
situation, we should reject only the specific updates that lead to the
problem, not every update on a table that merely has the potential for
it. We already do something similar: UPDATE/DELETE is rejected when
there's no replica identity and the table's publications publish those
operations. I'd like to apply the same principle here.

With that in mind, I could think of following two options:

Option 1
Check at DML time, inside heap_update(): Detect the problem per-row,
at the point where old/new tuple data is actually available when
following conditions are met: the relation is published and has
UPDATEs enabled, (b) some publication defines a row filter on it, (c)
the replica identity key changed value in this UPDATE, (d) the old
tuple has some externally-stored (TOASTed) attribute
(HeapTupleHasExternal()), and (e) some specific non-replica-identity
column's value is unchanged and still stored out-of-line.

Only an UPDATE that actually satisfies all five conditions is
rejected, with an error naming the offending column. All other UPDATEs
on the same table proceed normally, including ones that don't touch
the key, or ones where the TOASTed column did change.

Conditions (a), (b), (c) reuse state heap_update() already computed
for other purposes, so they add no real cost. Condition (d) gates
condition (e), so the per-attribute scan only runs when the old tuple
actually has a toasted value, which shouldn't be a hot code path as
such an update has other toast related overhead as well.

Option 2:
Check at statement time, inside CheckCmdReplicaIdentity(): Reject
upfront, before any row is touched, whenever: (a) the relation is
published and has UPDATEs enabled, (b) some publication defines a row
filter on it, (c) the relation has some toastable column outside the
replica identity, and (d) the relation has a TOAST table
(reltoastrelid is valid).

This is cheaper to check (no per-row work, no tuple deform) and fails
fast, but it's necessarily broader: toastability and the presence of a
toast table are static, table-wide properties, not row properties. A
table matching all four conditions would have every UPDATE rejected,
including ones that never touch the key column and ones where the
TOASTed column's current value happens to be short enough to be stored
inline.

I lean towards Option 1 (at least for master branch) for the reason
above. Thoughts?

--
With Regards,
Amit Kapila.

On Friday, August 14, 2026 8:50 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Aug 13, 2026 at 7:20 PM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:

On Wed, 12 Aug 2026 at 05:23, Shinya Kato <shinya11.kato@gmail.com>

wrote:

I see three ways to deal with this.

Option A: detect the missing value in pgoutput_row_filter() and
raise an error naming the table and the column, trading silent data
loss for a loud failure. [...]

Option B: when a table belongs to a publication with a row filter,
make heap_update() log the whole old tuple, as it already does for
REPLICA IDENTITY FULL. [...]

Option C: document the restriction and leave the behavior alone.
[...]

Or, an option D: Forbid the creation (and use) of filtered publication
table definitions for tables which contain a non-identity
varlena-typed column (i.e. the type's typlen is -1).

I think even if we want to block operations that can create such a situation, we
should reject only the specific updates that lead to the problem, not every
update on a table that merely has the potential for it. We already do
something similar: UPDATE/DELETE is rejected when there's no replica identity
and the table's publications publish those operations. I'd like to apply the same
principle here.

Yes, I was also concerned that disallowing all toastable columns would affect
too broad a range of cases. However, I agree that catching the issue earlier -
before replication happens, is better. So I also think we could try once to
catch this during DML, where we can narrow down the scope.

With that in mind, I could think of following two options:

Option 1
Check at DML time, inside heap_update(): Detect the problem per-row, at the
point where old/new tuple data is actually available when following conditions
are met: the relation is published and has UPDATEs enabled, (b) some
publication defines a row filter on it, (c) the replica identity key changed value
in this UPDATE, (d) the old tuple has some externally-stored (TOASTed)
attribute (HeapTupleHasExternal()), and (e) some specific non-replica-identity
column's value is unchanged and still stored out-of-line.
...

Option 2:
Check at statement time, inside CheckCmdReplicaIdentity(): Reject upfront,
before any row is touched, whenever: (a) the relation is published and has
UPDATEs enabled, (b) some publication defines a row filter on it, (c) the
relation has some toastable column outside the replica identity, and (d) the
relation has a TOAST table (reltoastrelid is valid).
...

I lean towards Option 1 (at least for master branch) for the reason above.
Thoughts?

For Option 1, the advantage is that it lays the groundwork for a future
improvement: automatically logging the unchanged toast values when it's possible
to convert an UPDATE to an INSERT with a publication row filter. (That could be
an optional feature, I think.). The underlying logic, finding unchanged toast
columns and detecting row filters, would be needed anyway.

For reference, I've generated both patches for comparison and evaluation:

0001: heap_update check
0002: CheckCmdReplicaIdentity check

I haven't added doc yet, but I can add it once we reach consensus.

Best Regards,
Zhijie Hou

Thank you all for the discussion. We now have options A to E and
Amit's Option 1 and 2, so let me sort the proposals by where each one
intervenes.

- Publication DDL time: forbid creating the publication (Matthias's
D), or warn (Greg's E).

- UPDATE time on the publisher: reject the UPDATE, per row in
heap_update() (Amit's Option 1, Hou's 0001), or per statement in
CheckCmdReplicaIdentity() (Amit's Option 2, Hou's 0002).

- Decode time on the publisher: error in pgoutput_row_filter() (my A).

- Apply time on the subscriber: error in apply_handle_insert() (Nikhil's patch).

- Make it work instead of erroring: WAL-log the missing values, either
always (my B) or for user-chosen columns (the INCLUDE-like idea
upthread).

- Document only (my C).

The earlier a check runs, the more it prevents and the less it knows.
The DDL and UPDATE time checks fire before anything is written to WAL,
but they have to be conservative. Even Option 1 rejects an UPDATE
whose old and new rows both match the filter, which replicates fine as
a plain UPDATE today. The decode and apply time checks are precise,
they fire exactly when a value is dropped, but by then the value is
gone. Between those two, the apply time error is recoverable with
ALTER SUBSCRIPTION SKIP while the decode time error leaves the slot
stuck, so Nikhil's check supersedes my A.

Given that, the combination I would aim for is:

- All branches (15+): Nikhil's apply time error plus a documentation
note. This is not redundant on master even after an UPDATE time check
lands there, because a master subscriber can replicate from an older
publisher that has no such check.

- master: additionally reject at UPDATE time. I agree with Amit's lean
towards Option 1. Option 2 rejects every UPDATE on a row-filtered
table that merely has a toastable column outside the replica identity,
which is close to D in impact.

- Future: the INCLUDE-like logging in a separate thread, which would
turn the remaining errors into working replication.

I will review Nikhil's patch and Hou's patches later.

Thoughts?

--
Shinya Kato
NTT OSS Center

On Sat, Aug 15, 2026 at 2:02 PM Shinya Kato <shinya11.kato@gmail.com> wrote:

Thank you all for the discussion. We now have options A to E and
Amit's Option 1 and 2, so let me sort the proposals by where each one
intervenes.

- Publication DDL time: forbid creating the publication (Matthias's
D), or warn (Greg's E).

- UPDATE time on the publisher: reject the UPDATE, per row in
heap_update() (Amit's Option 1, Hou's 0001), or per statement in
CheckCmdReplicaIdentity() (Amit's Option 2, Hou's 0002).

- Decode time on the publisher: error in pgoutput_row_filter() (my A).

- Apply time on the subscriber: error in apply_handle_insert() (Nikhil's patch).

- Make it work instead of erroring: WAL-log the missing values, either
always (my B) or for user-chosen columns (the INCLUDE-like idea
upthread).

- Document only (my C).

The earlier a check runs, the more it prevents and the less it knows.
The DDL and UPDATE time checks fire before anything is written to WAL,
but they have to be conservative. Even Option 1 rejects an UPDATE
whose old and new rows both match the filter, which replicates fine as
a plain UPDATE today.

Right, there will be some false positives due to that but I think we
can't avoid that without evaluating a row_filter which I don't think
is a good idea to do in the update code path as it can impact
performance.

The decode and apply time checks are precise,
they fire exactly when a value is dropped, but by then the value is
gone. Between those two, the apply time error is recoverable with
ALTER SUBSCRIPTION SKIP while the decode time error leaves the slot
stuck, so Nikhil's check supersedes my A.

Given that, the combination I would aim for is:

- All branches (15+): Nikhil's apply time error plus a documentation
note. This is not redundant on master even after an UPDATE time check
lands there, because a master subscriber can replicate from an older
publisher that has no such check.

Is there a reason for your preference for an apply-time patch for back
branches? I could think of following two reasons but not sure they are
worth having different fix in back-branches:
(a) a new member in exposed struct PublicationDesc; This is an ABI
break due to which ideally this shouldn't be preferred to be
backpatched? Though the risk is narrow as all six pre-existing fields
keep their old offsets exactly. Old code reading any of them still
gets the right value. It's only code that tries to read the new
rf_exists_for_update field (which by definition doesn't exist in
old-compiled code) that's affected, and only in the narrow "extension
itself declares PublicationDesc pubdesc; on the stack and calls
RelationBuildPublicationDesc()" scenario where the risk is an
out-of-bounds stack write by the new backend into memory the old-sized
struct doesn't own, not a silently-wrong read.
(b) We are adding the check in performance sensitive code path
(heap_update). However, it is guarded by multiple checks (like RI is
changed, row_filter exists, old tuple has toasted data, wal_level is
logical, etc.) which makes us traverse the attribute level loop in
non-performance critical code-path. But still we can run some
performance tests once the basic review of the patch is done.

- master: additionally reject at UPDATE time. I agree with Amit's lean
towards Option 1. Option 2 rejects every UPDATE on a row-filtered
table that merely has a toastable column outside the replica identity,
which is close to D in impact.

- Future: the INCLUDE-like logging in a separate thread, which would
turn the remaining errors into working replication.

Yeah, the INCLUDE can be discussed separately once we decide on the
main fix in this thread.

Thanks for helping in fixing this bug.

--
With Regards,
Amit Kapila.

On Fri, Aug 14, 2026 at 10:09 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:

0001: heap_update check

Few comments on 0001:
===================
1.
@@ -3453,6 +3478,34 @@ heap_update(Relation relation, const
ItemPointerData *otid, HeapTuple newtup,
id_attrs, &oldtup,
newtup, &id_has_external);

+ id_changed = bms_overlap(modified_attrs, id_attrs);
+
+ /*
+ * If the update could be transformed into an insert by a publication row
+ * filter during decoding, reject it when it would lose an unchanged
+ * out-of-line value of a column that is not part of the replica identity.
+ */
+ if (check_unchanged_external && id_changed)

Why did you place the above check in heap_update before label l2? If
the check ran before l2: (e.g. right where
modified_attrs/id_key_changed are first computed), a raised
ereport(ERROR) there could fire for an update attempt that was never
actually going to happen, the row might get updated by someone else in
the interim, EvalPlanQual retries with a different row version, and
our error would have been wrong or at least premature. Placing the
check after the TM_Ok confirmation and after the VM-pin retry (i.e.
after every goto l2 site) guarantees no more retries follow, so
raising the error here means the update really was about to proceed
against this exact tuple.

2. Can we check the required value from relation's pubdesc before
calling RelationBuildPublicationDesc()?

I haven't added doc yet, but I can add it once we reach consensus.

Feel free to add where required.

--
With Regards,
Amit Kapila.

On Sat, 15 Aug 2026 at 10:32, Shinya Kato <shinya11.kato@gmail.com> wrote:

Thank you all for the discussion. We now have options A to E and
Amit's Option 1 and 2, so let me sort the proposals by where each one
intervenes.

- Publication DDL time: forbid creating the publication (Matthias's
D), or warn (Greg's E).

- UPDATE time on the publisher: reject the UPDATE, per row in
heap_update() (Amit's Option 1, Hou's 0001), or per statement in
CheckCmdReplicaIdentity() (Amit's Option 2, Hou's 0002).

- Decode time on the publisher: error in pgoutput_row_filter() (my A).

- Apply time on the subscriber: error in apply_handle_insert() (Nikhil's patch).

- Make it work instead of erroring: WAL-log the missing values, either
always (my B) or for user-chosen columns (the INCLUDE-like idea
upthread).

- Document only (my C).

The earlier a check runs, the more it prevents and the less it knows.
The DDL and UPDATE time checks fire before anything is written to WAL,
but they have to be conservative. Even Option 1 rejects an UPDATE
whose old and new rows both match the filter, which replicates fine as
a plain UPDATE today. The decode and apply time checks are precise,
they fire exactly when a value is dropped, but by then the value is
gone. Between those two, the apply time error is recoverable with
ALTER SUBSCRIPTION SKIP while the decode time error leaves the slot
stuck, so Nikhil's check supersedes my A.

Given that, the combination I would aim for is:

- All branches (15+): Nikhil's apply time error plus a documentation
note. This is not redundant on master even after an UPDATE time check
lands there, because a master subscriber can replicate from an older
publisher that has no such check.

- master: additionally reject at UPDATE time. I agree with Amit's lean
towards Option 1. Option 2 rejects every UPDATE on a row-filtered
table that merely has a toastable column outside the replica identity,
which is close to D in impact.

- Future: the INCLUDE-like logging in a separate thread, which would
turn the remaining errors into working replication.

I will review Nikhil's patch and Hou's patches later.

Thoughts?

I don't understand the position of the LR developers here.

AFAIK, Logical Replication is (and has been) positioned as a
transparent add-on feature, that adds a new feature (replicating the
logical changes in a database) without removing functionality (such as
DML). LR's lack of support for certain functionalities (such as DDL,
or until recently sequences) didn't remove those functionalities from
the table or database that was configured for DDL, but instead the
feature was built in a way that replication couldn't be set up for
some features (sequences), or the replication stream would move into
an error state (most breaking DDL changes).

The solution that the developers seem to lean towards here is exactly
opposite to this: Enabling logical replication on a table breaks [^1]
(or would break [^2]) existing DML workloads.
To me, that looks like a clear inversion of responsibilities. I don't
think LR should push its hard problems to a user or workload that
might be unable to fix the relevant issues. If the logical
replication framework can't handle some configurations, it should
error out, the DDL handler should be adapted so that it can't be
configured in that way, or the framework should adapted to be able to
handle the configuration, but in no case should Logical Replication
push its limitations onto non-REPLICATION users that are just using
the table like any other normal table.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

[^1] tables without replication identity can recieve neither UPDATEs
nor DELETEs if those commands are included in the publication.
[^2] the general direction of this thread seems to lean towards
blocking DML on affected tables.

On Mon, Aug 17, 2026 at 5:45 PM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:

I don't understand the position of the LR developers here.

AFAIK, Logical Replication is (and has been) positioned as a
transparent add-on feature, that adds a new feature (replicating the
logical changes in a database) without removing functionality (such as
DML). LR's lack of support for certain functionalities (such as DDL,
or until recently sequences) didn't remove those functionalities from
the table or database that was configured for DDL, but instead the
feature was built in a way that replication couldn't be set up for
some features (sequences), or the replication stream would move into
an error state (most breaking DDL changes).

The solution that the developers seem to lean towards here is exactly
opposite to this: Enabling logical replication on a table breaks [^1]
(or would break [^2]) existing DML workloads.

The reason for existing behavior is that users can add/change RI after
creating publications, so we can't simply reject creating publications
when a proper RI is not defined on the table yet. Another related
example is that CREATE PUBLICATION p FOR TABLE t WHERE (non_ri_col >
5) succeeds today even if t's RI doesn't cover non_ri_col but will
give error at UPDATE/DELETE time. So, we are trying to follow the
similar pattern here.

[^1] tables without replication identity can recieve neither UPDATEs
nor DELETEs if those commands are included in the publication.
[^2] the general direction of this thread seems to lean towards
blocking DML on affected tables.

--
With Regards,
Amit Kapila.

On Monday, August 17, 2026 3:43 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Aug 14, 2026 at 10:09 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
wrote:

0001: heap_update check

Few comments on 0001:

Thanks for the comments.

===================
1.
@@ -3453,6 +3478,34 @@ heap_update(Relation relation, const
ItemPointerData *otid, HeapTuple newtup,
id_attrs, &oldtup,
newtup, &id_has_external);

+ id_changed = bms_overlap(modified_attrs, id_attrs);
+
+ /*
+ * If the update could be transformed into an insert by a publication
+ row
+ * filter during decoding, reject it when it would lose an unchanged
+ * out-of-line value of a column that is not part of the replica identity.
+ */
+ if (check_unchanged_external && id_changed)

Why did you place the above check in heap_update before label l2? If the
check ran before l2: (e.g. right where modified_attrs/id_key_changed are first
computed), a raised
ereport(ERROR) there could fire for an update attempt that was never actually
going to happen, the row might get updated by someone else in the interim,
EvalPlanQual retries with a different row version, and our error would have
been wrong or at least premature. Placing the check after the TM_Ok
confirmation and after the VM-pin retry (i.e.
after every goto l2 site) guarantees no more retries follow, so raising the error
here means the update really was about to proceed against this exact tuple.

Right, I agree we should move this after l2.

2. Can we check the required value from relation's pubdesc before calling
RelationBuildPublicationDesc()?

I think we can do this by adding a new relcache API that only accesses the new
flag. I've done that in this version.

I haven't added doc yet, but I can add it once we reach consensus.

Feel free to add where required.

Added.

Here's the updated version.

In this version, I extended pub_rf_contains_invalid_column to also check for row
filter existence, rather than adding a new function. This is fine for HEAD, but
for back branches we typically avoid changing public interfaces, so a new
function might be needed there. However, since this is an internal cache
function and I couldn't find any extensions (via GitHub or Debian code search)
that use it, changing the interface is probably acceptable.

I am sharing one version v2_PG18 that does not change existing function
interface for PG18 for reference.

BTW, I also couldn't find any extensions that depend on the size of this struct, so
I personally think backpatching should be fine.

Best Regards,
Zhijie Hou

On Monday, August 17, 2026 8:15 PM Matthias van de Meent <boekewurm+postgres@gmail.com> wrote:

I don't understand the position of the LR developers here.

AFAIK, Logical Replication is (and has been) positioned as a transparent add-
on feature, that adds a new feature (replicating the logical changes in a
database) without removing functionality (such as DML). LR's lack of support
for certain functionalities (such as DDL, or until recently sequences) didn't
remove those functionalities from the table or database that was configured
for DDL, but instead the feature was built in a way that replication couldn't be
set up for some features (sequences), or the replication stream would move
into an error state (most breaking DDL changes).

The solution that the developers seem to lean towards here is exactly
opposite to this: Enabling logical replication on a table breaks [^1] (or would
break [^2]) existing DML workloads.
To me, that looks like a clear inversion of responsibilities. I don't think LR
should push its hard problems to a user or workload that might be unable to
fix the relevant issues. If the logical replication framework can't handle some
configurations, it should error out, the DDL handler should be adapted so that
it can't be configured in that way, or the framework should adapted to be able
to handle the configuration, but in no case should Logical Replication push its
limitations onto non-REPLICATION users that are just using the table like any
other normal table.

The problem is that adding these checks in CREATE/ALTER PUBLICATION alone won't
catch all illegal cases.

Consider the existing rule: UPDATE and DELETE can only be published when a
replica identity exists. Even if we add checks in publication DDLs to error out
when replica identity is not specified and UPDATE is published, there are many
other DDLs unrelated to logical replication that could bypass the checks. The
most obvious ones are ALTER TABLE ... REPLICA IDENTITY and DROP INDEX, which can
remove the replica identity. Other DDLs like CREATE TABLE or ATTACH PARTITION
are also risky, because newly added or attached tables may be automatically
published due to FOR ALL TABLES publication settings. There could be even more
affected DDLs that we haven't noticed yet. So, catching it in DDL would also
affect the users that is not executing replication related commands.

Developers tend to add DML checks for all such restrictions - similar to the
replica identity existence check I mentioned above. Another example is the DML
check ensuring that row filter columns are also part of the replica identity. I
think one reason for this pattern is to avoid the maintenance burden of tracking
every risky DDL, both existing and future, and adding checks for each one
individually.

Best Regards,
Zhijie Hou

On Mon, Aug 17, 2026 at 2:52 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

Is there a reason for your preference for an apply-time patch for back
branches

Neither of the two you list. What I had in mind is that the
UPDATE-time check, by design, also rejects updates whose old and new
rows both match the filter, and those replicate correctly today.
Making them fail in a minor release seemed hard to justify, so I did
not think that check was a good candidate for back-patching.

But leaving the back branches as they are is worse, because the value
disappears with nothing on either side to indicate it. Some fix has to
go in, and that is why I suggested the apply-time check there, since
it fires only when a value is actually about to be dropped.

--
Shinya Kato
NTT OSS Center