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.
Date: Sun, 01 Mar 2020 14:29:37 -0500
Message-ID: <29676.1583090977@sss.pgh.pa.us> (raw)
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>
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
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-committers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pg@bowt.ie, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Add deduplication to nbtree.
In-Reply-To: <29676.1583090977@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