Date: Thu, 3 Oct 2019 20:36:10 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: =?utf-8?B?0JDQvdGC0L7QvSDQktC70LDRgdC+0LI=?= <druidvav@gmail.com>,
	pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16036: Segmentation fault while doing an update
Message-ID: <20191004033610.fb47ehgycodogn6w@alap3.anarazel.de>
References: <16036-28184c90d952fb7f@postgresql.org>
 <4E1FB33C-7280-487B-B3D1-D3C247AA9E3D@anarazel.de>
 <FBA5344D-9356-49BA-9B47-C3705C2E9481@gmail.com>
 <A1695349-530C-4871-AED5-0262A9BC9920@anarazel.de>
 <746398E3-F4E1-4F14-A686-3394F27011DC@gmail.com>
 <20191004032020.se46diwqas43e3tw@alap3.anarazel.de>
 <11540.1570159561@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <11540.1570159561@sss.pgh.pa.us>
Precedence: bulk

Hi,

On 2019-10-03 23:26:01 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > So I think what we need to do is:
> > 1) only call ExecCopySlot() if slot != newslot - this fixes the crash
> > 2) Add an assert to ExecCopySlot() ensuring source and target slot are
> >    distinct
> > 3) Figure out why our tests didn't catch this, this afaict should be a
> >    tested scenario
> > 4) Fix confusing variable naming around newSlot/newslot in ExecBRUpdateTriggers
> 
> Maybe instead of 1+2, change ExecCopySlot like this:
> 
> -	dstslot->tts_ops->copyslot(dstslot, srcslot);
> +	if (dstslot != srcslot)
> +	    dstslot->tts_ops->copyslot(dstslot, srcslot);
> 
> That would fix other similar instances, not just this one caller.

Yea, I was wondering about that too - but asking for a slot to be copied
into itself seems to indicate a logic bug more often than not? Expecting
the slots to be independent, afterwards. So I'm a bit hesitant doing so.

Greetings,

Andres Freund