On Friday, August 14, 2026 8:50 PM Amit Kapila <[email protected]> wrote: > > On Thu, Aug 13, 2026 at 7:20 PM Matthias van de Meent > <[email protected]> wrote: > > > > On Wed, 12 Aug 2026 at 05:23, Shinya Kato <[email protected]> > 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
v1-0001-Reject-UPDATEs-that-would-silently-lose-a-row-fil.patch
Description: v1-0001-Reject-UPDATEs-that-would-silently-lose-a-row-fil.patch
v1-0002-Reject-UPDATEs-that-would-silently-lose-a-row-fil.patch
Description: v1-0002-Reject-UPDATEs-that-would-silently-lose-a-row-fil.patch
