From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
cc: =?utf-8?Q?Ond=C5=99ej?= Jirman <ienieghapheoghaiwida@xff.cz>,
        pgsql-bugs@lists.postgresql.org, Andres Freund <andres@anarazel.de>
Subject: Re: BUG #16129: Segfault in tts_virtual_materialize in logical
 replication worker
In-reply-to: <20191122001110.zfv45fkjiwkmli6s@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>
 <13122.1574375845@sss.pgh.pa.us>
 <20191121234701.2wtyc77zd5ux3myf@development>
 <14166.1574380607@sss.pgh.pa.us>
 <20191122001110.zfv45fkjiwkmli6s@development>
Comments: In-reply-to Tomas Vondra <tomas.vondra@2ndquadrant.com>
	message dated "Fri, 22 Nov 2019 01:11:10 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <15666.1574382815.1@sss.pgh.pa.us>
Date: Thu, 21 Nov 2019 19:33:35 -0500
Message-ID: <15667.1574382815@sss.pgh.pa.us>
Precedence: bulk

Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> On Thu, Nov 21, 2019 at 06:56:47PM -0500, Tom Lane wrote:
>> "remoteslot" will contain its very own copy of the data, which
>> is then summarily freed by ExecClearSlot.

> But remoteslot is virtual, so it calls tts_virtual_copyslot, not the
> heap one. And AFAIK tts_virtual_copyslot only copies contents of the
> tts_values/tts_isnull arrays.

Really?

static void
tts_virtual_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
{
	...
	/* make sure storage doesn't depend on external memory */
	tts_virtual_materialize(dstslot);
}

In any case, I sure hope that Andres hasn't made the different
versions of ExecCopySlot have different semantics --- if he has,
he's got some explaining to do.  But at least on this point,
it looks to me like all three versions still satisfy the
semantics that were clearly defined previously (v11 and before):

/* --------------------------------
 *		ExecCopySlot
 *			Copy the source slot's contents into the destination slot.
 *
 *		The destination acquires a private copy that will not go away
 *		if the source is cleared.
 *
 *		The caller must ensure the slots have compatible tupdescs.
 * --------------------------------
 */

I'm sad that we seem to have lost this specification comment, though.

			regards, tom lane