Fix extremely nasty little bug observed when a sub-SELECT appears in

WHERE in a place where it can be part of a nestloop inner indexqual.
As the code stood, it put the same physical sub-Plan node into both
indxqual and indxqualorig of the IndexScan plan node.  That confused
later processing in the optimizer (which expected that tracing the
subPlan list would visit each subplan node exactly once), and would
probably have blown up in the executor if the planner hadn't choked first.
Fix by making the 'fixed' indexqual be a complete deep copy of the
original indexqual, rather than trying to share nodes below the topmost
operator node.  This had further ramifications though, because we were
making the aforesaid list of sub-Plan nodes during SS_process_sublinks
which is run before construction of the 'fixed' indexqual, meaning that
the copy of the sub-Plan didn't show up in that list.  Fix by rearranging
logic so that the sub-Plan list is built by the final set_plan_references
pass, not in SS_process_sublinks.  This may sound like a mess, but it's
actually a good deal cleaner now than it was before, because we are no
longer dependent on the assumption that planning will never make a copy
of a sub-Plan node.

1 files changed, 11 insertions, 11 deletions

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 8e20ae302e1..3fab7f08b87 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.87 2000/03/22 22:08:34 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.88 2000/04/04 01:21:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -724,7 +724,9 @@ create_hashjoin_node(HashPath *best_path,
  * track of which index applies to each subgroup of index qual clauses...
  *
  * Returns a modified copy of the indexqual list --- the original is not
- * changed.
+ * changed.  Note also that the copy shares no substructure with the
+ * original; this is needed in case there is a subplan in it (we need
+ * two separate copies of the subplan tree, or things will go awry).
  */
 
 static List *
@@ -808,11 +810,13 @@ fix_indxqual_sublist(List *indexqual, int baserelid, Oid relam,
 		get_relattval((Node *) clause, baserelid,
 					  &relid, &attno, &constval, &flag);
 
-		/* Copy enough structure to allow commuting and replacing an operand
-		 * without changing original clause.
+		/* Make a copy that will become the fixed clause.
+		 *
+		 * We used to try to do a shallow copy here, but that fails if there
+		 * is a subplan in the arguments of the opclause.  So just do a
+		 * full copy.
 		 */
-		newclause = make_clause(clause->opType, clause->oper,
-								listCopy(clause->args));
+		newclause = (Expr *) copyObject((Node *) clause);
 
 		/* If the indexkey is on the right, commute the clause. */
 		if ((flag & SEL_RIGHT) == 0)
@@ -834,11 +838,7 @@ fix_indxqual_sublist(List *indexqual, int baserelid, Oid relam,
 		newopno = indexable_operator(newclause, opclass, relam, true);
 		if (newopno == InvalidOid)
 			elog(ERROR, "fix_indxqual_sublist: failed to find substitute op");
-		if (newopno != ((Oper *) newclause->oper)->opno)
-		{
-			newclause->oper = (Node *) copyObject(newclause->oper);
-			((Oper *) newclause->oper)->opno = newopno;
-		}
+		((Oper *) newclause->oper)->opno = newopno;
 
 		fixed_qual = lappend(fixed_qual, newclause);
 	}