Fix bug in hashbulkdelete.
authorRobert Haas <rhaas@postgresql.org>
Tue, 13 Dec 2016 17:16:02 +0000 (12:16 -0500)
committerRobert Haas <rhaas@postgresql.org>
Tue, 13 Dec 2016 17:16:02 +0000 (12:16 -0500)
Commit 6d46f4783efe457f74816a75173eb23ed8930020 failed to account for
the possibility that hashbulkdelete() might encounter a bucket that
has been split since it began scanning the bucket array.  Repair.

Extracted from a larger pathc by Amit Kapila; I rewrote the comment.

src/backend/access/hash/hash.c

index 6806e327ece3f17c69538bcea039bd8d171246c7..f1511d0d82fb2d2f295df59ef4f09e3f30170273 100644 (file)
@@ -523,7 +523,8 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
    orig_maxbucket = metap->hashm_maxbucket;
    orig_ntuples = metap->hashm_ntuples;
    memcpy(&local_metapage, metap, sizeof(local_metapage));
-   _hash_relbuf(rel, metabuf);
+   /* release the lock, but keep pin */
+   _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);
 
    /* Scan the buckets that we know exist */
    cur_bucket = 0;
@@ -563,8 +564,23 @@ loop_top:
         */
        if (!H_BUCKET_BEING_SPLIT(bucket_opaque) &&
            H_NEEDS_SPLIT_CLEANUP(bucket_opaque))
+       {
            split_cleanup = true;
 
+           /*
+            * This bucket might have been split since we last held a lock on
+            * the metapage.  If so, hashm_maxbucket, hashm_highmask and
+            * hashm_lowmask might be old enough to cause us to fail to remove
+            * tuples left behind by the most recent split.  To prevent that,
+            * now that the primary page of the target bucket has been locked
+            * (and thus can't be further split), update our cached metapage
+            * data.
+            */
+           _hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_READ);
+           memcpy(&local_metapage, metap, sizeof(local_metapage));
+           _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);
+       }
+
        bucket_buf = buf;
 
        hashbucketcleanup(rel, cur_bucket, bucket_buf, blkno, info->strategy,
@@ -581,7 +597,7 @@ loop_top:
    }
 
    /* Write-lock metapage and check for split since we started */
-   metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_WRITE, LH_META_PAGE);
+   _hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
    metap = HashPageGetMeta(BufferGetPage(metabuf));
 
    if (cur_maxbucket != metap->hashm_maxbucket)
@@ -589,7 +605,7 @@ loop_top:
        /* There's been a split, so process the additional bucket(s) */
        cur_maxbucket = metap->hashm_maxbucket;
        memcpy(&local_metapage, metap, sizeof(local_metapage));
-       _hash_relbuf(rel, metabuf);
+       _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);
        goto loop_top;
    }