From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
Cc: | Euler Taveira <euler(at)eulerto(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Petr Jelinek <petr(dot)jelinek(at)enterprisedb(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [BUG]Update Toast data failure in logical replication |
Date: | 2022-01-29 10:26:53 |
Message-ID: | CAA4eK1L6fs5ANytZ3x1twe+HEt0itu0ExxX0=1sWuqwMNvfMDQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Jan 28, 2022 at 12:16 PM Dilip Kumar <dilipbalaut(at)gmail(dot)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.
Attachment | Content-Type | Size |
---|---|---|
v6-0001-WAL-log-unchanged-toasted-replica-identity-key-at.patch | application/octet-stream | 13.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Banck | 2022-01-29 11:10:26 | Re: [PATCH] New default role allowing to change per-role/database settings |
Previous Message | Julien Rouhaud | 2022-01-29 07:51:33 | Re: Multiple Query IDs for a rewritten parse tree |