diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 8810ae005a2e464ee413e9d7078d3acdea1fb20f..8f8e65c374a52a22fd268348fe9c9124ad8a174a 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/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;
}
diff --git a/src/backend/nodes/freefuncs.c b/src/backend/nodes/freefuncs.c
index dd3f040ad4534ad19eb1ae0ec70e5d00dde1c8a4..c41c5fb4318d7bacba338d1b6ee9853d87a1e831 100644
--- a/src/backend/nodes/freefuncs.c
+++ b/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);
}
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 78c90ff420720746ecaaa6a5108702abfd4406b6..11f79e4722001f0870225a38545c904576787a5b 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/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);
}
/*
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index f0fa81a4e2e417443005803680e5aa36b714a374..3e1dca5a345860cad9c727efb4367fd477d9ed24 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/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;
}
diff --git a/src/backend/optimizer/path/mergeutils.c b/src/backend/optimizer/path/mergeutils.c
index 7d0009c3e8b1f39d1a98665a1d6405a0fe07e5a0..5f5a11bc7874406a13deca1c206cf979ed00bfed 100644
--- a/src/backend/optimizer/path/mergeutils.c
+++ b/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,
- ((JoinMethod *) xmergeinfo)->clauses);
- ((JoinMethod *) xmergeinfo)->jmkeys = lcons(jmkeys,
- ((JoinMethod *) xmergeinfo)->jmkeys);
+ xmergeinfo->jmethod.clauses = lcons(clause,
+ xmergeinfo->jmethod.clauses);
+ xmergeinfo->jmethod.jmkeys = lcons(jmkeys,
+ xmergeinfo->jmethod.jmkeys);
}
}
return mergeinfo_list;
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 68e40a4e08f9e9251716a03544ec1087ed0cd840..10d7f559130673d264b7da0cf108841847952add 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/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,
- Oid *leftorder, Plan *righttree, Plan *lefttree);
+ List *mergeclauses, 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,
- outer_order,
- outer_node,
- NONAME_SORT);
+ best_path->outersortkeys,
+ outer_order,
+ outer_node,
+ 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.
- *
- * Returns the rearranged list ?
- *
- * XXX Shouldn't the operator be commuted?!
+ * the right. The original list is not touched; a modified list
+ * is returned.
*/
static List *
switch_outer(List *clauses)
{
List *t_list = NIL;
- Expr *temp = NULL;
- List *i = NIL;
+ Expr *temp;
+ 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;
- List *i = NIL;
+ Node *pathkey;
+ 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;
}
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 3fb0a901a045c0137aae182e9f2f194edbd09403..113f78103b629a974a965324336475f02e29eae2 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/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);
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 40883fa7a866345cdd02dc79dc8332ec02c3f678..3d3b2ca917d5cdc8942d23b9d4c6e960d4c58e06 100644
--- a/src/include/nodes/plannodes.h
+++ b/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;
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 92cb1be9485412867ffd3371f9bbf7595ece508e..b720c5d9cbbccf66658789824a0e1e48ec16162d 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/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)
+
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index c2567d294a98fb6864e81dfad30e262ce90e5b18..1a522468163d0f8b3deae402237c35be8274d43c 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/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.
--- A mergesort link leads from an '=' operator to the
+-- Look for mergejoin operators that don't match their links.
+-- 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');