On Friday, August 28, 2026 3:16 PM Shinya Kato <[email protected]> wrote: > On Thu, Aug 27, 2026 at 11:23 AM Zhijie Hou (Fujitsu) > <[email protected]> wrote: > > Thanks for the comments. They look good to me and I have addressed them > > in V4 patch. > > Thanks for working on this! I reviewed v4 and have two comments for now.
Thanks for the comments. > > 1. The extra logging does not actually depend on the key changing. > > ``` > + old_key_tuple = BuildOldKeyTuple(relation, &oldtup, newtup, > + bms_overlap(modified_attrs, id_attrs) || > + id_has_external, > + log_unchanged_external && > + HeapTupleHasExternal(&oldtup) && > + HeapTupleHasExternal(newtup), > + &old_key_copied); > ``` > > The new columns are only kept when BuildOldKeyTuple() gets past its > `if (!key_required) return NULL`, and the key_required passed above is > `bms_overlap(modified_attrs, id_attrs) || id_has_external`, so > id_has_external alone gets it there. HeapDetermineColumnsInfo() sets > id_has_external when a replica identity column of the old tuple is > stored externally, with no key change at all. So on a table whose > replica identity covers a toasted column and which has another toasted > column outside it, an UPDATE that touches neither cannot be > transformed, yet still detoasts and flattens that column into the WAL > record. Right. I updated the patch to skip logging for this case. > > 2. The test never checks that val is actually stored out-of-line. If > that ever changed, the test would keep passing without exercising the > fix, since an inline value replicates fine anyway. Asserting > pg_column_toast_chunk_id(val) IS NOT NULL on the publisher before the > UPDATE would pin that down. Added the test. Here is the updated patch which addressed all comments including Kuroda-San's[1]. [1] https://www.postgresql.org/message-id/OS9PR01MB12149A8BE71B8C9E29A9C8CC1F5AD2%40OS9PR01MB12149.jpnprd01.prod.outlook.com Best Regards, Zhijie Hou
v5-0001-Log-unchanged-out-of-line-columns-for-row-filtere.patch
Description: v5-0001-Log-unchanged-out-of-line-columns-for-row-filtere.patch
