Clean up appendonly/aocs_getnext().

Make both of them to return a bool. Now they're consistent, and a bool
seems most useful for the callers.
Reviewed-by: NAshwin Agrawal <aagrawal@pivotal.io>
Reviewed-by: NBhuvnesh Chaudhary <bchaudhary@pivotal.io>

src/backend/access/aocs/aocs_compaction.c

@@ -353,16 +353,14 @@ AOCSSegmentFileFullCompaction(Relation aorel,
 	estate->es_num_result_relations = 1;
 	estate->es_result_relation_info = resultRelInfo;
 
-	aocs_getnext(scanDesc, ForwardScanDirection, slot);
-	while (!TupIsNull(slot))
+	while (aocs_getnext(scanDesc, ForwardScanDirection, slot))
 	{
 		CHECK_FOR_INTERRUPTS();
 
 		aoTupleId = (AOTupleId *) slot_get_ctid(slot);
 		if (AppendOnlyVisimap_IsVisible(&scanDesc->visibilityMap, aoTupleId))
 		{
-			AOCSMoveTuple(
-						  slot,
+			AOCSMoveTuple(slot,
 						  insertDesc,
 						  resultRelInfo,
 						  estate);
@@ -370,11 +368,8 @@ AOCSSegmentFileFullCompaction(Relation aorel,
 		}
 		else
 		{
-			MemTuple	tuple = TupGetMemTuple(slot);
-
 			/* Tuple is invisible and needs to be dropped */
 			AppendOnlyThrowAwayTuple(aorel,
-									 tuple,
 									 slot,
 									 mt_bind);
 		}
@@ -387,9 +382,6 @@ AOCSSegmentFileFullCompaction(Relation aorel,
 		{
 			vacuum_delay_point();
 		}
-
-		aocs_getnext(scanDesc, ForwardScanDirection, slot);
-
 	}
 
 	SetAOCSFileSegInfoState(aorel, compact_segno,

src/backend/access/aocs/aocsam.c

@@ -612,7 +612,7 @@ static void upgrade_datum_fetch(AOCSFetchDesc fetch, int attno, Datum values[],
 					   values, isnull, formatversion);
 }
 
-void
+bool
 aocs_getnext(AOCSScanDesc scan, ScanDirection direction, TupleTableSlot *slot)
 {
 	int			ncol;
@@ -643,7 +643,7 @@ ReadNext:
 				/* No more seg, we are at the end */
 				ExecClearTuple(slot);
 				scan->cur_seg = -1;
-				return;
+				return false;
 			}
 			scan->cur_seg_row = 0;
 		}
@@ -720,11 +720,11 @@ ReadNext:
 
 		TupSetVirtualTupleNValid(slot, ncol);
 		slot_set_ctid(slot, &(scan->cdb_fake_ctid));
-		return;
+		return true;
 	}
 
 	Assert(!"Never here");
-	return;
+	return false;
 }
 
 

src/backend/access/appendonly/appendonly_compaction.c

@@ -267,13 +267,13 @@ AppendOnlySegmentFileTruncateToEOF(Relation aorel,
 }
 
 static void
-AppendOnlyMoveTuple(MemTuple tuple,
-					TupleTableSlot *slot,
+AppendOnlyMoveTuple(TupleTableSlot *slot,
 					MemTupleBinding *mt_bind,
 					AppendOnlyInsertDesc insertDesc,
 					ResultRelInfo *resultRelInfo,
 					EState *estate)
 {
+	MemTuple	tuple;
 	AOTupleId  *oldAoTupleId;
 	Oid			tupleOid;
 	AOTupleId	newAoTupleId;
@@ -287,6 +287,7 @@ AppendOnlyMoveTuple(MemTuple tuple,
 	/* Extract all the values of the tuple */
 	slot_getallattrs(slot);
 
+	tuple = TupGetMemTuple(slot);
 	tupleOid = MemTupleGetOid(tuple, mt_bind);
 	appendonly_insert(insertDesc,
 					  tuple,
@@ -308,10 +309,10 @@ AppendOnlyMoveTuple(MemTuple tuple,
 
 void
 AppendOnlyThrowAwayTuple(Relation rel,
-						 MemTuple tuple,
 						 TupleTableSlot *slot,
 						 MemTupleBinding *mt_bind)
 {
+	MemTuple	tuple;
 	AOTupleId  *oldAoTupleId;
 
 	Assert(slot);
@@ -321,6 +322,7 @@ AppendOnlyThrowAwayTuple(Relation rel,
 	/* Extract all the values of the tuple */
 	slot_getallattrs(slot);
 
+	tuple = TupGetMemTuple(slot);
 	if (MemTupleHasExternal(tuple, mt_bind))
 	{
 		toast_delete(rel, (GenericTuple) tuple, mt_bind);
@@ -346,7 +348,6 @@ AppendOnlySegmentFileFullCompaction(Relation aorel,
 	AppendOnlyVisimap visiMap;
 	AppendOnlyScanDesc scanDesc;
 	TupleDesc	tupDesc;
-	MemTuple	tuple;
 	TupleTableSlot *slot;
 	MemTupleBinding *mt_bind;
 	int			compact_segno;
@@ -409,7 +410,7 @@ AppendOnlySegmentFileFullCompaction(Relation aorel,
 	/*
 	 * Go through all visible tuples and move them to a new segfile.
 	 */
-	while ((tuple = appendonly_getnext(scanDesc, ForwardScanDirection, slot)) != NULL)
+	while (appendonly_getnext(scanDesc, ForwardScanDirection, slot))
 	{
 		/* Check interrupts as this may take time. */
 		CHECK_FOR_INTERRUPTS();
@@ -417,8 +418,7 @@ AppendOnlySegmentFileFullCompaction(Relation aorel,
 		aoTupleId = (AOTupleId *) slot_get_ctid(slot);
 		if (AppendOnlyVisimap_IsVisible(&scanDesc->visibilityMap, aoTupleId))
 		{
-			AppendOnlyMoveTuple(tuple,
-								slot,
+			AppendOnlyMoveTuple(slot,
 								mt_bind,
 								insertDesc,
 								resultRelInfo,
@@ -429,7 +429,6 @@ AppendOnlySegmentFileFullCompaction(Relation aorel,
 		{
 			/* Tuple is invisible and needs to be dropped */
 			AppendOnlyThrowAwayTuple(aorel,
-									 tuple,
 									 slot,
 									 mt_bind);
 		}

src/backend/access/appendonly/appendonlyam.c

@@ -1044,7 +1044,7 @@ AppendOnlyExecutorReadBlock_ProcessTuple(AppendOnlyExecutorReadBlock *executorRe
 	return valid;
 }
 
-static MemTuple
+static bool
 AppendOnlyExecutorReadBlock_ScanNextTuple(AppendOnlyExecutorReadBlock *executorReadBlock,
 										  int nkeys,
 										  ScanKey key,
@@ -1076,7 +1076,7 @@ AppendOnlyExecutorReadBlock_ScanNextTuple(AppendOnlyExecutorReadBlock *executorR
 					/* no more items in the varblock, get new buffer */
 					AppendOnlyExecutionReadBlock_FinishedScanBlock(
 																   executorReadBlock);
-					return NULL;
+					return false;
 				}
 
 				executorReadBlock->currentItemCount++;
@@ -1098,7 +1098,7 @@ AppendOnlyExecutorReadBlock_ScanNextTuple(AppendOnlyExecutorReadBlock *executorR
 																 nkeys,
 																 key,
 																 slot))
-						return TupGetMemTuple(slot);
+						return true;
 				}
 
 			}
@@ -1119,7 +1119,7 @@ AppendOnlyExecutorReadBlock_ScanNextTuple(AppendOnlyExecutorReadBlock *executorR
 				{
 					AppendOnlyExecutionReadBlock_FinishedScanBlock(
 																   executorReadBlock);
-					return NULL;
+					return false;
 					/* Force fetching new block. */
 				}
 
@@ -1144,7 +1144,7 @@ AppendOnlyExecutorReadBlock_ScanNextTuple(AppendOnlyExecutorReadBlock *executorR
 															 nkeys,
 															 key,
 															 slot))
-					return TupGetMemTuple(slot);
+					return true;
 			}
 			break;
 
@@ -1156,7 +1156,7 @@ AppendOnlyExecutorReadBlock_ScanNextTuple(AppendOnlyExecutorReadBlock *executorR
 
 	AppendOnlyExecutionReadBlock_FinishedScanBlock(
 												   executorReadBlock);
-	return NULL;
+	return false;
 	/* No match. */
 }
 
@@ -1298,15 +1298,13 @@ getNextBlock(AppendOnlyScanDesc scan)
  * the scankeys.
  * ----------------
  */
-static MemTuple
+static bool
 appendonlygettup(AppendOnlyScanDesc scan,
 				 ScanDirection dir __attribute__((unused)),
 				 int nkeys,
 				 ScanKey key,
 				 TupleTableSlot *slot)
 {
-	MemTuple	tuple;
-
 	Assert(ScanDirectionIsForward(dir));
 	Assert(scan->usableBlockSize > 0);
 
@@ -1314,6 +1312,8 @@ appendonlygettup(AppendOnlyScanDesc scan,
 
 	for (;;)
 	{
+		bool		found;
+
 		if (scan->bufferDone)
 		{
 			/*
@@ -1325,18 +1325,17 @@ appendonlygettup(AppendOnlyScanDesc scan,
 			{
 				/* have we read all this relation's data. done! */
 				if (scan->aos_done_all_segfiles)
-					return NULL;
+					return false;
 			}
 
 			scan->bufferDone = false;
 		}
 
-		tuple = AppendOnlyExecutorReadBlock_ScanNextTuple(
-														  &scan->executorReadBlock,
+		found = AppendOnlyExecutorReadBlock_ScanNextTuple(&scan->executorReadBlock,
 														  nkeys,
 														  key,
 														  slot);
-		if (tuple != NULL)
+		if (found)
 		{
 
 			/*
@@ -1353,7 +1352,7 @@ appendonlygettup(AppendOnlyScanDesc scan,
 			else
 			{
 				/* The tuple is visible */
-				return tuple;
+				return true;
 			}
 		}
 		else
@@ -1361,9 +1360,7 @@ appendonlygettup(AppendOnlyScanDesc scan,
 			/* no more items in the varblock, get new buffer */
 			scan->bufferDone = true;
 		}
-
 	}
-
 }
 
 static void
@@ -1789,22 +1786,22 @@ appendonly_endscan(AppendOnlyScanDesc scan)
  *		appendonly_getnext	- retrieve next tuple in scan
  * ----------------
  */
-MemTuple
+bool
 appendonly_getnext(AppendOnlyScanDesc scan, ScanDirection direction, TupleTableSlot *slot)
 {
-	MemTuple	tup = appendonlygettup(scan, direction, scan->aos_nkeys, scan->aos_key, slot);
+	if (appendonlygettup(scan, direction, scan->aos_nkeys, scan->aos_key, slot))
+	{
+		pgstat_count_heap_getnext(scan->aos_rd);
 
-	if (tup == NULL)
+		return true;
+	}
+	else
 	{
 		if (slot)
 			ExecClearTuple(slot);
 
-		return NULL;
+		return false;
 	}
-
-	pgstat_count_heap_getnext(scan->aos_rd);
-
-	return tup;
 }
 
 static void

src/backend/catalog/index.c

@@ -2743,7 +2743,7 @@ IndexBuildAppendOnlyRowScan(Relation parentRelation,
 		blockDirectory = aoscan->blockDirectory;
 	}
 	
-	while (appendonly_getnext(aoscan, ForwardScanDirection, slot) != NULL)
+	while (appendonly_getnext(aoscan, ForwardScanDirection, slot))
 	{
 		CHECK_FOR_INTERRUPTS();
 
@@ -2878,18 +2878,14 @@ IndexBuildAppendOnlyColScan(Relation parentRelation,
 	}
 		
 
-	while (true)
+	while (aocs_getnext(aocsscan, ForwardScanDirection, slot))
 	{
 		CHECK_FOR_INTERRUPTS();
-		
-		aocs_getnext(aocsscan, ForwardScanDirection, slot);
-		if (TupIsNull(slot))
-			break;
 
 		reltuples++;
-		
+
 		MemoryContextReset(econtext->ecxt_per_tuple_memory);
-		
+
 		if (predicate != NIL)
 		{
 			if (!ExecQual(predicate, econtext, false))
@@ -2911,11 +2907,10 @@ IndexBuildAppendOnlyColScan(Relation parentRelation,
 
 		callback(indexRelation, slot_get_ctid(slot),
 				 values, isnull, true, callback_state);
-		
 	}
 
 	pfree(proj);
-	
+
 	aocs_endscan(aocsscan);
 
 	if (blockDirectory != NULL)

src/backend/commands/analyze.c

@@ -1689,7 +1689,7 @@ acquire_sample_rows_ao(Relation onerel, int elevel,
 	for (;;)
 	{
 		if (aoScanDesc)
-			(void) appendonly_getnext(aoScanDesc, ForwardScanDirection, slot);
+			appendonly_getnext(aoScanDesc, ForwardScanDirection, slot);
 		else
 			aocs_getnext(aocsScanDesc, ForwardScanDirection, slot);
 

src/backend/commands/copy.c

@@ -2969,24 +2969,32 @@ CopyTo(CopyState cstate)
 			}
 			else if (RelationIsAoRows(rel))
 			{
-				MemTuple		tuple;
 				TupleTableSlot	*slot = MakeSingleTupleTableSlot(tupDesc);
 				MemTupleBinding *mt_bind = create_memtuple_binding(tupDesc);
 
 				aoscandesc = appendonly_beginscan(rel, GetActiveSnapshot(),
 												  GetActiveSnapshot(), 0, NULL);
 
-				while ((tuple = appendonly_getnext(aoscandesc, ForwardScanDirection, slot)) != NULL)
+				while (appendonly_getnext(aoscandesc, ForwardScanDirection, slot))
 				{
+					MemTuple	tuple;
+					Oid			tupleOid = InvalidOid;
+
 					CHECK_FOR_INTERRUPTS();
 
-					/* Extract all the values of the  tuple */
+					/* Extract all the values of the tuple */
 					slot_getallattrs(slot);
 					values = slot_get_values(slot);
 					nulls = slot_get_isnull(slot);
 
+					if (mtbind_has_oid(mt_bind))
+					{
+						tuple = TupGetMemTuple(slot);
+						tupleOid = MemTupleGetOid(tuple, mt_bind);
+					}
+
 					/* Format and send the data */
-					CopyOneRowTo(cstate, MemTupleGetOid(tuple, mt_bind), values, nulls);
+					CopyOneRowTo(cstate, tupleOid, values, nulls);
 					processed++;
 				}
 
@@ -3016,14 +3024,10 @@ CopyTo(CopyState cstate)
 									  GetActiveSnapshot(),
 									  NULL /* relationTupleDesc */, proj);
 
-				for(;;)
+				while (aocs_getnext(scan, ForwardScanDirection, slot))
 				{
 				    CHECK_FOR_INTERRUPTS();
 
-				    aocs_getnext(scan, ForwardScanDirection, slot);
-				    if (TupIsNull(slot))
-				        break;
-
 				    slot_getallattrs(slot);
 				    values = slot_get_values(slot);
 				    nulls = slot_get_isnull(slot);

src/backend/commands/tablecmds.c

@@ -6587,8 +6587,10 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
 			 */
 			oldCxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
 
-			while ((mtuple = appendonly_getnext(aoscan, ForwardScanDirection, oldslot)) != NULL)
+			while (appendonly_getnext(aoscan, ForwardScanDirection, oldslot))
 			{
+				mtuple = TupGetMemTuple(oldslot);
+
 				if (newrel)
 				{
 					Oid			tupOid = InvalidOid;
@@ -6717,9 +6719,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
 
 			sdesc = aocs_beginscan(oldrel, snapshot, snapshot, oldTupDesc, proj);
 
-			aocs_getnext(sdesc, ForwardScanDirection, oldslot);
-
-			while(!TupIsNull(oldslot))
+			while (aocs_getnext(sdesc, ForwardScanDirection, oldslot))
 			{
 				oldCxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
 				econtext->ecxt_scantuple = oldslot;
@@ -6808,7 +6808,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
 				CHECK_FOR_INTERRUPTS();
 
 				MemoryContextSwitchTo(oldCxt);
-				aocs_getnext(sdesc, ForwardScanDirection, oldslot);
 			}
 
 			aocs_endscan(sdesc);
@@ -17185,10 +17184,8 @@ split_rows(Relation intoa, Relation intob, Relation temprel)
 		}
 		else if (RelationIsAoRows(temprel))
 		{
-			MemTuple mtuple;
-
-			mtuple = appendonly_getnext(aoscan, ForwardScanDirection, slotT);
-			if (!PointerIsValid(mtuple))
+			appendonly_getnext(aoscan, ForwardScanDirection, slotT);
+			if (TupIsNull(slotT))
 				break;
 
 			TupClearShouldFree(slotT);

src/include/access/appendonly_compaction.h

@@ -33,7 +33,7 @@ extern bool AppendOnlyCompaction_ShouldCompact(
 								   int64 segmentTotalTupcount,
 								   bool isFull,
 								   Snapshot appendOnlyMetaDataSnapshot);
-extern void AppendOnlyThrowAwayTuple(Relation rel, MemTuple tuple,
+extern void AppendOnlyThrowAwayTuple(Relation rel,
 						 TupleTableSlot *slot, MemTupleBinding *mt_bind);
 extern void AppendOnlyTruncateToEOF(Relation aorel);
 extern bool HasLockForSegmentFileDrop(Relation aorel);

src/include/cdb/cdbaocsam.h

@@ -211,7 +211,7 @@ extern AOCSScanDesc aocs_beginrangescan(Relation relation, Snapshot snapshot,
 extern void aocs_rescan(AOCSScanDesc scan);
 extern void aocs_endscan(AOCSScanDesc scan);
 
-extern void aocs_getnext(AOCSScanDesc scan, ScanDirection direction, TupleTableSlot *slot);
+extern bool aocs_getnext(AOCSScanDesc scan, ScanDirection direction, TupleTableSlot *slot);
 extern AOCSInsertDesc aocs_insert_init(Relation rel, int segno, bool update_mode);
 extern Oid aocs_insert_values(AOCSInsertDesc idesc, Datum *d, bool *null, AOTupleId *aoTupleId);
 static inline Oid aocs_insert(AOCSInsertDesc idesc, TupleTableSlot *slot)

src/include/cdb/cdbappendonlyam.h

@@ -338,9 +338,9 @@ extern AppendOnlyScanDesc appendonly_beginrangescan(Relation relation,
 		int nkeys, ScanKey keys);
 extern void appendonly_rescan(AppendOnlyScanDesc scan, ScanKey key);
 extern void appendonly_endscan(AppendOnlyScanDesc scan);
-extern MemTuple appendonly_getnext(AppendOnlyScanDesc scan, 
-									ScanDirection direction,
-									TupleTableSlot *slot);
+extern bool appendonly_getnext(AppendOnlyScanDesc scan,
+							   ScanDirection direction,
+							   TupleTableSlot *slot);
 extern AppendOnlyFetchDesc appendonly_fetch_init(
 	Relation 	relation,
 	Snapshot    snapshot,