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