Add a new column to pg_am to specify whether an index AM supports backward

scanning; GiST and GIN do not, and it seems like too much trouble to make
them do so.  By teaching ExecSupportsBackwardScan() about this restriction,
we ensure that the planner will protect a scroll cursor from the problem
by adding a Materialize node.

In passing, fix another longstanding bug in the same area: backwards scan of
a plan with set-returning functions in the targetlist did not work either,
since the TupFromTlist expansion code pays no attention to direction (and
has no way to run a SRF backwards anyway).  Again the fix is to make
ExecSupportsBackwardScan check this restriction.

Also adjust the index AM API specification to note that mark/restore support
is unnecessary if the AM can't produce ordered output.

1 files changed, 68 insertions, 5 deletions

diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c
index 1de3f5a778e..9b2e32576e1 100644
--- a/src/backend/executor/execAmi.c
+++ b/src/backend/executor/execAmi.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/executor/execAmi.c,v 1.99 2008/10/04 21:56:53 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/executor/execAmi.c,v 1.100 2008/10/17 22:10:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,6 +42,12 @@
 #include "executor/nodeValuesscan.h"
 #include "executor/nodeCtescan.h"
 #include "executor/nodeWorktablescan.h"
+#include "nodes/nodeFuncs.h"
+#include "utils/syscache.h"
+
+
+static bool TargetListSupportsBackwardScan(List *targetlist);
+static bool IndexSupportsBackwardScan(Oid indexid);
 
 
 /*
@@ -390,7 +396,8 @@ ExecSupportsBackwardScan(Plan *node)
 	{
 		case T_Result:
 			if (outerPlan(node) != NULL)
-				return ExecSupportsBackwardScan(outerPlan(node));
+				return ExecSupportsBackwardScan(outerPlan(node)) &&
+					TargetListSupportsBackwardScan(node->targetlist);
 			else
 				return false;
 
@@ -403,29 +410,85 @@ ExecSupportsBackwardScan(Plan *node)
 					if (!ExecSupportsBackwardScan((Plan *) lfirst(l)))
 						return false;
 				}
+				/* need not check tlist because Append doesn't evaluate it */
 				return true;
 			}
 
 		case T_SeqScan:
-		case T_IndexScan:
 		case T_TidScan:
 		case T_FunctionScan:
 		case T_ValuesScan:
 		case T_CteScan:
 		case T_WorkTableScan:
-			return true;
+			return TargetListSupportsBackwardScan(node->targetlist);
+
+		case T_IndexScan:
+			return IndexSupportsBackwardScan(((IndexScan *) node)->indexid) &&
+				TargetListSupportsBackwardScan(node->targetlist);
 
 		case T_SubqueryScan:
-			return ExecSupportsBackwardScan(((SubqueryScan *) node)->subplan);
+			return ExecSupportsBackwardScan(((SubqueryScan *) node)->subplan) &&
+				TargetListSupportsBackwardScan(node->targetlist);
 
 		case T_Material:
 		case T_Sort:
+			/* these don't evaluate tlist */
 			return true;
 
 		case T_Limit:
+			/* doesn't evaluate tlist */
 			return ExecSupportsBackwardScan(outerPlan(node));
 
 		default:
 			return false;
 	}
 }
+
+/*
+ * If the tlist contains set-returning functions, we can't support backward
+ * scan, because the TupFromTlist code is direction-ignorant.
+ */
+static bool
+TargetListSupportsBackwardScan(List *targetlist)
+{
+	if (expression_returns_set((Node *) targetlist))
+		return false;
+	return true;
+}
+
+/*
+ * An IndexScan node supports backward scan only if the index's AM does.
+ */
+static bool
+IndexSupportsBackwardScan(Oid indexid)
+{
+	bool		result;
+	HeapTuple	ht_idxrel;
+	HeapTuple	ht_am;
+	Form_pg_class idxrelrec;
+	Form_pg_am	amrec;
+
+	/* Fetch the pg_class tuple of the index relation */
+	ht_idxrel = SearchSysCache(RELOID,
+							   ObjectIdGetDatum(indexid),
+							   0, 0, 0);
+	if (!HeapTupleIsValid(ht_idxrel))
+		elog(ERROR, "cache lookup failed for relation %u", indexid);
+	idxrelrec = (Form_pg_class) GETSTRUCT(ht_idxrel);
+
+	/* Fetch the pg_am tuple of the index' access method */
+	ht_am = SearchSysCache(AMOID,
+						   ObjectIdGetDatum(idxrelrec->relam),
+						   0, 0, 0);
+	if (!HeapTupleIsValid(ht_am))
+		elog(ERROR, "cache lookup failed for access method %u",
+			 idxrelrec->relam);
+	amrec = (Form_pg_am) GETSTRUCT(ht_am);
+
+	result = amrec->amcanbackward;
+
+	ReleaseSysCache(ht_idxrel);
+	ReleaseSysCache(ht_am);
+
+	return result;
+}