From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | thomas(dot)munro(at)gmail(dot)com |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Strange coding in _mdfd_openseg() |
Date: | 2019-04-03 04:33:57 |
Message-ID: | 20190403.133357.75054686.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
At Wed, 3 Apr 2019 17:14:36 +1300, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote in <CA+hUKG+NBw+uSzxF1os-SO6gUuw=cqO5DAybk6KnHKzgGvxhxA(at)mail(dot)gmail(dot)com>
> Hello,
>
> I think the following conditional code is misleading, and I wonder if
> it would be better like so:
>
> --- a/src/backend/storage/smgr/md.c
> +++ b/src/backend/storage/smgr/md.c
> @@ -1787,8 +1787,13 @@ _mdfd_openseg(SMgrRelation reln, ForkNumber
> forknum, BlockNumber segno,
> if (fd < 0)
> return NULL;
>
> - if (segno <= reln->md_num_open_segs[forknum])
> - _fdvec_resize(reln, forknum, segno + 1);
> + /*
> + * Segments are always opened in order from lowest to highest,
> so we must
> + * be adding a new one at the end.
> + */
> + Assert(segno == reln->md_num_open_segs[forknum]);
> +
> + _fdvec_resize(reln, forknum, segno + 1);
>
> /* fill the entry */
> v = &reln->md_seg_fds[forknum][segno];
>
> I think the condition is always true, and with == it would also always
> be true. If that weren't the case, the call to _fdvec_resize() code
> would effectively leak vfds.
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];
| nextsegno <= targetseg; nextsegno++)
| ...
| v = _mdfd_openseg(reln, forknum, nextsegno, flags);
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2019-04-03 04:49:16 | Re: Re: A separate table level option to control compression |
Previous Message | Kyotaro HORIGUCHI | 2019-04-03 04:19:30 | Re: ToDo: show size of partitioned table |