Phase 2 of project to make index operator lossiness be determined at runtime

instead of plan time.  Extend the amgettuple API so that the index AM returns
a boolean indicating whether the indexquals need to be rechecked, and make
that rechecking happen in nodeIndexscan.c (currently the only place where
it's expected to be needed; other callers of index_getnext are just erroring
out for now).  For the moment, GIN and GIST have stub logic that just always
sets the recheck flag to TRUE --- I'm hoping to get Teodor to handle pushing
that control down to the opclass consistent() functions.  The planner no
longer pays any attention to amopreqcheck, and that catalog column will go
away in due course.

6 files changed, 63 insertions, 31 deletions

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 4fb5ee556c5..af38717340a 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *			$PostgreSQL: pgsql/src/backend/access/gin/ginget.c,v 1.11 2008/04/10 22:25:25 tgl Exp $
+ *			$PostgreSQL: pgsql/src/backend/access/gin/ginget.c,v 1.12 2008/04/13 19:18:13 tgl Exp $
  *-------------------------------------------------------------------------
  */
 
@@ -428,11 +428,14 @@ keyGetItem(Relation index, GinState *ginstate, MemoryContext tempCtx, GinScanKey
  * returns true if found
  */
 static bool
-scanGetItem(IndexScanDesc scan, ItemPointerData *item)
+scanGetItem(IndexScanDesc scan, ItemPointerData *item, bool *recheck)
 {
 	uint32		i;
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 
+	/* XXX for the moment, treat all GIN operators as lossy */
+	*recheck = true;
+
 	ItemPointerSetMin(item);
 	for (i = 0; i < so->nkeys; i++)
 	{
@@ -496,13 +499,14 @@ gingetbitmap(PG_FUNCTION_ARGS)
 	for (;;)
 	{
 		ItemPointerData iptr;
+		bool		recheck;
 
 		CHECK_FOR_INTERRUPTS();
 
-		if (!scanGetItem(scan, &iptr))
+		if (!scanGetItem(scan, &iptr, &recheck))
 			break;
 
-		tbm_add_tuples(tbm, &iptr, 1, false);
+		tbm_add_tuples(tbm, &iptr, 1, recheck);
 		ntids++;
 	}
 
@@ -528,7 +532,7 @@ gingettuple(PG_FUNCTION_ARGS)
 		PG_RETURN_BOOL(false);
 
 	startScan(scan);
-	res = scanGetItem(scan, &scan->xs_ctup.t_self);
+	res = scanGetItem(scan, &scan->xs_ctup.t_self, &scan->xs_recheck);
 	stopScan(scan);
 
 	PG_RETURN_BOOL(res);
diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
index 4533ff8c85f..8e921395825 100644
--- a/src/backend/access/gist/gistget.c
+++ b/src/backend/access/gist/gistget.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.70 2008/04/10 22:25:25 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.71 2008/04/13 19:18:14 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -23,9 +23,7 @@
 
 static OffsetNumber gistfindnext(IndexScanDesc scan, OffsetNumber n,
 			 ScanDirection dir);
-static int64 gistnext(IndexScanDesc scan, ScanDirection dir,
-					  ItemPointer tid, TIDBitmap *tbm,
-					  bool ignore_killed_tuples);
+static int64 gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm);
 static bool gistindex_keytest(IndexTuple tuple, IndexScanDesc scan,
 				  OffsetNumber offset);
 
@@ -100,7 +98,6 @@ gistgettuple(PG_FUNCTION_ARGS)
 	IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
 	ScanDirection dir = (ScanDirection) PG_GETARG_INT32(1);
 	GISTScanOpaque so;
-	ItemPointerData tid;
 	bool		res;
 
 	so = (GISTScanOpaque) scan->opaque;
@@ -113,11 +110,9 @@ gistgettuple(PG_FUNCTION_ARGS)
 		killtuple(scan->indexRelation, so, &(so->curpos));
 
 	/*
-	 * Get the next tuple that matches the search key. If asked to skip killed
-	 * tuples, continue looping until we find a non-killed tuple that matches
-	 * the search key.
+	 * Get the next tuple that matches the search key.
 	 */
-	res = (gistnext(scan, dir, &tid, NULL, scan->ignore_killed_tuples) > 0) ? true : false;
+	res = (gistnext(scan, dir, NULL) > 0);
 
 	PG_RETURN_BOOL(res);
 }
@@ -129,7 +124,7 @@ gistgetbitmap(PG_FUNCTION_ARGS)
 	TIDBitmap *tbm = (TIDBitmap *) PG_GETARG_POINTER(1);
 	int64	   ntids;
 
-	ntids = gistnext(scan, ForwardScanDirection, NULL, tbm, false);
+	ntids = gistnext(scan, ForwardScanDirection, tbm);
 
 	PG_RETURN_INT64(ntids);
 }
@@ -140,20 +135,23 @@ gistgetbitmap(PG_FUNCTION_ARGS)
  *
  * This function is used by both gistgettuple and gistgetbitmap. When
  * invoked from gistgettuple, tbm is null and the next matching tuple
- * is returned in *tid. When invoked from getbitmap, tid is null and
- * all matching tuples are added to tbm. In both cases, the function
- * result is the number of returned tuples.
+ * is returned in scan->xs_ctup.t_self.  When invoked from getbitmap,
+ * tbm is non-null and all matching tuples are added to tbm before
+ * returning.  In both cases, the function result is the number of
+ * returned tuples.
+ *
+ * If scan specifies to skip killed tuples, continue looping until we find a
+ * non-killed tuple that matches the search key.
  */
 static int64
-gistnext(IndexScanDesc scan, ScanDirection dir,
-		 ItemPointer tid, TIDBitmap *tbm,
-		 bool ignore_killed_tuples)
+gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
 {
 	Page		p;
 	OffsetNumber n;
 	GISTScanOpaque so;
 	GISTSearchStack *stk;
 	IndexTuple	it;
+	bool		recheck;
 	GISTPageOpaque opaque;
 	bool		resetoffset = false;
 	int64		ntids = 0;
@@ -194,7 +192,7 @@ gistnext(IndexScanDesc scan, ScanDirection dir,
 
 		if (XLogRecPtrIsInvalid(so->stack->lsn) || !XLByteEQ(so->stack->lsn, PageGetLSN(p)))
 		{
-			/* page changed from last visit or visit first time , reset offset */
+			/* first visit or page changed from last visit, reset offset */
 			so->stack->lsn = PageGetLSN(p);
 			resetoffset = true;
 
@@ -259,6 +257,8 @@ gistnext(IndexScanDesc scan, ScanDirection dir,
 		for (;;)
 		{
 			n = gistfindnext(scan, n, dir);
+			/* XXX for the moment, treat all GIST operators as lossy */
+			recheck = true;
 
 			if (!OffsetNumberIsValid(n))
 			{
@@ -298,15 +298,17 @@ gistnext(IndexScanDesc scan, ScanDirection dir,
 				ItemPointerSet(&(so->curpos),
 							   BufferGetBlockNumber(so->curbuf), n);
 
-				if (!(ignore_killed_tuples && ItemIdIsDead(PageGetItemId(p, n))))
+				if (!(scan->ignore_killed_tuples &&
+					  ItemIdIsDead(PageGetItemId(p, n))))
 				{
 					it = (IndexTuple) PageGetItem(p, PageGetItemId(p, n));
 					ntids++;
 					if (tbm != NULL)
-						tbm_add_tuples(tbm, &it->t_tid, 1, false);
+						tbm_add_tuples(tbm, &it->t_tid, 1, recheck);
 					else 
 					{
-						*tid = scan->xs_ctup.t_self = it->t_tid;
+						scan->xs_ctup.t_self = it->t_tid;
+						scan->xs_recheck = recheck;
 
 						LockBuffer(so->curbuf, GIST_UNLOCK);
 						return ntids; /* always 1 */
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index c090cfef8bb..997eff31058 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/hash/hash.c,v 1.101 2008/04/10 22:25:25 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/hash/hash.c,v 1.102 2008/04/13 19:18:14 tgl Exp $
  *
  * NOTES
  *	  This file contains only the public interface routines.
@@ -210,6 +210,9 @@ hashgettuple(PG_FUNCTION_ARGS)
 	OffsetNumber offnum;
 	bool		res;
 
+	/* Hash indexes are never lossy (at the moment anyway) */
+	scan->xs_recheck = false;
+
 	/*
 	 * We hold pin but not lock on current buffer while outside the hash AM.
 	 * Reacquire the read lock here.
diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
index cafb1e6867a..e8958abca2b 100644
--- a/src/backend/access/index/genam.c
+++ b/src/backend/access/index/genam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.67 2008/04/12 23:14:21 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.68 2008/04/13 19:18:14 tgl Exp $
  *
  * NOTES
  *	  many of the old access method routines have been turned into
@@ -96,9 +96,9 @@ RelationGetIndexScan(Relation indexRelation,
 	ItemPointerSetInvalid(&scan->xs_ctup.t_self);
 	scan->xs_ctup.t_data = NULL;
 	scan->xs_cbuf = InvalidBuffer;
-	scan->xs_prev_xmax = InvalidTransactionId;
-	scan->xs_next_hot = InvalidOffsetNumber;
 	scan->xs_hot_dead = false;
+	scan->xs_next_hot = InvalidOffsetNumber;
+	scan->xs_prev_xmax = InvalidTransactionId;
 
 	/*
 	 * Let the AM fill in the key and any opaque data it wants.
@@ -233,7 +233,18 @@ systable_getnext(SysScanDesc sysscan)
 	HeapTuple	htup;
 
 	if (sysscan->irel)
+	{
 		htup = index_getnext(sysscan->iscan, ForwardScanDirection);
+		/*
+		 * We currently don't need to support lossy index operators for
+		 * any system catalog scan.  It could be done here, using the
+		 * scan keys to drive the operator calls, if we arranged to save
+		 * the heap attnums during systable_beginscan(); this is practical
+		 * because we still wouldn't need to support indexes on expressions.
+		 */
+		if (htup && sysscan->iscan->xs_recheck)
+			elog(ERROR, "system catalog scans with lossy index conditions are not implemented");
+	}
 	else
 		htup = heap_getnext(sysscan->scan, ForwardScanDirection);
 
@@ -328,6 +339,9 @@ systable_getnext_ordered(SysScanDesc sysscan, ScanDirection direction)
 
 	Assert(sysscan->irel);
 	htup = index_getnext(sysscan->iscan, direction);
+	/* See notes in systable_getnext */
+	if (htup && sysscan->iscan->xs_recheck)
+		elog(ERROR, "system catalog scans with lossy index conditions are not implemented");
 
 	return htup;
 }
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 7a06d0074e8..ece3ae8ea40 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.106 2008/04/12 23:14:21 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.107 2008/04/13 19:18:14 tgl Exp $
  *
  * INTERFACE ROUTINES
  *		index_open		- open an index relation by relation OID
@@ -402,6 +402,10 @@ index_restrpos(IndexScanDesc scan)
  * snapshot, or NULL if no more matching tuples exist.	On success,
  * the buffer containing the heap tuple is pinned (the pin will be dropped
  * at the next index_getnext or index_endscan).
+ *
+ * Note: caller must check scan->xs_recheck, and perform rechecking of the
+ * scan keys if required.  We do not do that here because we don't have
+ * enough information to do it efficiently in the general case.
  * ----------------
  */
 HeapTuple
@@ -455,6 +459,8 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
 			/*
 			 * The AM's gettuple proc finds the next index entry matching the
 			 * scan keys, and puts the TID in xs_ctup.t_self (ie, *tid).
+			 * It should also set scan->xs_recheck, though we pay no
+			 * attention to that here.
 			 */
 			found = DatumGetBool(FunctionCall2(procedure,
 											   PointerGetDatum(scan),
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index d96348ace46..24f9b4c77b2 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.157 2008/04/10 22:25:25 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.158 2008/04/13 19:18:14 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -240,6 +240,9 @@ btgettuple(PG_FUNCTION_ARGS)
 	BTScanOpaque so = (BTScanOpaque) scan->opaque;
 	bool		res;
 
+	/* btree indexes are never lossy */
+	scan->xs_recheck = false;
+
 	/*
 	 * If we've already initialized this scan, we can just advance it in the
 	 * appropriate direction.  If we haven't done so yet, we call a routine to