Refactor confusing code in _mdfd_openseg().
authorThomas Munro
Sun, 26 Jan 2020 20:05:27 +0000 (09:05 +1300)
committerThomas Munro
Sun, 26 Jan 2020 20:12:56 +0000 (09:12 +1300)
As reported independently by a couple of people, _mdfd_openseg() is coded in a
way that seems to imply that the segments could be opened in an order that
isn't strictly sequential.  Even if that were true, it's also using the wrong
comparison.  It's not an active bug, since the condition is always true anyway,
but it's confusing, so replace it with an assertion.

Author: Thomas Munro
Reviewed-by: Andres Freund, Kyotaro Horiguchi, Noah Misch
Discussion: https://postgr.es/m/CA%2BhUKG%2BNBw%2BuSzxF1os-SO6gUuw%3DcqO5DAybk6KnHKzgGvxhxA%40mail.gmail.com
Discussion: https://postgr.es/m/20191222091930.GA1280238%40rfd.leadboat.com

src/backend/storage/smgr/md.c

index 85b7115400621307182d058202f0cbc16bea4798..c5b771c53113f76f66d7f6d0af1898729638930a 100644 (file)
@@ -1100,8 +1100,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];