Fixes in bloom contrib module

feodor · feodor · commit 80afb62db0f1 · 2016-04-02T13:47:04.000+03:00
Looking at result of buildfarm member jaguarundi it seems to me that
BloomOptions isn't inited sometime, but I don't see yet how it's possible.
Nevertheless, check of signature length's is missed, so, add
a limit of it. Also add missed GenericXLogAbort() in case of already
deleted page in vacuum + minor code refactoring.
diff --git a/contrib/bloom/bloom.h b/contrib/bloom/bloom.h
@@ -63,6 +63,12 @@ typedef BloomPageOpaqueData *BloomPageOpaque;
 #define BLOOM_METAPAGE_BLKNO	(0)
 #define BLOOM_HEAD_BLKNO		(1)		/* first data page */
 
+/*
+ * Maximum of bloom signature length in uint16. Actual value
+ * is 512 bytes
+ */
+#define MAX_BLOOM_LENGTH		(256)
+
 /* Bloom index options */
 typedef struct BloomOptions
 {
diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
@@ -177,7 +177,7 @@ myRand()
 	/*
 	 * Compute x = (7^5 * x) mod (2^31 - 1)
 	 * without overflowing 31 bits:
-	 *      (2^31 - 1) = 127773 * (7^5) + 2836
+	 *		(2^31 - 1) = 127773 * (7^5) + 2836
 	 * From "Random number generators: good ones are hard to find",
 	 * Park and Miller, Communications of the ACM, vol. 31, no. 10,
 	 * October 1988, p. 1195.
@@ -370,8 +370,11 @@ adjustBloomOptions(BloomOptions *opts)
 	/* Default length of bloom filter is 5 of 16-bit integers */
 	if (opts->bloomLength <= 0)
 		opts->bloomLength = 5;
-	else
-		opts->bloomLength = opts->bloomLength;
+	else if (opts->bloomLength > MAX_BLOOM_LENGTH)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("length of bloom signature (%d) is greater than maximum %d",
+						opts->bloomLength, MAX_BLOOM_LENGTH)));
 
 	/* Check singnature length */
 	for (i = 0; i < INDEX_MAX_KEYS; i++)
@@ -382,7 +385,7 @@ adjustBloomOptions(BloomOptions *opts)
 		 * with 2 bits default.
 		 */
 		if (opts->bitSize[i] <= 0
-			|| opts->bitSize[i] >= opts->bloomLength * sizeof(SignType))
+			|| opts->bitSize[i] >= opts->bloomLength * sizeof(SignType) * BITS_PER_BYTE)
 			opts->bitSize[i] = 2;
 	}
 }
diff --git a/contrib/bloom/blvacuum.c b/contrib/bloom/blvacuum.c
@@ -70,14 +70,15 @@ blbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		if (BloomPageIsDeleted(page))
 		{
 			UnlockReleaseBuffer(buffer);
+			GenericXLogAbort(gxlogState);
 			CHECK_FOR_INTERRUPTS();
 			continue;
 		}
 
 		/* Iterate over the tuples */
-		itup = BloomPageGetTuple(&state, page, 1);
-		itupPtr = BloomPageGetTuple(&state, page, 1);
-		itupEnd = BloomPageGetTuple(&state, page, BloomPageGetMaxOffset(page) + 1);
+		itup = itupPtr = BloomPageGetTuple(&state, page, FirstOffsetNumber);
+		itupEnd = BloomPageGetTuple(&state, page,
+								OffsetNumberNext(BloomPageGetMaxOffset(page)));
 		while (itup < itupEnd)
 		{
 			/* Do we have to delete this tuple? */
@@ -104,10 +105,10 @@ blbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 			itup = BloomPageGetNextTuple(&state, itup);
 		}
 
-		Assert(itupPtr == BloomPageGetTuple(&state, page, BloomPageGetMaxOffset(page) + 1));
+		Assert(itupPtr == BloomPageGetTuple(&state, page,
+								OffsetNumberNext(BloomPageGetMaxOffset(page))));
 
-		if (!BloomPageIsDeleted(page) &&
-			BloomPageGetFreeSpace(&state, page) > state.sizeOfBloomTuple &&
+		if (BloomPageGetFreeSpace(&state, page) > state.sizeOfBloomTuple &&
 			countPage < BloomMetaBlockN)
 			notFullPage[countPage++] = blkno;
 
