From: Andres Freund <andres@anarazel.de>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers <pgsql-committers@lists.postgresql.org>
Subject: Re: pgsql: Add deduplication to nbtree.
Date: Sun, 29 Mar 2020 15:41:50 -0700
Message-ID: <20200329224150.p2kmwqzmrdsej7iw@alap3.anarazel.de> (raw)
In-Reply-To: <CAH2-Wzmpm6=smT9b75gLZ3-7ui=pBPLt46FdTa3TM=Zk3__=8Q@mail.gmail.com>
References: <27003.1583087095@sss.pgh.pa.us>
<CAH2-Wzn9_zQR9Q96JUyVBAdro52Ov6+bQ1zcE_FPxEVXGw-ebQ@mail.gmail.com>
<29676.1583090977@sss.pgh.pa.us>
<CAH2-WzmAufHiOku6AGiFD=81VQs5nYJ1L2YkhW1t+BH4CMsgRw@mail.gmail.com>
<CAH2-Wz=Tz_e8e39qTs3Mt78P6XD9s3u7TdOUVrYZg2eTEgfB0w@mail.gmail.com>
<5838.1583100843@sss.pgh.pa.us>
<CAH2-WzkPR8KMwkL0ap976kmXwBCeukTeHz6fB-U__wvuP1S9Zg@mail.gmail.com>
<CAH2-WznRH9zmUn3tthVgHp3KDNvqCEL-E2u+xTgQYeT2xtfHqw@mail.gmail.com>
<20200329221544.w2ihd7u3qdztnbr2@alap3.anarazel.de>
<CAH2-Wzmpm6=smT9b75gLZ3-7ui=pBPLt46FdTa3TM=Zk3__=8Q@mail.gmail.com>
Hi,
On 2020-03-29 15:19:50 -0700, Peter Geoghegan wrote:
> On Sun, Mar 29, 2020 at 3:15 PM Andres Freund <andres@anarazel.de> wrote:
> > Is it perhaps possible to silence the warnign with somethign along the
> > lines of
> > Assert(nhtids + vacposting->ndeletedtids == BTreeTupleGetNPosting(origtuple))
> > I don't know this code, but it looks like that'd have to be true?
> > Perhaps that'd be enough to silence coverity too?
>
> It would have to be true. It's a tautology. That is, the value of
> nhtids comes from "vacposting->ndeletedtids" and
> "BTreeTupleGetNPosting(origtuple)" anyway, and we don't mutate any of
> that state in _bt_update_posting().
Right. It doesn't look like coverity understands that currently though.
> Wouldn't it at least be necessary to Assert() something about the
> final tuple, and/or other work state?
I don't see why? The assert might not help, but if it does, I don't
think it'd need more than that check? It depends a bit on what
coverity's precise logic here is. If its ARRAY_VS_SINGLETON checker
allows index (i.e. ui) 0 but not 1, then I think the suggested assert
could help it recognize that that's unreachable.
After staring more at the coverity trace, it looks however like
ARRAY_VS_SINGLETON might not allow any array like access, not even 0?
Seems like it assumes that BTreeTupleGetNPosting(origtuple) is at least
two, and that in the first loop d < vacposting->ndeletedtids and
vacposting->deletetids[d] == i are true, but in the second iteration
assumes d < vacposting->ndeletedtids is true but
vacposting->deletetids[d] == i. Since it doesn't record a third
iteration, it seems like it ought to see ui == 0 at that point.
I'll mark it as ignored.
Greetings,
Andres Freund
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: andres@anarazel.de, pg@bowt.ie, tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Add deduplication to nbtree.
In-Reply-To: <20200329224150.p2kmwqzmrdsej7iw@alap3.anarazel.de>
* 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