You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by re...@apache.org on 2025/01/09 12:11:41 UTC

(cloudberry) 03/05: brin: table AM API for block traversal

This is an automated email from the ASF dual-hosted git repository.

reshke pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git

commit 588f5c9c6f824eb80966678e3c1d916843771763
Author: Soumyadeep Chakraborty <so...@gmail.com>
AuthorDate: Mon Feb 27 14:39:04 2023 -0800

    brin: table AM API for block traversal
    
    We have introduced a new table AM API relation_get_block_sequences()
    that helps unify code for block-based iteration for BRIN scan and
    summarization, in a table AM agnostic manner.
    
    At its heart is the critical observation that BRIN indexes deal with
    chunks of the tid space, represented in units of heap blocks. The
    resultant heap block boundaries are physical for heap tables, but are
    logical for AO/CO tables (AO/CO tables may have holes in the tid space
    due to how rownumber allocation is done). BRIN conceptually operates on
    these logical boundaries for tid selection, and we fully exploit that.
    
    A contiguous set of these logical heap blocks is what we term as
    BlockSequences.  Two BlockSequences may not be contiguous, but are
    contiguous within themselves.  The table AM API expects that we return a
    set of such BlockSequences. Heap tables have only 1 block sequence,
    whereas AO/CO tables have 1 block sequence per aoseg/aocsseg. See
    BlockSequence for more details.
    
    Once we have the list of BlockSequences, we introduce an outer loop for
    iterating over these. Within the outer loop the existing upstream loop
    is leveraged. This applies to both bringetbitmap() and brinsummarize().
    
    Thus, we end up with more unified code.
    
    The existing code was error-prone in the way it was changing the
    iteration from within the loop etc. This also fixes issue: #14843
    
    Note: Summarize tests are to be improved in a future commit.
    
    Note: The uao/limit_indexscan_inits had to be updated since we no longer
    do an extra segno=0 gp_fastsequence lookup in
    relation_get_block_sequences(), even if segno=0 is absent, as we were
    doing in brin_ao_tid_ranges() (Possibly we were doing so because
    brin_ao_tid_ranges() was copy-pasted from appendonly_fetch_init()).
    
    Co-authored-by: Ashwin Agrawal <aa...@vmware.com>
---
 src/backend/access/aocs/aocsam_handler.c           |  56 +++++
 .../access/appendonly/appendonlyam_handler.c       |  56 +++++
 src/backend/access/brin/README                     |  24 +++
 src/backend/access/brin/brin.c                     | 240 ++++++++-------------
 src/backend/access/brin/brin_revmap.c              |  15 --
 src/backend/access/heap/heapam_handler.c           |  20 ++
 src/backend/access/table/tableamapi.c              |   1 +
 src/include/access/appendonlytid.h                 |  18 +-
 src/include/access/brin_revmap.h                   |   1 -
 src/include/access/tableam.h                       |  43 ++++
 .../input/uao/limit_indexscan_inits.source         |   9 +-
 .../output/uao/limit_indexscan_inits.source        |   7 +-
 12 files changed, 316 insertions(+), 174 deletions(-)

diff --git a/src/backend/access/aocs/aocsam_handler.c b/src/backend/access/aocs/aocsam_handler.c
index 40ba70cd73..c9dec06882 100644
--- a/src/backend/access/aocs/aocsam_handler.c
+++ b/src/backend/access/aocs/aocsam_handler.c
@@ -24,6 +24,7 @@
 #include "access/tsmapi.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
+#include "catalog/gp_fastsequence.h"
 #include "catalog/heap.h"
 #include "catalog/index.h"
 #include "catalog/pg_am.h"
@@ -2044,6 +2045,60 @@ aoco_relation_size(Relation rel, ForkNumber forkNumber)
 	return totalbytes;
 }
 
+/*
+ * For each AO segment, get the starting heap block number and the number of
+ * heap blocks (together termed as a BlockSequence). The starting heap block
+ * number is always deterministic given a segment number. See AOtupleId.
+ *
+ * The number of heap blocks can be determined from the last row number present
+ * in the segment. See appendonlytid.h for details.
+ */
+static BlockSequence *
+aoco_relation_get_block_sequences(Relation rel, int *numSequences)
+{
+	Snapshot			snapshot;
+	Oid					segrelid;
+	int					nsegs;
+	BlockSequence		*sequences;
+	AOCSFileSegInfo 	**seginfos;
+
+	Assert(RelationIsValid(rel));
+	Assert(numSequences);
+
+	snapshot = RegisterSnapshot(GetCatalogSnapshot(InvalidOid));
+
+	seginfos = GetAllAOCSFileSegInfo(rel, snapshot, &nsegs, &segrelid);
+	Assert(nsegs <= AOTupleId_MaxSegmentFileNum);
+	sequences = (BlockSequence *) palloc(sizeof(BlockSequence) * nsegs);
+	*numSequences = nsegs;
+
+	/*
+	 * For each aoseg, the sequence starts at a fixed heap block number and
+	 * contains up to the highest numbered heap block corresponding to the
+	 * lastSequence value of that segment.
+	 */
+	for (int i = 0; i < nsegs; i++)
+	{
+		int segno = seginfos[i]->segno;
+		int64 lastSequence = ReadLastSequence(segrelid, segno);
+
+		sequences[i].startblknum = AOSegmentGet_startHeapBlock(segno);
+		sequences[i].nblocks = lastSequence / AO_MAX_TUPLES_PER_HEAP_BLOCK;
+		if (lastSequence % AO_MAX_TUPLES_PER_HEAP_BLOCK > 0)
+			sequences[i].nblocks += 1;
+	}
+
+	UnregisterSnapshot(snapshot);
+
+	if (seginfos != NULL)
+	{
+		FreeAllAOCSSegFileInfo(seginfos, nsegs);
+		pfree(seginfos);
+	}
+
+	return sequences;
+}
+
 static bool
 aoco_relation_needs_toast_table(Relation rel)
 {
@@ -2547,6 +2602,7 @@ static TableAmRoutine ao_column_methods = {
 	.index_validate_scan = aoco_index_validate_scan,
 
 	.relation_size = aoco_relation_size,
+	.relation_get_block_sequences = aoco_relation_get_block_sequences,
 	.relation_needs_toast_table = aoco_relation_needs_toast_table,
 
 	.relation_estimate_size = aoco_estimate_rel_size,
diff --git a/src/backend/access/appendonly/appendonlyam_handler.c b/src/backend/access/appendonly/appendonlyam_handler.c
index 39858fe393..4c1d2422f5 100644
--- a/src/backend/access/appendonly/appendonlyam_handler.c
+++ b/src/backend/access/appendonly/appendonlyam_handler.c
@@ -1876,6 +1876,61 @@ appendonly_relation_size(Relation rel, ForkNumber forkNumber)
 	return result;
 }
 
+/*
+ * For each AO segment, get the starting heap block number and the number of
+ * heap blocks (together termed as a BlockSequence). The starting heap block
+ * number is always deterministic given a segment number. See AOtupleId.
+ *
+ * The number of heap blocks can be determined from the last row number present
+ * in the segment. See appendonlytid.h for details.
+ */
+static BlockSequence *
+appendonly_relation_get_block_sequences(Relation rel,
+										int *numSequences)
+{
+	Snapshot		snapshot;
+	Oid				segrelid;
+	int				nsegs;
+	BlockSequence	*sequences;
+	FileSegInfo 	**seginfos;
+
+	Assert(RelationIsValid(rel));
+	Assert(numSequences);
+
+	snapshot = RegisterSnapshot(GetCatalogSnapshot(InvalidOid));
+
+	seginfos = GetAllFileSegInfo(rel, snapshot, &nsegs, &segrelid);
+	Assert(nsegs <= AOTupleId_MaxSegmentFileNum);
+	sequences = (BlockSequence *) palloc(sizeof(BlockSequence) * nsegs);
+	*numSequences = nsegs;
+
+	/*
+	 * For each aoseg, the sequence starts at a fixed heap block number and
+	 * contains up to the highest numbered heap block corresponding to the
+	 * lastSequence value of that segment.
+	 */
+	for (int i = 0; i < nsegs; i++)
+	{
+		int segno = seginfos[i]->segno;
+		int64 lastSequence = ReadLastSequence(segrelid, segno);
+
+		sequences[i].startblknum = AOSegmentGet_startHeapBlock(segno);
+		sequences[i].nblocks = lastSequence / AO_MAX_TUPLES_PER_HEAP_BLOCK;
+		if (lastSequence % AO_MAX_TUPLES_PER_HEAP_BLOCK > 0)
+			sequences[i].nblocks += 1;
+	}
+
+	UnregisterSnapshot(snapshot);
+
+	if (seginfos != NULL)
+	{
+		FreeAllSegFileInfo(seginfos, nsegs);
+		pfree(seginfos);
+	}
+
+	return sequences;
+}
+
 /*
  * Check to see whether the table needs a TOAST table.  It does only if
  * (1) there are any toastable attributes, and (2) the maximum length
@@ -2220,6 +2275,7 @@ static const TableAmRoutine ao_row_methods = {
 	.index_validate_scan = appendonly_index_validate_scan,
 
 	.relation_size = appendonly_relation_size,
+	.relation_get_block_sequences = appendonly_relation_get_block_sequences,
 	.relation_needs_toast_table = appendonly_relation_needs_toast_table,
 	.relation_toast_am = appendonly_relation_toast_am,
 
diff --git a/src/backend/access/brin/README b/src/backend/access/brin/README
index 656222e3db..a8e756fe95 100644
--- a/src/backend/access/brin/README
+++ b/src/backend/access/brin/README
@@ -189,6 +189,30 @@ Future improvements
   summarization on blocks that have seen lots of tuple deletions.
 
 
+GPDB:
+
+We have introduced a new table AM API relation_get_block_sequences() that helps
+unify code for block-based iteration for BRIN scan and summarization, in a
+table AM agnostic manner.
+
+At its heart is the critical observation that BRIN indexes deal with chunks of
+the tid space, represented in units of heap blocks. The resultant heap block
+boundaries are physical for heap tables, but are logical for AO/CO tables
+(AO/CO tables may have holes in the tid space due to how rownumber allocation
+is done). BRIN conceptually operates on these logical boundaries for tid
+selection, and we fully exploit that.
+
+A contiguous set of these logical heap blocks is what we term as BlockSequences.
+Two BlockSequences may not be contiguous, but are contiguous within themselves.
+The table AM API expects that we return a set of such BlockSequences. Heap
+tables have only 1 block sequence, whereas AO/CO tables have 1 block sequence
+per aoseg/aocsseg. See BlockSequence for more details.
+
+Once we have the list of BlockSequences, we introduce an outer loop for
+iterating over these. Within the outer loop the existing upstream loop is
+leveraged. This applies to both bringetbitmap() and brinsummarize().
+
+
 BRIN on append only tables
 --------------------------
 
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index abdcbdc149..40a93fddbe 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -347,72 +347,6 @@ brinbeginscan(Relation r, int nkeys, int norderbys)
 	return scan;
 }
 
-static BlockNumber
-brin_ao_tid_ranges(Relation rel, BlockNumber *aoblks)
-{
-	Snapshot	snapshot;
-	BlockNumber seg_start_blk;
-	BlockNumber nblocks = 0;
-	Oid			segrelid;
-	int64		lastSequence;
-	int			segnos[AOTupleId_MaxSegmentFileNum] = {0};
-	int			nsegs;
-
-	Assert(RelationIsValid(rel));
-
-	snapshot = RegisterSnapshot(GetCatalogSnapshot(InvalidOid));
-
-	if (RelationIsAoRows(rel))
-	{
-		FileSegInfo **seginfos = GetAllFileSegInfo(rel, snapshot, &nsegs, &segrelid);
-
-		Assert(nsegs <= AOTupleId_MaxSegmentFileNum);
-
-		for (int i = 0; i < nsegs; i++)
-			segnos[i] = seginfos[i]->segno;
-
-		if (seginfos != NULL)
-		{
-			FreeAllSegFileInfo(seginfos, nsegs);
-			pfree(seginfos);
-		}
-	}	
-	else
-	{
-		AOCSFileSegInfo **seginfos = GetAllAOCSFileSegInfo(rel, snapshot, &nsegs, &segrelid);
-
-		Assert(nsegs <= AOTupleId_MaxSegmentFileNum);
-
-		for (int i = 0; i < nsegs; i++)
-			segnos[i] = seginfos[i]->segno;
-
-		if (seginfos != NULL)
-		{
-			FreeAllAOCSSegFileInfo(seginfos, nsegs);
-			pfree(seginfos);
-		}
-	}
-
-	/* call ReadLastSequence() only for segnos corresponding to the target relation */
-	for (int i = -1, segno; i < nsegs; i++)
-	{
-		/* always initailize segment 0 */
-		segno = (i < 0 ? 0 : segnos[i]);
-		lastSequence = ReadLastSequence(segrelid, segno);
-
-		seg_start_blk = segnoGetCurrentAosegStart(segno);
-		aoblks[segno] = lastSequence / 32768;
-		if (lastSequence % 32768 > 0)
-			aoblks[segno] += 1;
-		if (lastSequence > 0)
-			nblocks = seg_start_blk + aoblks[segno];
-	}
-
-	UnregisterSnapshot(snapshot);
-
-	return nblocks;
-}
-
 /*
  * Execute the index scan.
  *
@@ -435,8 +369,6 @@ bringetbitmap(IndexScanDesc scan, Node **bmNodeP)
 	Oid			heapOid;
 	Relation	heapRel;
 	BrinOpaque *opaque;
-	BlockNumber nblocks = 0;
-	BlockNumber aoBlocks[AOTupleId_MaxSegmentFileNum];
 	BlockNumber heapBlk;
 	int			totalpages = 0;
 	FmgrInfo   *consistentFn;
@@ -453,8 +385,10 @@ bringetbitmap(IndexScanDesc scan, Node **bmNodeP)
 	char	   *ptr;
 	Size		len;
 	char	   *tmp PG_USED_FOR_ASSERTS_ONLY;
-	int			segno;
-	BlockNumber seg_start_blk;
+
+	/* GPDB: Used for iterating over the revmap */
+	int         	numSequences;
+	BlockSequence 	*sequences;
 
 	opaque = (BrinOpaque *) scan->opaque;
 	bdesc = opaque->bo_bdesc;
@@ -477,22 +411,10 @@ bringetbitmap(IndexScanDesc scan, Node **bmNodeP)
 	else
 		tbm = (TIDBitmap *)*bmNodeP;
 
-	/*
-	 * We need to know the size of the table so that we know how long to
-	 * iterate on the revmap.
-	 */
 	heapOid = IndexGetRelation(RelationGetRelid(idxRel), false);
 	heapRel = table_open(heapOid, AccessShareLock);
-
-	/*
-	 * If the data table is append only table, we need to calculate the range
-	 * of tid in each aoseg.
-	 */
-	if (RelationIsAppendOptimized(heapRel))
-		nblocks = brin_ao_tid_ranges(heapRel, aoBlocks);
-	else
-		nblocks = RelationGetNumberOfBlocks(heapRel);
-
+	sequences = table_relation_get_block_sequences(heapRel,
+												   &numSequences);
 	table_close(heapRel, AccessShareLock);
 
 	/*
@@ -624,12 +546,25 @@ bringetbitmap(IndexScanDesc scan, Node **bmNodeP)
 	oldcxt = MemoryContextSwitchTo(perRangeCxt);
 
 	/*
-	 * Now scan the revmap.  We start by querying for heap page 0,
-	 * incrementing by the number of pages per range; this gives us a full
-	 * view of the table.
+	 * GPDB: We have the notion of BlockSequences to keep the following code
+	 * section unified for AO/CO vs heap tables. Heap tables have only 1
+	 * block sequence, whereas AO/CO tables may have up to AOTupleId_MaxSegmentFileNum
+	 * number of such sequences. The outer loop is thus a GPDB addition, whereas
+	 * the inner one mostly stays the same (barring offset recalculation)
 	 */
