Re: pgsql: Add deduplication to nbtree.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: pgsql-committers <pgsql-committers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgsql: Add deduplication to nbtree.
Date: 2020-03-01 19:29:37
Message-ID: 29676.1583090977@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Peter Geoghegan <pg(at)bowt(dot)ie> writes:
> On Sun, Mar 1, 2020 at 10:24 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Geoghegan 2020-03-01 19:42:07 Re: pgsql: Add deduplication to nbtree.
Previous Message Peter Geoghegan 2020-03-01 18:57:10 Re: pgsql: Add deduplication to nbtree.