Horton

Re: [BUG]Update Toast data failure in logical replication

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Petr Jelinek <petr.jelinek@enterprisedb.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Andres Freund <andres@anarazel.de>, Kuntal Ghosh <kuntalghosh.2007@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-01-29T10:26:53Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. WAL log unchanged toasted replica identity key attributes.

  2. Add new wal_level, logical, sufficient for logical decoding.

  3. Add the notion of REPLICA IDENTITY for a table.

Attachments

On Fri, Jan 28, 2022 at 12:16 PM Dilip Kumar <dilipbalaut@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.