On Fri, Jan 28, 2022 at 12:16 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > I think the best way is to do some refactoring and renaming of the > function, because as part of HeapDetermineModifiedColumns we are > already processing the tuple so we can not put extra overhead of > reprocessing it again. In short I like the idea of renaming the > HeapDetermineModifiedColumns and moving part of heap_tuple_attr_equals > code into the caller. Here is the patch set for the same. I have > divided it into two patches which can eventually be merged, 0001- for > refactoring 0002- does the actual work. >
+ /* + * If it's a whole-tuple reference, say "not equal". It's not really + * worth supporting this case, since it could only succeed after a + * no-op update, which is hardly a case worth optimizing for. + */ + if (attrnum == 0) + continue; + + /* + * Likewise, automatically say "not equal" for any system attribute + * other than tableOID; we cannot expect these to be consistent in a + * HOT chain, or even to be set correctly yet in the new tuple. + */ + if (attrnum < 0) + { + if (attrnum != TableOidAttributeNumber) + continue; + } These two cases need to be considered as the corresponding attribute is modified, so the attnum needs to be added in the bitmapset of modified attrs. I have changed this and various other comments in the patch. I have modified the docs as well to reflect the changes. I thought of adding a test but I think the current test in toast.sql seems sufficient. Kindly let me know what you think of the attached? I think we should backpatch this till v10. What do you think? Does anyone else have better ideas to fix this? -- With Regards, Amit Kapila.
v6-0001-WAL-log-unchanged-toasted-replica-identity-key-at.patch
Description: Binary data