From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Strange coding in _mdfd_openseg() |
Date: | 2019-04-03 20:47:46 |
Message-ID: | 20190403204746.2yumq7c2mirmodsg@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2019-04-04 09:24:49 +1300, Thomas Munro wrote:
> On Wed, Apr 3, 2019 at 5:34 PM Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > I may be missing something, but it seems possible that
> > _mdfd_getseg calls it with segno > opensegs.
> >
> > | for (nextsegno = reln->md_num_open_segs[forknum];
>
> Here nextsegno starts out equal to opensegs.
>
> > | nextsegno <= targetseg; nextsegno++)
>
> Here we add one to nextsegno...
>
> > | ...
> > | v = _mdfd_openseg(reln, forknum, nextsegno, flags);
>
> ... after adding one to opensegs. So they're always equal. This fits
> a general programming pattern when appending to an array, the next
> element's index is the same as the number of elements. But I claim
> the coding is weird, because _mdfd_openseg's *looks* like it can
> handle opening segments in any order, except that the author
> accidentally wrote "<=" instead of ">=". In fact it can't open them
> in any order, because we don't support "holes" in the array. So I
> think it should really be "==", and it should be an assertion, not a
> condition.
Yea, I totally agree it's weird. I'm not sure if I'd go for an assertion
of equality, or just invert the >= (which I agree I probably just
screwed up and didn't notice when reviewing the patch because it looked
close enough to correct and it didn't have a measurable effect).
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Sergei Kornilov | 2019-04-03 20:49:54 | Re: allow online change primary_conninfo |
Previous Message | Jeff Janes | 2019-04-03 20:42:14 | Re: pg_upgrade: Pass -j down to vacuumdb |