From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Ondřej Jirman <ienieghapheoghaiwida@xff.cz>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16129: Segfault in tts_virtual_materialize in logical replication worker
Date: Fri, 22 Nov 2019 11:50:03 -0500
Message-ID: <26015.1574441403@sss.pgh.pa.us> (raw)
In-Reply-To: <20191122161248.qnzeoh6t7fvg5wl3@development>
References: <20191121155707.pqkgjphh65o5fapz@core.my.home>
<20191121161502.xfiy65i3cm6adscr@core.my.home>
<20191121165752.dffge6bh756xlfdg@development>
<20191121173555.midj6vp4dmiiqs24@core.my.home>
<20191121185844.6qsqhwzzuhu3fpsg@core.my.home>
<20191121214115.3tgefmvkmrzaim74@development>
<20191121220854.ebdzjtloel56sfwu@development>
<20191122003626.ydgwvjfx363blgip@core.my.home>
<16189.1574383534@sss.pgh.pa.us>
<20191122005401.bkdpmtqwuu6mvnyg@core.my.home>
<20191122161248.qnzeoh6t7fvg5wl3@development>
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> On Fri, Nov 22, 2019 at 01:54:01AM +0100, Ondřej Jirman wrote:
>> On Thu, Nov 21, 2019 at 07:45:34PM -0500, Tom Lane wrote:
>>> =?utf-8?Q?Ond=C5=99ej?= Jirman <ienieghapheoghaiwida@xff.cz> writes:
>>>> newtup.changed
>>>> {true, true, false, true, true, true, true, true, false <repeats 1656 times>}
>>> So column 3 is not getting replaced.
> Can you show us the attribute list as defined in the system, including
> e.g. dropped columns? That is, something like
> SELECT attnum, attname, atttypid FROM pg_attribute
> WHERE attrelid = 'public.videos'::regclass;
> both from the published and subscriber.
After digging around a bit more in the logrep code, it seems like we'd
only have a situation with newtup.changed[i] = false if the publisher
had hit this bit in logicalrep_write_tuple:
else if (att->attlen == -1 && VARATT_IS_EXTERNAL_ONDISK(values[i]))
{
pq_sendbyte(out, 'u'); /* unchanged toast column */
continue;
}
So this seems to indicate that Ondřej's case involves an update on a row
that contained a toasted-out-of-line column that did not get updated.
That's different from the trigger conditions that I postulated, but the
end result is the same: slot_modify_cstrings would have a case where
it's supposed to retain the old data for a pass-by-ref column, and it'd
fail to do that correctly.
I also discovered that before v12, the calling code looked like this:
ExecStoreTuple(localslot->tts_tuple, remoteslot, InvalidBuffer, false);
slot_modify_cstrings(remoteslot, rel, newtup.values, newtup.changed);
so that at that time, "remoteslot" indeed did not have its own local
copy of the tuple, and thus the code failed to fail. Perhaps this was
intentional on the part of whoever wrote this, but it's still an
unacceptable abuse of the API if you ask me.
I added some tests involving dropped columns to what I had last night
and pushed it. However, looking at this now, I see that we really
still don't have enough coverage --- the code path I showed above
is not hit by the regression tests, according to the code coverage
bot. I don't think it's really acceptable for such a corner case
bit of the logrep protocol to not get tested :-(
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, tomas.vondra@2ndquadrant.com, ienieghapheoghaiwida@xff.cz, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16129: Segfault in tts_virtual_materialize in logical replication worker
In-Reply-To: <26015.1574441403@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox