From 922460a1d56c0365770bfb08bdd8e2b291164362 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Thu, 30 Nov 2023 18:12:16 +0100
Subject: [PATCH v7 3/3] BRIN: Backfill empty ranges

If we don't, then an index with WHERE (practically_nonnull IS NULL)
would be a full table scan due to missing entries in the ranges table.

The issue is fixed for both normal and parallel index creation.
---
 src/backend/access/brin/brin.c | 69 ++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 251350fde2..9d2179e6d7 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -165,6 +165,7 @@ typedef struct BrinBuildState
 	double		bs_numtuples;
 	double		bs_reltuples;
 	Buffer		bs_currentInsertBuf;
+	BlockNumber	bs_tablePages;
 	BlockNumber bs_pagesPerRange;
 	BlockNumber bs_currRangeStart;
 	BrinRevmap *bs_rmAccess;
@@ -1134,6 +1135,7 @@ brinbuild(Relation heap, Relation index, IndexInfo *indexInfo)
 	state->bs_spool = (BrinSpool *) palloc0(sizeof(BrinSpool));
 	state->bs_spool->heap = heap;
 	state->bs_spool->index = index;
+	state->bs_tablePages = RelationGetNumberOfBlocks(heap);
 
 	/*
 	 * Attempt to launch parallel worker scan when required
@@ -1208,6 +1210,23 @@ brinbuild(Relation heap, Relation index, IndexInfo *indexInfo)
 		/* process the final batch */
 		form_and_insert_tuple(state);
 
+		state->bs_currRangeStart += state->bs_pagesPerRange;
+		/*
+		 * Backfill the final ranges with empty data.
+		 *
+		 * This saves us from doing what amounts to full table scans when the
+		 * index is built on stupid index quals like WHERE (nonnull_column IS
+		 * NULL).
+		 */
+		while (state->bs_currRangeStart + state->bs_pagesPerRange - 1 < state->bs_tablePages)
+		{
+			brin_memtuple_initialize(state->bs_dtuple, state->bs_bdesc);
+
+			form_and_insert_tuple(state);
+
+			state->bs_currRangeStart += state->bs_pagesPerRange;
+		}
+
 		/* track the number of relation tuples */
 		state->bs_reltuples = reltuples;
 	}
@@ -2625,13 +2644,34 @@ _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state)
 
 	tuplesort_end(spool->sortstate);
 
-	/* Fill empty ranges at the end, for all ranges missing in the tuplesort. */
-	prevblkno = (prevblkno == InvalidBlockNumber) ? 0 : prevblkno;
-	while (prevblkno + state->bs_pagesPerRange < memtuple->bt_blkno)
+	/* Fill the BRIN tuple for the last page range with data. */
+	if (prevblkno != InvalidBlockNumber)
 	{
-		/* the missing range */
+		BrinTuple  *tmp;
+		Size		len;
+
+		tmp = brin_form_tuple(state->bs_bdesc, memtuple->bt_blkno,
+							  memtuple, &len);
+
+		brin_doinsert(state->bs_irel, state->bs_pagesPerRange, state->bs_rmAccess,
+					  &state->bs_currentInsertBuf, tmp->bt_blkno, tmp, len);
+
+		pfree(tmp);
+	}
+
+	/*
+	 * Fill empty ranges at the end, for all ranges missing in the tuplesort.
+	 *
+	 * Starting from here, prevblkno is the to-be-inserted range's start block
+	 * number. Note that we don't fill in the relation's last page range.
+	 */
+	if (prevblkno == InvalidBlockNumber)
+		prevblkno = 0;
+	else
 		prevblkno += state->bs_pagesPerRange;
 
+	while (prevblkno + state->bs_pagesPerRange < state->bs_tablePages)
+	{
 		/* Did we already build the empty range? If not, do it now. */
 		if (emptyTuple == NULL)
 		{
@@ -2641,32 +2681,21 @@ _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state)
 		}
 		else
 		{
-			/* we already have am "empty range" tuple, just set the block */
+			/* we already have an "empty range" tuple, just set the block */
 			emptyTuple->bt_blkno = prevblkno;
 		}
 
+		/* Insert the missing range */
 		brin_doinsert(state->bs_irel, state->bs_pagesPerRange, state->bs_rmAccess,
 					  &state->bs_currentInsertBuf,
 					  emptyTuple->bt_blkno, emptyTuple, emptySize);
-	}
 
-	/* Fill the BRIN tuple for the last page range. */
-	if (prevblkno != InvalidBlockNumber)
-	{
-		BrinTuple  *tmp;
-		Size		len;
-
-		tmp = brin_form_tuple(state->bs_bdesc, memtuple->bt_blkno,
-							  memtuple, &len);
-
-		brin_doinsert(state->bs_irel, state->bs_pagesPerRange, state->bs_rmAccess,
-					  &state->bs_currentInsertBuf, tmp->bt_blkno, tmp, len);
-
-		pfree(tmp);
+		/* ... and update to the next range's block number */
+		prevblkno += state->bs_pagesPerRange;
 	}
 
 	/*
-	 * Switch back to the originam memory context, and destroy the one we
+	 * Switch back to the original memory context, and destroy the one we
 	 * created to isolate the union_tuple calls.
 	 */
 	MemoryContextSwitchTo(oldCxt);
-- 
2.40.1