Re: Binary support for pgoutput plugin

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Dave Cramer <davecramer(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary support for pgoutput plugin
Date: 2020-07-20 15:51:19
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Peter Geoghegan <pg(at)bowt(dot)ie> writes:
> Skink's latest run reports a failure that I surmise was caused by this patch:

Yeah, I've just been digging through that. The patch didn't create
the bug, but it allowed valgrind to detect it, because the column
status array is now "just big enough" rather than being always
MaxTupleAttributeNumber entries. To wit, the problem is that the
code in apply_handle_update that computes target_rte->updatedCols
is junk.

The immediate issue is that it fails to apply the remote-to-local
column number mapping, so that it's looking at the wrong colstatus
entries, possibly including entries past the end of the array.

I'm fixing that, but even after that, there's a semantic problem:
LOGICALREP_COLUMN_UNCHANGED is just a weak optimization, cf the code
that sends it, in proto.c around line 480. colstatus will often *not*
be that for columns that were in fact not updated on the remote side.
I wonder whether we need to take steps to improve that.

CC'ing Peter E., as this issue arose with b9c130a1fdf.

regards, tom lane

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Dolgov 2020-07-20 16:02:16 Re: [HACKERS] [PATCH] Generic type subscripting
Previous Message Georgios Kokolatos 2020-07-20 15:36:44 Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions