Executor no longer cares about mergejoinop, mergerightorder, mergeleftorder,

so remove them from MergeJoin node.  Hack together a partial
solution for commuted mergejoin operators --- yesterday
a mergejoin int4 = int8 would crash if the planner decided to
commute it, today it works.  The planner's representation of
mergejoins really needs a rewrite though.
Also, further testing of mergejoin ops in opr_sanity regress test.

src/backend/nodes/copyfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.74 1999/02/22 19:55:42 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.75 1999/03/01 00:10:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -326,16 +326,6 @@ _copyMergeJoin(MergeJoin *from)
 	 */
 	Node_Copy(from, newnode, mergeclauses);
 
-	newnode->mergejoinop = from->mergejoinop;
-
-	newnode->mergerightorder = (Oid *) palloc(sizeof(Oid) * 2);
-	newnode->mergerightorder[0] = from->mergerightorder[0];
-	newnode->mergerightorder[1] = 0;
-
-	newnode->mergeleftorder = (Oid *) palloc(sizeof(Oid) * 2);
-	newnode->mergeleftorder[0] = from->mergeleftorder[0];
-	newnode->mergeleftorder[1] = 0;
-
 	return newnode;
 }
 

src/backend/nodes/freefuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/Attic/freefuncs.c,v 1.14 1999/02/22 19:55:42 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/Attic/freefuncs.c,v 1.15 1999/03/01 00:10:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -267,9 +267,6 @@ _freeMergeJoin(MergeJoin *node)
 	 */
 	freeObject(node->mergeclauses);
 
-	pfree(node->mergerightorder);
-	pfree(node->mergeleftorder);
-
 	pfree(node);
 }
 

src/backend/nodes/outfuncs.c

@@ -5,7 +5,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- *  $Id: outfuncs.c,v 1.76 1999/02/23 08:01:47 thomas Exp $
+ *  $Id: outfuncs.c,v 1.77 1999/03/01 00:10:31 tgl Exp $
  *
  * NOTES
  *	  Every (plan) node in POSTGRES has an associated "out" routine which
@@ -371,12 +371,6 @@ _outMergeJoin(StringInfo str, MergeJoin *node)
 
 	appendStringInfo(str, " :mergeclauses ");
 	_outNode(str, node->mergeclauses);
-
-	appendStringInfo(str, 
-			" :mergejoinop %u :mergerightorder %u :mergeleftorder %u ",
-			node->mergejoinop, 
-			node->mergerightorder,
-			node->mergeleftorder);
 }
 
 /*

src/backend/nodes/readfuncs.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.59 1999/02/18 00:49:15 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.60 1999/03/01 00:10:31 tgl Exp $
  *
  * NOTES
  *	  Most of the read functions for plan nodes are tested. (In fact, they
@@ -426,10 +426,6 @@ _readMergeJoin()
 	token = lsptok(NULL, &length);		/* eat :mergeclauses */
 	local_node->mergeclauses = nodeRead(true);	/* now read it */
 
-	token = lsptok(NULL, &length);		/* eat :mergejoinop */
-	token = lsptok(NULL, &length);		/* get mergejoinop */
-	local_node->mergejoinop = atol(token);
-
 	return local_node;
 }
 

src/backend/optimizer/path/mergeutils.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/mergeutils.c,v 1.19 1999/02/15 03:22:06 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/mergeutils.c,v 1.20 1999/03/01 00:10:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -27,6 +27,14 @@
  *	  it within a mergeinfo node containing other clause nodes with the same
  *	  mergejoin ordering.
  *
+ * XXX This is completely braindead: there is no reason anymore to segregate
+ * mergejoin clauses by join operator, since the executor can handle mergejoin
+ * clause sets with different operators in them.  Instead, we ought to be
+ * building a MergeInfo for each potentially useful ordering of the input
+ * relations.  But right now the optimizer's internal data structures do not
+ * support that (MergeInfo can only store one MergeOrder for a set of clauses).
+ * Something to fix next time...
+ *
  * 'restrictinfo_list' is the list of restrictinfo nodes
  * 'inner_relid' is the relid of the inner join relation
  *
@@ -38,7 +46,7 @@ group_clauses_by_order(List *restrictinfo_list,
 					   int inner_relid)
 {
 	List	   *mergeinfo_list = NIL;
-	List	   *xrestrictinfo = NIL;
+	List	   *xrestrictinfo;
 
 	foreach(xrestrictinfo, restrictinfo_list)
 	{
@@ -84,10 +92,10 @@ group_clauses_by_order(List *restrictinfo_list,
 									   mergeinfo_list);
 			}
 
-			((JoinMethod *) xmergeinfo)->clauses = lcons(clause,
+			xmergeinfo->jmethod.clauses = lcons(clause,
-					  ((JoinMethod *) xmergeinfo)->clauses);
+												xmergeinfo->jmethod.clauses);
-			((JoinMethod *) xmergeinfo)->jmkeys = lcons(jmkeys,
+			xmergeinfo->jmethod.jmkeys = lcons(jmkeys,
-					  ((JoinMethod *) xmergeinfo)->jmkeys);
+											   xmergeinfo->jmethod.jmkeys);
 		}
 	}
 	return mergeinfo_list;

src/backend/optimizer/plan/createplan.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.49 1999/02/21 03:48:45 scrappy Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.50 1999/03/01 00:10:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -44,6 +44,8 @@
 #define NONAME_MATERIAL	2
 
 static List *switch_outer(List *clauses);
+static Oid *generate_merge_input_sortorder(List *pathkeys,
+										   MergeOrder *mergeorder);
 static Scan *create_scan_node(Path *best_path, List *tlist);
 static Join *create_join_node(JoinPath *best_path, List *tlist);
 static SeqScan *create_seqscan_node(Path *best_path, List *tlist,
@@ -70,8 +72,7 @@ static HashJoin *make_hashjoin(List *tlist, List *qpqual,
 			  List *hashclauses, Plan *lefttree, Plan *righttree);
 static Hash *make_hash(List *tlist, Var *hashkey, Plan *lefttree);
 static MergeJoin *make_mergejoin(List *tlist, List *qpqual,
-			   List *mergeclauses, Oid opcode, Oid *rightorder,
+			   List *mergeclauses, Plan *righttree, Plan *lefttree);
-			   Oid *leftorder, Plan *righttree, Plan *lefttree);
 static Material *make_material(List *tlist, Oid nonameid, Plan *lefttree,
 			  int keycount);
 
@@ -505,9 +506,6 @@ create_mergejoin_node(MergePath *best_path,
 {
 	List	   *qpqual,
 			   *mergeclauses;
-	RegProcedure opcode;
-	Oid		   *outer_order,
-			   *inner_order;
 	MergeJoin  *join_node;
 
 
@@ -528,27 +526,20 @@ create_mergejoin_node(MergePath *best_path,
 												outer_tlist,
 												inner_tlist));
 
-	opcode = get_opcode((best_path->jpath.path.pathorder->ord.merge)->join_operator);
-
-	outer_order = (Oid *) palloc(sizeof(Oid) * 2);
-	outer_order[0] = (best_path->jpath.path.pathorder->ord.merge)->left_operator;
-	outer_order[1] = 0;
-
-	inner_order = (Oid *) palloc(sizeof(Oid) * 2);
-	inner_order[0] = (best_path->jpath.path.pathorder->ord.merge)->right_operator;
-	inner_order[1] = 0;
-
 	/*
 	 * Create explicit sort paths for the outer and inner join paths if
 	 * necessary.  The sort cost was already accounted for in the path.
 	 */
 	if (best_path->outersortkeys)
 	{
+		Oid		   *outer_order = generate_merge_input_sortorder(
+								best_path->outersortkeys,
+								best_path->jpath.path.pathorder->ord.merge);
 		Noname	   *sorted_outer_node = make_noname(outer_tlist,
-												best_path->outersortkeys,
+													best_path->outersortkeys,
-												  outer_order,
+													outer_order,
-												  outer_node,
+													outer_node,
-												  NONAME_SORT);
+													NONAME_SORT);
 
 		sorted_outer_node->plan.cost = outer_node->cost;
 		outer_node = (Plan *) sorted_outer_node;
@@ -556,22 +547,22 @@ create_mergejoin_node(MergePath *best_path,
 
 	if (best_path->innersortkeys)
 	{
+		Oid		   *inner_order = generate_merge_input_sortorder(
+								best_path->innersortkeys,
+								best_path->jpath.path.pathorder->ord.merge);
 		Noname	   *sorted_inner_node = make_noname(inner_tlist,
 												best_path->innersortkeys,
 												  inner_order,
 												  inner_node,
 												  NONAME_SORT);
 
-		sorted_inner_node->plan.cost = outer_node->cost;
+		sorted_inner_node->plan.cost = outer_node->cost; /* XXX not inner_node? */
 		inner_node = (Plan *) sorted_inner_node;
 	}
 
 	join_node = make_mergejoin(tlist,
 							   qpqual,
 							   mergeclauses,
-							   opcode,
-							   inner_order,
-							   outer_order,
 							   inner_node,
 							   outer_node);
 
@@ -662,7 +653,7 @@ fix_indxqual_references(Node *clause, Path *index_path)
 					pos++;
 				}
 			}