-	segno = 0;
-	for (heapBlk = 0; heapBlk < nblocks; heapBlk += opaque->bo_pagesPerRange)
+	for (int i = 0; i < numSequences; i++)
+	{
+	/* code in the loop left unindented to prevent merge conflicts */
+
+	/*
+	 * Now scan the revmap. We start by querying for the 1st heap page in
+	 * the ith block sequence, incrementing by the number of pages per range;
+	 * this gives us a full view of each block sequence and ultimately, the
+	 * full table.
+	 */
+	BlockNumber startblknum = sequences[i].startblknum;
+	BlockNumber endblknum = sequences[i].startblknum + sequences[i].nblocks;
+	for (heapBlk = startblknum; heapBlk < endblknum; heapBlk += opaque->bo_pagesPerRange)
 	{
 		bool		addrange;
 		bool		gottuple = false;
@@ -639,23 +574,6 @@ bringetbitmap(IndexScanDesc scan, Node **bmNodeP)
 
 		CHECK_FOR_INTERRUPTS();
 
-		/*
-		 * If the largest row number of the current aoseg is scanned, switch to
-		 * the next aoseg.
-		 */
-		if (RelationIsAppendOptimized(heapRel))
-		{
-			seg_start_blk = segnoGetCurrentAosegStart(segno);
-
-			if (heapBlk >= seg_start_blk + aoBlocks[segno])
-			{
-				segno++;
-				continue;
-			}
-			if (heapBlk < seg_start_blk)
-				heapBlk = seg_start_blk;
-		}
-
 		MemoryContextResetAndDeleteChildren(perRangeCxt);
 
 		tup = brinGetTupleForHeapBlock(opaque->bo_rmAccess, heapBlk, &buf,
@@ -819,7 +737,7 @@ bringetbitmap(IndexScanDesc scan, Node **bmNodeP)
 			BlockNumber pageno;
 
 			for (pageno = heapBlk;
-				 pageno <= Min(nblocks, heapBlk + opaque->bo_pagesPerRange) - 1;
+				 pageno <= Min(endblknum, heapBlk + opaque->bo_pagesPerRange) - 1;
 				 pageno++)
 			{
 				MemoryContextSwitchTo(oldcxt);
@@ -830,8 +748,12 @@ bringetbitmap(IndexScanDesc scan, Node **bmNodeP)
 		}
 	}
 
+	/* outer loop end */
+	}
+
 	MemoryContextSwitchTo(oldcxt);
 	MemoryContextDelete(perRangeCxt);
+	pfree(sequences);
 
 	if (buf != InvalidBuffer)
 		ReleaseBuffer(buf);
@@ -1619,48 +1541,78 @@ brinsummarize(Relation index, Relation heapRel, BlockNumber pageRange,
 {
 	BrinRevmap *revmap;
 	BrinBuildState *state = NULL;
-	IndexInfo  *indexInfo = NULL;
-	BlockNumber heapNumBlocks = 0;
+	IndexInfo   *indexInfo = NULL;
 	BlockNumber pagesPerRange;
-	BlockNumber startBlk;
-	BlockNumber aoBlocks[AOTupleId_MaxSegmentFileNum] = {0};
 	Buffer		buf;
-	int			segno;
-	BlockNumber seg_start_blk;
+
+	/* GPDB: Used for iterating over the revmap */
+	int         	numSequences;
+	BlockSequence 	*sequences;
+	BlockNumber		startBlk = InvalidBlockNumber;
+	BlockNumber		endBlk = InvalidBlockNumber;
 
 	revmap = brinRevmapInitialize(index, &pagesPerRange, NULL);
 
-	/* determine range of pages to process */
+	/* determine sequence(s) of pages to process */
+	sequences = table_relation_get_block_sequences(heapRel,
+												   &numSequences);
+
+	buf = InvalidBuffer;
 
 	/*
-	 * If the data table is append only table, we need to calculate the range
-	 * of tid in each aoseg.
+	 * GPDB: We have the notion of BlockSequences to keep the following code
+	 * section unified for AO/CO vs heap tables. Heap tables have only 1
+	 * block sequence, whereas AO/CO tables may have up to AOTupleId_MaxSegmentFileNum
+	 * number of such sequences. The outer loop is thus a GPDB addition, whereas
+	 * the inner one mostly stays the same (barring offset recalculation for
+	 * both the all ranges case and specific range case)
 	 */
-	if (RelationIsAppendOptimized(heapRel))
-		heapNumBlocks = brin_ao_tid_ranges(heapRel, aoBlocks);
-	else
-		heapNumBlocks = RelationGetNumberOfBlocks(heapRel);
+
+	for (int i = 0; i < numSequences; i++)
+	{
+	/* code in the loop left unindented to prevent merge conflicts */
 
 	if (pageRange == BRIN_ALL_BLOCKRANGES)
-		startBlk = 0;
-	else
 	{
-		startBlk = (pageRange / pagesPerRange) * pagesPerRange;
-		heapNumBlocks = Min(heapNumBlocks, startBlk + pagesPerRange);
+		/* set up the start and end blocks for the next block sequence */
+		startBlk = sequences[i].startblknum;
+		endBlk = sequences[i].startblknum + sequences[i].nblocks;
 	}
-	if (startBlk > heapNumBlocks)
+	else
 	{
-		/* Nothing to do if start point is beyond end of table */
-		brinRevmapTerminate(revmap);
-		return;
+		/* we have to scan the supplied heap block in its specified range */
+
+		/*
+		 * XXX: This branch contains code that only works for heap tables, and
+		 * assumes that there is only 1 range. To support AO/CO tables, we will
+		 * need to define a table AM API:
+		 * BlockSequence * (* relation_get_block_sequence) (Relation rel,
+		 * 													BlockNumber blkNo)
+		 * with which we can find the endBlk of the specific range we have been
+		 * asked to scan.
+		 */
+
+		Assert(RelationIsHeap(heapRel));
+
+		/* should have to loop only once as there is only 1 sequence for heap */
+		Assert(numSequences == 1);
+
+		startBlk = (pageRange / pagesPerRange) * pagesPerRange;
+		endBlk = Min(sequences[i].nblocks, startBlk + pagesPerRange);
+		if (startBlk > endBlk)
+		{
+			/* Nothing to do if start point is beyond end of table */
+			brinRevmapTerminate(revmap);
+			pfree(sequences);
+			return;
+		}
 	}
 
 	/*
-	 * Scan the revmap to find unsummarized items.
+	 * Scan the revmap to find unsummarized items for each block sequence
+	 * involved.
 	 */
-	buf = InvalidBuffer;
-	segno = 0;
-	for (; startBlk < heapNumBlocks; startBlk += pagesPerRange)
+	for (; startBlk < endBlk; startBlk += pagesPerRange)
 	{
 		BrinTuple  *tup;
 		OffsetNumber off;
@@ -1673,27 +1625,11 @@ brinsummarize(Relation index, Relation heapRel, BlockNumber pageRange,
 		 * result of arbitrarily-scheduled maintenance command (vacuuming).
 		 */
 		if (!include_partial &&
-			(startBlk + pagesPerRange > heapNumBlocks))
+			(startBlk + pagesPerRange > endBlk))
 			break;
 
 		CHECK_FOR_INTERRUPTS();
 
-		/*
-		 * If the data table is append only table, we need to calculate the range
-		 * of tid in each aoseg.
-		 */
-		if (RelationIsAppendOptimized(heapRel))
-		{
-			seg_start_blk = segnoGetCurrentAosegStart(segno);
-
-			if (startBlk >= seg_start_blk + aoBlocks[segno])
-			{
-				segno++;
-				continue;
-			}
-			if (startBlk < seg_start_blk)
-				startBlk = seg_start_blk;
-		}
 		tup = brinGetTupleForHeapBlock(revmap, startBlk, &buf, &off, NULL,
 									   BUFFER_LOCK_SHARE, NULL);
 		if (tup == NULL)
@@ -1707,7 +1643,7 @@ brinsummarize(Relation index, Relation heapRel, BlockNumber pageRange,
 												   pagesPerRange);
 				indexInfo = BuildIndexInfo(index);
 			}
-			summarize_range(indexInfo, state, heapRel, startBlk, heapNumBlocks);
+			summarize_range(indexInfo, state, heapRel, startBlk, endBlk);
 
 			/* and re-initialize state for the next range */
 			brin_memtuple_initialize(state->bs_dtuple, state->bs_bdesc);
@@ -1723,6 +1659,9 @@ brinsummarize(Relation index, Relation heapRel, BlockNumber pageRange,
 		}
 	}
 
+	/* outer loop end */
+	}
+
 	if (BufferIsValid(buf))
 		ReleaseBuffer(buf);
 
@@ -1733,6 +1672,7 @@ brinsummarize(Relation index, Relation heapRel, BlockNumber pageRange,
 		terminate_brin_buildstate(state);
 		pfree(indexInfo);
 	}
+	pfree(sequences);
 }
 
 /*
diff --git a/src/backend/access/brin/brin_revmap.c b/src/backend/access/brin/brin_revmap.c
index ece5a53107..948797a44f 100644
--- a/src/backend/access/brin/brin_revmap.c
+++ b/src/backend/access/brin/brin_revmap.c
@@ -833,18 +833,3 @@ heapBlockGetCurrentAosegStart(BlockNumber heapBlk)
 {
 	return heapBlk & 0xFE000000;
 }
-
-/*
- * Get the start block number of the current aoseg by seg number.
- *
- * append-optimized table logically has 128 segment files. The highest 7 bits
- * of the logical Tid represent the segment file number. So, segment file number
- * with zero after is the start block number in a segment file.
- */
-BlockNumber
-segnoGetCurrentAosegStart(int segno)
-{
-	BlockNumber blk;
-	blk = segno;
-	return blk << 25;
-}
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 6ed3ad07b3..5894f918de 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2054,6 +2054,25 @@ heapam_scan_get_blocks_done(HeapScanDesc hscan)
  * ------------------------------------------------------------------------
  */
 
+/*
+ * GPDB: Heap tables only have 1 block sequence as they don't have segments like
+ * append-optimized tables. This sequence extends from block 0 to the number of
+ * blocks in the table.
+ */
+static BlockSequence *
+heap_relation_get_block_sequences(Relation rel,
+								  int *numSequences)
+{
+	BlockSequence *blockSequence = palloc(sizeof(BlockSequence) * 1);
+	Assert(numSequences);
+	*numSequences = 1;
+
+	blockSequence->startblknum = 0;
+	blockSequence->nblocks = RelationGetNumberOfBlocks(rel);
+
+	return blockSequence;
+}
+
 /*
  * Check to see whether the table needs a TOAST table.  It does only if
  * (1) there are any toastable attributes, and (2) the maximum length
@@ -2645,6 +2664,7 @@ static const TableAmRoutine heapam_methods = {
 	.index_validate_scan = heapam_index_validate_scan,
 
 	.relation_size = table_block_relation_size,
+	.relation_get_block_sequences = heap_relation_get_block_sequences,
 	.relation_needs_toast_table = heapam_relation_needs_toast_table,
 	.relation_toast_am = heapam_relation_toast_am,
 	.relation_fetch_toast_slice = heap_fetch_toast_slice,
diff --git a/src/backend/access/table/tableamapi.c b/src/backend/access/table/tableamapi.c
index 7a87987fd0..86e9789220 100644
--- a/src/backend/access/table/tableamapi.c
+++ b/src/backend/access/table/tableamapi.c
@@ -93,6 +93,7 @@ GetTableAmRoutine(Oid amhandler)
 	Assert(routine->index_validate_scan != NULL);
 
 	Assert(routine->relation_size != NULL);
+	Assert(routine->relation_get_block_sequences != NULL);
 	Assert(routine->relation_needs_toast_table != NULL);
 
 	Assert(routine->relation_estimate_size != NULL);
diff --git a/src/include/access/appendonlytid.h b/src/include/access/appendonlytid.h
index be95d2808e..aa1952f187 100755
--- a/src/include/access/appendonlytid.h
+++ b/src/include/access/appendonlytid.h
@@ -56,9 +56,25 @@ typedef struct AOTupleId
 	uint16		bytes_4_5;
 } AOTupleId;
 
-#define AO_MAX_OFFSET	32768
+/*
+ * This represents the maximum number of row numbers (or tuples) per logical
+ * heap block that an AO table can hold. The lower bytes: bytes_4_5 of AOTupleId
+ * gives us this value.
+ */
+#define AO_MAX_TUPLES_PER_HEAP_BLOCK	(1 << 15)
+
 #define AOTupleIdGet_segmentFileNum(h)        ((((h)->bytes_0_1&0xFE00)>>9)) // 7 bits
 
+/*
+ * Get the start block number of the specified aoseg.
+ *
+ * The higher order 32 bits of an AOTupleId (bytes_0_1 and bytes_2_3), form the
+ * heap block number when it is interpreted as an ItemPointer. Within that the
+ * top 7 bits form the segno. The starting heap block number is thus those
+ * combos of those 7 bits with the remaining 25 bits all being 0.
+ */
+#define AOSegmentGet_startHeapBlock(segno) ((segno) << 25)
+
 static inline uint64
 AOTupleIdGet_rowNum(AOTupleId *h)
 {
diff --git a/src/include/access/brin_revmap.h b/src/include/access/brin_revmap.h
index 64d005d8e7..ef230e4e5f 100644
--- a/src/include/access/brin_revmap.h
+++ b/src/include/access/brin_revmap.h
@@ -57,6 +57,5 @@ extern bool brinRevmapDesummarizeRange(Relation idxrel, BlockNumber heapBlk);
 
 extern void brin_init_upper_pages(Relation index, BlockNumber pagesPerRange);
 extern BlockNumber heapBlockGetCurrentAosegStart(BlockNumber heapBlk);
-extern BlockNumber segnoGetCurrentAosegStart(int segno);
 
 #endif							/* BRIN_REVMAP_H */
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index f5469785c0..d2d0629629 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -277,6 +277,34 @@ typedef bytea *(*tamoptions_function)(Datum reloptions,
 									  char relkind,
 									  bool validate);
 
+/*
+ * GPDB: Tables in GPDB can be thought of a set of BlockSequences, each of which
+ * is a monotonically increasing sequence of tids. The tids may or may not be
+ * contiguous. For eg. in AO/CO tables, row number allocations are batched for
+ * each insert(-like) command and the row numbers may have holes.
+ *
+ * Furthermore, each sequence of tids is represented in terms of BlockNumbers,
+ * for brevity. The BlockNumbers inside a BlockSequence are contiguous.
+ * BlockSequences do not overlap with each other and need not be contiguous
+ * between themselves.
+ *
+ * This generalization helps to unify code for heap vs AO/CO tables (eg. BRIN).
+ *
+ * Heap tables can be thought of as having only one BlockSequence having range:
+ * [0,num_blocks]. AO/CO tables will have one such sequence for every segno:
+ * [0,blocks_seg0], [33554432,33554432+blocks_seg1], [67108864,67108864+blocks_seg2], ...
+ * where blocks_segi is the number of heap blocks represented in that segment.
+ *
+ * To be clear, this does NOT mean that there must be (blocks_segi * BLCKSZ)
+ * number of bytes in that segment. Please refer to how AOTupleIds can be
+ * treated as ItemPointers.
+ */
+typedef struct BlockSequence
+{
+	BlockNumber startblknum; /* starting block number in sequence */
+	BlockNumber nblocks; /* number of heap blocks represented in sequence */
+} BlockSequence;
+
 /*
  * API struct for a table AM.  Note this must be allocated in a
  * server-lifetime manner, typically as a static const struct, which then gets
@@ -758,6 +786,11 @@ typedef struct TableAmRoutine
 	 */
 	uint64		(*relation_size) (Relation rel, ForkNumber forkNumber);
 
+	/*
+	 * See table_relation_get_block_sequences()
+	 */
+	BlockSequence		*(*relation_get_block_sequences) (Relation rel,
+														  int *numSequences);
 
 	/*
 	 * This callback should return true if the relation requires a TOAST table
@@ -2023,6 +2056,16 @@ table_relation_size(Relation rel, ForkNumber forkNumber)
 	return rel->rd_tableam->relation_size(rel, forkNumber);
 }
 
+/*
+ * GPDB: Returns the block sequences contained in this relation. See
+ * BlockSequence for details. Currently used by BRIN.
+ */
+static inline BlockSequence *
+table_relation_get_block_sequences(Relation rel, int *numSequences)
+{
+	return rel->rd_tableam->relation_get_block_sequences(rel, numSequences);
+}
+
 /*
  * table_relation_needs_toast_table - does this relation need a toast table?
  */
diff --git a/src/test/isolation2/input/uao/limit_indexscan_inits.source b/src/test/isolation2/input/uao/limit_indexscan_inits.source
index f3faedaaf7..8f148de632 100644
--- a/src/test/isolation2/input/uao/limit_indexscan_inits.source
+++ b/src/test/isolation2/input/uao/limit_indexscan_inits.source
@@ -5,10 +5,11 @@
 -- We are using gp_ao_or_aocs_seg() helper UDF to obtain the number of AO/CO segfiles (segnos),
 -- then plus 1 to count in segfile0 due to the implementation constraint.
 -- For BTREE index, we verify the equation: post_nscans - pre_nscans == segnos + 1;
--- for BRIN index, we verify the equation: post_nscans - pre_nscans == (segnos + 1) * 2.
+-- for BRIN index, we verify the equation: post_nscans - pre_nscans == ((segnos + 1) * 2 - 1).
 -- BRIN index doubles the number of nscans of BTREE because lastSequence was initialized twice
 -- during BRIN indexed bitmapheapscan (in index_getbitmap and ao/co fetch_init), which is probably
--- optimizable, too.
+-- optimizable, too. We also deduct 1 because gp_fastsequence is not scanned for
+-- seg0 to determine block sequences for AO/CO tables (if seg0 is absent).
 
 create or replace function test_iscan_inits_same_as_aosegs(tablename text, icol text, itype text) returns bool as $$
 declare
@@ -27,8 +28,8 @@ begin
         select post_nscans - pre_nscans = segnos + 1 into result; /* calculate the diff and compare to segnos plus 1 to count in segfile0, expect equal */
         raise notice '[BTREE] expect: post_nscans - pre_nscans == segnos + 1'; /* in func */
     elsif quote_ident(itype) = 'brin' then /* for BRIN index */
-        select post_nscans - pre_nscans = (segnos + 1) * 2 into result; /* BRIN doubles nscans(of BTREE) due to implementation constraint */
-        raise notice '[BRIN] expect: post_nscans - pre_nscans == (segnos + 1) * 2'; /* in func */
+        select post_nscans - pre_nscans = ((segnos + 1) * 2 - 1) into result; /* BRIN doubles nscans(of BTREE) due to implementation constraint */
+        raise notice '[BRIN] expect: post_nscans - pre_nscans == ((segnos + 1) * 2 - 1)'; /* in func */
     else /* in func */
         raise exception 'unexpected type of index %', itype::text; /* in func */
     end if; /* in func */
diff --git a/src/test/isolation2/output/uao/limit_indexscan_inits.source b/src/test/isolation2/output/uao/limit_indexscan_inits.source
index 18c52dbdc2..034c66a57c 100644
--- a/src/test/isolation2/output/uao/limit_indexscan_inits.source
+++ b/src/test/isolation2/output/uao/limit_indexscan_inits.source
@@ -5,14 +5,15 @@
 -- We are using gp_ao_or_aocs_seg() helper UDF to obtain the number of AO/CO segfiles (segnos),
 -- then plus 1 to count in segfile0 due to the implementation constraint.
 -- For BTREE index, we verify the equation: post_nscans - pre_nscans == segnos + 1;
--- for BRIN index, we verify the equation: post_nscans - pre_nscans == (segnos + 1) * 2.
+-- for BRIN index, we verify the equation: post_nscans - pre_nscans == ((segnos + 1) * 2 - 1).
 -- BRIN index doubles the number of nscans of BTREE because lastSequence was initialized twice
 -- during BRIN indexed bitmapheapscan (in index_getbitmap and ao/co fetch_init), which is probably
--- optimizable, too.
+-- optimizable, too. We also deduct 1 because gp_fastsequence is not scanned for
+-- seg0 to determine block sequences for AO/CO tables (if seg0 is absent).
 
 create or replace function test_iscan_inits_same_as_aosegs(tablename text, icol text, itype text) returns bool as $$ declare segnos smallint; /* in func */ pre_nscans smallint; /* in func */ post_nscans smallint; /* in func */ result bool; /* in func */ begin select count(segno) into segnos from gp_ao_or_aocs_seg(tablename); /* in func */ 
 select pg_stat_get_xact_numscans('gp_fastsequence_objid_objmod_index'::regclass) into pre_nscans; /* get idx_scan before query */ execute 'select * from ' || quote_ident(tablename) || ' where ' || quote_ident(icol) || ' = 2'; /* vaule 2 is distributed to the segment with content 0 */ select pg_stat_get_xact_numscans('gp_fastsequence_objid_objmod_index'::regclass) into post_nscans; /* get idx_scan after query */ 
-if quote_ident(itype) = 'btree' then /* for BTREE index */ select post_nscans - pre_nscans = segnos + 1 into result; /* calculate the diff and compare to segnos plus 1 to count in segfile0, expect equal */ raise notice '[BTREE] expect: post_nscans - pre_nscans == segnos + 1'; /* in func */ elsif quote_ident(itype) = 'brin' then /* for BRIN index */ select post_nscans - pre_nscans = (segnos + 1) * 2 into result; /* BRIN doubles nscans(of BTREE) due to implementation constraint */ raise no [...]
+if quote_ident(itype) = 'btree' then /* for BTREE index */ select post_nscans - pre_nscans = segnos + 1 into result; /* calculate the diff and compare to segnos plus 1 to count in segfile0, expect equal */ raise notice '[BTREE] expect: post_nscans - pre_nscans == segnos + 1'; /* in func */ elsif quote_ident(itype) = 'brin' then /* for BRIN index */ select post_nscans - pre_nscans = ((segnos + 1) * 2 - 1) into result; /* BRIN doubles nscans(of BTREE) due to implementation constraint */ ra [...]
 raise notice 'pre_nscans = %, post_nscans = %, segnos = %', pre_nscans, post_nscans, segnos; /* verbose */ 
 return result; /* in func */ end; /* in func */ $$ language plpgsql;
 CREATE


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org