From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-committers <pgsql-committers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgsql: Add deduplication to nbtree. |
Date: | 2020-03-29 22:41:50 |
Message-ID: | 20200329224150.p2kmwqzmrdsej7iw@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
Hi,
On 2020-03-29 15:19:50 -0700, Peter Geoghegan wrote:
> On Sun, Mar 29, 2020 at 3:15 PM Andres Freund <andres(at)anarazel(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2020-03-29 22:48:18 | Re: pgsql: Allow vacuum command to process indexes in parallel. |
Previous Message | Peter Geoghegan | 2020-03-29 22:19:50 | Re: pgsql: Add deduplication to nbtree. |