-			newclause = copyObject((Node *) clause);
+			newclause = copyObject(clause);
 			((Var *) newclause)->varattno = pos + 1;
 			return newclause;
 		}
@@ -760,24 +751,22 @@ fix_indxqual_references(Node *clause, Path *index_path)
  * switch_outer
  *	  Given a list of merge clauses, rearranges the elements within the
  *	  clauses so the outer join variable is on the left and the inner is on
- *	  the right.
+ *	  the right.  The original list is not touched; a modified list
- *
+ *	  is returned.
- *	  Returns the rearranged list ?
- *
- *	  XXX Shouldn't the operator be commuted?!
  */
 static List *
 switch_outer(List *clauses)
 {
 	List	   *t_list = NIL;
-	Expr	   *temp = NULL;
+	Expr	   *temp;
-	List	   *i = NIL;
+	List	   *i;
 	Expr	   *clause;
 	Node	   *op;
 
 	foreach(i, clauses)
 	{
 		clause = lfirst(i);
+		Assert(is_opclause((Node*) clause));
 		op = (Node *) get_rightop(clause);
 		Assert(op != (Node*) NULL);
 		if (IsA(op, ArrayRef))
@@ -785,10 +774,16 @@ switch_outer(List *clauses)
 		Assert(IsA(op, Var));
 		if (var_is_outer((Var *) op))
 		{
+			/* Duplicate just enough of the structure to allow commuting
+			 * the clause without changing the original list.  Could use
+			 * copyObject, but a complete deep copy is overkill.
+			 */
 			temp = make_clause(clause->opType, clause->oper,
-							   lcons(get_rightop(clause),
+							   lcons(get_leftop(clause),
-									 lcons(get_leftop(clause),
+									 lcons(get_rightop(clause),
 										   NIL)));
+			/* Commute it --- note this modifies the temp node in-place. */
+			CommuteClause((Node *) temp);
 			t_list = lappend(t_list, temp);
 		}
 		else
@@ -797,6 +792,45 @@ switch_outer(List *clauses)
 	return t_list;
 }
 
+/*
+ * generate_merge_input_sortorder
+ *
+ * Generate the list of sort ops needed to sort one of the input paths for
+ * a merge.  We may have to use either left or right sortop for each item,
+ * since the original mergejoin clause may or may not have been commuted
+ * (compare switch_outer above).
+ *
+ * XXX This is largely a crock.  It works only because group_clauses_by_order
+ * only groups together mergejoin clauses that have identical MergeOrder info,
+ * which means we can safely use a single MergeOrder input to deal with all
+ * the data.  There should be a more general data structure that allows coping
+ * with groups of mergejoin clauses that have different join operators.
+ */
+static Oid *
+generate_merge_input_sortorder(List *pathkeys, MergeOrder *mergeorder)
+{
+	int			listlength = length(pathkeys);
+	Oid		   *result = (Oid*) palloc(sizeof(Oid) * (listlength+1));
+	Oid		   *nextsortop = result;
+	List	   *p;
+
+	foreach(p, pathkeys)
+	{
+		Var		   *pkey = (Var*) lfirst((List*) lfirst(p));
+		Assert(IsA(pkey, Var));
+		if (pkey->vartype == mergeorder->left_type)
+			*nextsortop++ = mergeorder->left_operator;
+		else if (pkey->vartype == mergeorder->right_type)
+			*nextsortop++ = mergeorder->right_operator;
+		else
+			elog(ERROR,
+				 "generate_merge_input_sortorder: can't handle data type %d",
+				 pkey->vartype);
+	}
+	*nextsortop++ = InvalidOid;
+	return result;
+}
+
 /*
  * set_noname_tlist_operators
  *	  Sets the key and keyop fields of resdom nodes in a target list.
@@ -806,18 +840,16 @@ switch_outer(List *clauses)
  *				corresponding to vars in the target list that are to
  *				be sorted or hashed
  *	  'operators' is the corresponding list of N sort or hash operators
- *	  'keyno' is the first key number
- *	  XXX - keyno ? doesn't exist - jeff
  *
- *	  Returns the modified target list.
+ *	  Returns the modified-in-place target list.
  */
 static List *
 set_noname_tlist_operators(List *tlist, List *pathkeys, Oid *operators)
 {
-	Node	   *pathkey = NULL;
 	int			keyno = 1;
-	Resdom	   *resdom = (Resdom *) NULL;
+	Node	   *pathkey;
-	List	   *i = NIL;
+	Resdom	   *resdom;
+	List	   *i;
 
 	foreach(i, pathkeys)
 	{
@@ -828,12 +860,9 @@ set_noname_tlist_operators(List *tlist, List *pathkeys, Oid *operators)
 			/*
 			 * Order the resdom pathkey and replace the operator OID for each
 			 * key with the regproc OID.
-			 *
-			 * XXX Note that the optimizer only generates merge joins with 1
-			 * operator (see create_mergejoin_node)  - ay 2/95
 			 */
 			resdom->reskey = keyno;
-			resdom->reskeyop = get_opcode(operators[0]);
+			resdom->reskeyop = get_opcode(operators[keyno-1]);
 		}
 		keyno += 1;
 	}
@@ -1024,9 +1053,6 @@ static MergeJoin *
 make_mergejoin(List *tlist,
 			   List *qpqual,
 			   List *mergeclauses,
-			   Oid opcode,
-			   Oid *rightorder,
-			   Oid *leftorder,
 			   Plan *righttree,
 			   Plan *lefttree)
 {
@@ -1041,9 +1067,6 @@ make_mergejoin(List *tlist,
 	plan->lefttree = lefttree;
 	plan->righttree = righttree;
 	node->mergeclauses = mergeclauses;
-	node->mergejoinop = opcode;
-	node->mergerightorder = rightorder;
-	node->mergeleftorder = leftorder;
 
 	return node;
 }

src/backend/optimizer/util/clauses.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.31 1999/02/18 00:49:37 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.32 1999/03/01 00:10:35 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -780,24 +780,25 @@ CommuteClause(Node *clause)
 	HeapTuple	heapTup;
 
 	if (!is_opclause(clause))
-		return;
+		elog(ERROR, "CommuteClause: applied to non-operator clause");
 
 	heapTup = (HeapTuple)
 		get_operator_tuple(get_commutator(((Oper *) ((Expr *) clause)->oper)->opno));
 
 	if (heapTup == (HeapTuple) NULL)
-		return;
+		elog(ERROR, "CommuteClause: no commutator for operator %d",
+			 ((Oper *) ((Expr *) clause)->oper)->opno);
 
 	commuTup = (Form_pg_operator) GETSTRUCT(heapTup);
 
 	commu = makeOper(heapTup->t_data->t_oid,
-					 InvalidOid,
+					 commuTup->oprcode,
 					 commuTup->oprresult,
 					 ((Oper *) ((Expr *) clause)->oper)->opsize,
 					 NULL);
 
 	/*
-	 * reform the clause -> (operator func/var constant)
+	 * re-form the clause in-place!
 	 */
 	((Expr *) clause)->oper = (Node *) commu;
 	temp = lfirst(((Expr *) clause)->args);

src/include/nodes/plannodes.h

@@ -6,7 +6,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: plannodes.h,v 1.22 1999/02/13 23:21:40 momjian Exp $
+ * $Id: plannodes.h,v 1.23 1999/03/01 00:10:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -208,9 +208,6 @@ typedef struct MergeJoin
 {
 	Join		join;
 	List	   *mergeclauses;
-	Oid			mergejoinop;
-	Oid		   *mergerightorder;/* inner sort operator */
-	Oid		   *mergeleftorder; /* outer sort operator */
 	MergeJoinState *mergestate;
 } MergeJoin;
 

src/test/regress/expected/opr_sanity.out

@@ -51,7 +51,7 @@ WHERE p1.oid != p2.oid AND
     p1.oprkind = p2.oprkind AND
     p1.oprleft = p2.oprleft AND
     p1.oprright = p2.oprright;
-oid|oprcode|oid|oprcode       
+oid|oprcode|oid|oprcode
 ---+-------+---+-------
 (0 rows)
 
@@ -108,3 +108,32 @@ oid|oprcode|oid|oprcode
 ---+-------+---+-------
 (0 rows)
 
+QUERY: SELECT p1.oid, p1.* FROM pg_operator AS p1
+WHERE p1.oprlsortop != 0 AND
+      p1.oprcom = 0;
+oid|oprname|oprowner|oprprec|oprkind|oprisleft|oprcanhash|oprleft|oprright|oprresult|oprcom|oprnegate|oprlsortop|oprrsortop|oprcode|oprrest|oprjoin
+---+-------+--------+-------+-------+---------+----------+-------+--------+---------+------+---------+----------+----------+-------+-------+-------
+(0 rows)
+
+QUERY: SELECT p1.oid, p1.* FROM pg_operator AS p1
+WHERE p1.oprlsortop != 0 AND NOT
+      EXISTS(SELECT * FROM pg_operator AS p2 WHERE
+	p2.oprname = '<' AND
+	p2.oprleft = p1.oprleft AND
+	p2.oprright = p1.oprright AND
+	p2.oprkind = 'b');
+oid|oprname|oprowner|oprprec|oprkind|oprisleft|oprcanhash|oprleft|oprright|oprresult|oprcom|oprnegate|oprlsortop|oprrsortop|oprcode|oprrest|oprjoin
+---+-------+--------+-------+-------+---------+----------+-------+--------+---------+------+---------+----------+----------+-------+-------+-------
+(0 rows)
+
+QUERY: SELECT p1.oid, p1.* FROM pg_operator AS p1
+WHERE p1.oprlsortop != 0 AND NOT
+      EXISTS(SELECT * FROM pg_operator AS p2 WHERE
+	p2.oprname = '>' AND
+	p2.oprleft = p1.oprleft AND
+	p2.oprright = p1.oprright AND
+	p2.oprkind = 'b');
+oid|oprname|oprowner|oprprec|oprkind|oprisleft|oprcanhash|oprleft|oprright|oprresult|oprcom|oprnegate|oprlsortop|oprrsortop|oprcode|oprrest|oprjoin
+---+-------+--------+-------+-------+---------+----------+-------+--------+---------+------+---------+----------+----------+-------+-------+-------
+(0 rows)
+

src/test/regress/sql/opr_sanity.sql

@@ -81,8 +81,8 @@ WHERE p1.oprnegate = p2.oid AND
      p2.oprresult != 16 OR
      p1.oid != p2.oprnegate);
 
--- Look for mergesort operators that don't match.
+-- Look for mergejoin operators that don't match their links.
--- A mergesort link leads from an '=' operator to the
+-- A mergejoin link leads from an '=' operator to the
 -- sort operator ('<' operator) that's appropriate for
 -- its left-side or right-side data type.
 
@@ -107,3 +107,29 @@ WHERE p1.oprrsortop = p2.oid AND
      p1.oprresult != 16 OR
      p2.oprresult != 16 OR
      p1.oprlsortop = 0);
+
+-- A mergejoinable = operator must have a commutator (usually itself)
+-- as well as corresponding < and > operators.  Note that the "corresponding"
+-- operators have the same L and R input datatypes as the = operator,
+-- whereas the operators linked to by oprlsortop and oprrsortop have input
+-- datatypes L,L and R,R respectively.
+
+SELECT p1.oid, p1.* FROM pg_operator AS p1
+WHERE p1.oprlsortop != 0 AND
+      p1.oprcom = 0;
+
+SELECT p1.oid, p1.* FROM pg_operator AS p1
+WHERE p1.oprlsortop != 0 AND NOT
+      EXISTS(SELECT * FROM pg_operator AS p2 WHERE
+	p2.oprname = '<' AND
+	p2.oprleft = p1.oprleft AND
+	p2.oprright = p1.oprright AND
+	p2.oprkind = 'b');
+
+SELECT p1.oid, p1.* FROM pg_operator AS p1
+WHERE p1.oprlsortop != 0 AND NOT
+      EXISTS(SELECT * FROM pg_operator AS p2 WHERE
+	p2.oprname = '>' AND
+	p2.oprleft = p1.oprleft AND
+	p2.oprright = p1.oprright AND
+	p2.oprkind = 'b');