From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
cc: pgsql-committers <pgsql-committers@lists.postgresql.org>
Subject: Re: pgsql: Add deduplication to nbtree.
In-reply-to: 
 <CAH2-Wzn9_zQR9Q96JUyVBAdro52Ov6+bQ1zcE_FPxEVXGw-ebQ@mail.gmail.com>
References: <E1j73sn-0003FQ-KO@gemulon.postgresql.org>
 <27003.1583087095@sss.pgh.pa.us>
 <CAH2-Wzn9_zQR9Q96JUyVBAdro52Ov6+bQ1zcE_FPxEVXGw-ebQ@mail.gmail.com>
Comments: In-reply-to Peter Geoghegan <pg@bowt.ie>
	message dated "Sun, 01 Mar 2020 10:57:10 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <29675.1583090977.1@sss.pgh.pa.us>
Date: Sun, 01 Mar 2020 14:29:37 -0500
Message-ID: <29676.1583090977@sss.pgh.pa.us>
Precedence: bulk

Peter Geoghegan <pg@bowt.ie> writes:
> On Sun, Mar 1, 2020 at 10:24 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I can see its point: asserting after the fact that you didn't clobber
>> memory isn't a terribly safe coding method, especially in a production
>> build where you won't even have the asserts.  Not sure if there's a
>> better way though.

> I found it slightly more elegant to treat itup->t_tid as a degenerate
> 1 element posting list here, but I'm not particularly attached to that
> approach. The loop is only truly necessary when dealing with a posting
> list tuple.
> Do you think that _bt_update_posting() should avoid this loop when
> itup is just a plain tuple, that lacks a posting list? I can do it
> that way if you prefer.

Hm.  That would probably be enough to shut up Coverity, but I'm unsure
whether it'd really be an improvement from the legibility and safety
viewpoints.  Do you want to try coding it that way and see what it
comes out like?

Note that we do have the ability to just dismiss the Coverity complaint,
if we decide that there's no better way to write the function.  But
it's usually worth looking to see if it could be written more cleanly.

			regards, tom lane