From 3d40d5e70ebe21b7d52467987bffad8aea16f29b Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 5 Aug 2008 21:28:29 +0000
Subject: [PATCH] Do not allow Unique nodes to be scanned backwards.  The code
 claimed that it would work, but in fact it didn't return the same rows when
 moving backwards as when moving forwards.  This would have no visible effect
 in a DISTINCT query (at least assuming the column datatypes use a strong
 definition of equality), but it gave entirely wrong answers for DISTINCT ON
 queries.

---
 src/backend/executor/execAmi.c    |  5 +----
 src/backend/executor/nodeUnique.c | 27 ++++++++++++++-------------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c
index 154301a67f..1381a4a4f0 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.96 2008/07/26 19:15:35 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/executor/execAmi.c,v 1.97 2008/08/05 21:28:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -414,9 +414,6 @@ ExecSupportsBackwardScan(Plan *node)
 		case T_Sort:
 			return true;
 
-		case T_Unique:
-			return ExecSupportsBackwardScan(outerPlan(node));
-
 		case T_Limit:
 			return ExecSupportsBackwardScan(outerPlan(node));
 
diff --git a/src/backend/executor/nodeUnique.c b/src/backend/executor/nodeUnique.c
index 545ba848d6..90eb7eeef0 100644
--- a/src/backend/executor/nodeUnique.c
+++ b/src/backend/executor/nodeUnique.c
@@ -3,19 +3,27 @@
  * nodeUnique.c
  *	  Routines to handle unique'ing of queries where appropriate
  *
+ * Unique is a very simple node type that just filters out duplicate
+ * tuples from a stream of sorted tuples from its subplan.  It's essentially
+ * a dumbed-down form of Group: the duplicate-removal functionality is
+ * identical.  However, Unique doesn't do projection nor qual checking,
+ * so it's marginally more efficient for cases where neither is needed.
+ * (It's debatable whether the savings justifies carrying two plan node
+ * types, though.)
+ *
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeUnique.c,v 1.56 2008/01/01 19:45:49 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeUnique.c,v 1.57 2008/08/05 21:28:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 /*
  * INTERFACE ROUTINES
  *		ExecUnique		- generate a unique'd temporary relation
- *		ExecInitUnique	- initialize node and subnodes..
+ *		ExecInitUnique	- initialize node and subnodes
  *		ExecEndUnique	- shutdown node and subnodes
  *
  * NOTES
@@ -32,9 +40,6 @@
 
 /* ----------------------------------------------------------------
  *		ExecUnique
- *
- *		This is a very simple node which filters out duplicate
- *		tuples from a stream of sorted tuples from a subplan.
  * ----------------------------------------------------------------
  */
 TupleTableSlot *				/* return: a tuple or NULL */
@@ -54,11 +59,7 @@ ExecUnique(UniqueState *node)
 	/*
 	 * now loop, returning only non-duplicate tuples. We assume that the
 	 * tuples arrive in sorted order so we can detect duplicates easily.
-	 *
-	 * We return the first tuple from each group of duplicates (or the last
-	 * tuple of each group, when moving backwards).  At either end of the
-	 * subplan, clear the result slot so that we correctly return the
-	 * first/last tuple when reversing direction.
+	 * The first tuple of each group is returned.
 	 */
 	for (;;)
 	{
@@ -68,13 +69,13 @@ ExecUnique(UniqueState *node)
 		slot = ExecProcNode(outerPlan);
 		if (TupIsNull(slot))
 		{
-			/* end of subplan; reset in case we change direction */
+			/* end of subplan, so we're done */
 			ExecClearTuple(resultTupleSlot);
 			return NULL;
 		}
 
 		/*
-		 * Always return the first/last tuple from the subplan.
+		 * Always return the first tuple from the subplan.
 		 */
 		if (TupIsNull(resultTupleSlot))
 			break;
@@ -113,7 +114,7 @@ ExecInitUnique(Unique *node, EState *estate, int eflags)
 	UniqueState *uniquestate;
 
 	/* check for unsupported flags */
-	Assert(!(eflags & EXEC_FLAG_MARK));
+	Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK)));
 
 	/*
 	 * create state structure
-- 
2.40.0