diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 94a0b99cc3941872ac01097ae56a6994cc0d7837..01e663b255bd932c187364e1573eae00a2c66a05 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/path/indxpath.c,v 1.67 1999/07/30 04:07:23 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/path/indxpath.c,v 1.68 1999/08/12 04:32:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -33,6 +33,7 @@ #include "optimizer/paths.h" #include "optimizer/plancat.h" #include "optimizer/restrictinfo.h" +#include "optimizer/var.h" #include "parser/parse_coerce.h" #include "parser/parse_expr.h" #include "parser/parse_oper.h" @@ -56,6 +57,8 @@ static List *group_clauses_by_ikey_for_joins(RelOptInfo *rel, RelOptInfo *index, static bool match_clause_to_indexkey(RelOptInfo *rel, RelOptInfo *index, int indexkey, int xclass, Expr *clause, bool join); +static bool indexable_operator(Expr *clause, int xclass, Oid relam, + bool indexkey_on_left); static bool pred_test(List *predicate_list, List *restrictinfo_list, List *joininfo_list); static bool one_pred_test(Expr *predicate, List *restrictinfo_list); @@ -68,7 +71,7 @@ static void indexable_joinclauses(RelOptInfo *rel, RelOptInfo *index, static List *index_innerjoin(Query *root, RelOptInfo *rel, RelOptInfo *index, List *clausegroup_list, List *outerrelids_list); static bool useful_for_mergejoin(RelOptInfo *index, List *clausegroup_list); -static bool match_index_to_operand(int indexkey, Expr *operand, +static bool match_index_to_operand(int indexkey, Var *operand, RelOptInfo *rel, RelOptInfo *index); static bool function_index_operand(Expr *funcOpnd, RelOptInfo *rel, RelOptInfo *index); static bool match_special_index_operator(Expr *clause, bool indexkey_on_left); @@ -285,7 +288,7 @@ match_index_orclauses(RelOptInfo *rel, * A match means that: * (1) the operator within the subclause can be used with the * index's specified operator class, and - * (2) the variable on one side of the subclause matches the index key. + * (2) one operand of the subclause matches the index key. * * 'or_clauses' is the list of subclauses within the 'or' clause * 'other_matching_indices' is the list of information on other indices @@ -528,26 +531,28 @@ group_clauses_by_ikey_for_joins(RelOptInfo *rel, * Determines whether a restriction or join clause matches * a key of an index. * - * To match, the clause must: - * (1) be in the form (var op const) for a restriction clause, - * or (var op var) for a join clause, where the var or one - * of the vars matches the index key; and - * (2) contain an operator which is in the same class as the index - * operator for this key, or is a "special" operator as recognized - * by match_special_index_operator(). + * To match, the clause: + + * (1a) for a restriction clause: must be in the form (indexkey op const) + * or (const op indexkey), or + * (1b) for a join clause: must be in the form (indexkey op others) + * or (others op indexkey), where others is an expression involving + * only vars of the other relation(s); and + * (2) must contain an operator which is in the same class as the index + * operator for this key, or is a "special" operator as recognized + * by match_special_index_operator(). * - * In the restriction case, we can cope with (const op var) by commuting - * the clause to (var op const), if there is a commutator operator. - * XXX why do we bother to commute? The executor doesn't care!! + * Presently, the executor can only deal with indexquals that have the + * indexkey on the left, so we can only use clauses that have the indexkey + * on the right if we can commute the clause to put the key on the left. + * We do not actually do the commuting here, but we check whether a + * suitable commutator operator is available. * - * In the join case, later code will try to commute the clause if needed - * to put the inner relation's var on the right. We have no idea here - * which relation might wind up on the inside, so we just accept - * a match for either var. - * XXX is this right? We are making a list for this relation to - * be an inner join relation, so if there is any commuting then - * this rel must be on the right. But again, it's not really clear - * that we have to commute at all! + * Note that in the join case, we already know that the clause as a + * whole uses vars from the interesting set of relations. But we need + * to defend against expressions like (a.f1 OP (b.f2 OP a.f3)); that's + * not processable by an indexscan nestloop join, whereas + * (a.f1 OP (b.f2 OP c.f3)) is. * * 'rel' is the relation of interest. * 'index' is an index on 'rel'. @@ -568,162 +573,181 @@ match_clause_to_indexkey(RelOptInfo *rel, Expr *clause, bool join) { - bool isIndexable = false; Var *leftop, *rightop; - Oid expr_op; + /* Clause must be a binary opclause. */ if (! is_opclause((Node *) clause)) return false; leftop = get_leftop(clause); rightop = get_rightop(clause); if (! leftop || ! rightop) return false; - expr_op = ((Oper *) clause->oper)->opno; if (!join) { /* * Not considering joins, so check for clauses of the form: - * (var/func operator constant) and (constant operator var/func) + * (indexkey operator constant) or (constant operator indexkey). + * We will accept a Param as being constant. */ - /* - * Check for standard s-argable clause - */ if ((IsA(rightop, Const) || IsA(rightop, Param)) && - match_index_to_operand(indexkey, (Expr *) leftop, - rel, index)) + match_index_to_operand(indexkey, leftop, rel, index)) { - isIndexable = op_class(expr_op, xclass, index->relam); - -#ifndef IGNORE_BINARY_COMPATIBLE_INDICES + if (indexable_operator(clause, xclass, index->relam, true)) + return true; /* - * Didn't find an index? Then maybe we can find another - * binary-compatible index instead... thomas 1998-08-14 + * If we didn't find a member of the index's opclass, + * see whether it is a "special" indexable operator. */ - if (!isIndexable) - { - Oid ltype = exprType((Node *) leftop); - Oid rtype = exprType((Node *) rightop); - - /* - * make sure we have two different binary-compatible - * types... - */ - if (ltype != rtype && IS_BINARY_COMPATIBLE(ltype, rtype)) - { - char *opname = get_opname(expr_op); - Operator newop = NULL; - - if (opname != NULL) - newop = oper(opname, ltype, ltype, TRUE); - - /* actually have a different operator to try? */ - if (HeapTupleIsValid(newop) && oprid(newop) != expr_op) - { - expr_op = oprid(newop); - isIndexable = op_class(expr_op, xclass, index->relam); - if (isIndexable) - ((Oper *) clause->oper)->opno = expr_op; - } - } - } -#endif - + if (match_special_index_operator(clause, true)) + return true; + return false; + } + if ((IsA(leftop, Const) || IsA(leftop, Param)) && + match_index_to_operand(indexkey, rightop, rel, index)) + { + if (indexable_operator(clause, xclass, index->relam, false)) + return true; /* * If we didn't find a member of the index's opclass, * see whether it is a "special" indexable operator. */ - if (!isIndexable) - isIndexable = match_special_index_operator(clause, true); - + if (match_special_index_operator(clause, false)) + return true; + return false; } - + } + else + { /* - * Must try to commute the clause to standard s-arg format. - * XXX do we really have to commute it? The executor doesn't care! + * Check for an indexqual that could be handled by a nestloop join. + * We need the index key to be compared against an expression + * that uses none of the indexed relation's vars. */ - else if ((IsA(leftop, Const) || IsA(leftop, Param)) && - match_index_to_operand(indexkey, (Expr *) rightop, - rel, index)) + if (match_index_to_operand(indexkey, leftop, rel, index)) { - Oid commuted_op = get_commutator(expr_op); + List *othervarnos = pull_varnos((Node *) rightop); + bool isIndexable; - isIndexable = ((commuted_op != InvalidOid) && - op_class(commuted_op, xclass, index->relam)); + isIndexable = ! intMember(lfirsti(rel->relids), othervarnos); + freeList(othervarnos); + if (isIndexable && + indexable_operator(clause, xclass, index->relam, true)) + return true; + } + else if (match_index_to_operand(indexkey, rightop, rel, index)) + { + List *othervarnos = pull_varnos((Node *) leftop); + bool isIndexable; -#ifndef IGNORE_BINARY_COMPATIBLE_INDICES - if (!isIndexable) - { - Oid ltype = exprType((Node *) leftop); - Oid rtype = exprType((Node *) rightop); + isIndexable = ! intMember(lfirsti(rel->relids), othervarnos); + freeList(othervarnos); + if (isIndexable && + indexable_operator(clause, xclass, index->relam, false)) + return true; + } + } - if (ltype != rtype && IS_BINARY_COMPATIBLE(ltype, rtype)) - { - char *opname = get_opname(expr_op); - Operator newop = NULL; - - /* note we use rtype, ie, the indexkey's type */ - if (opname != NULL) - newop = oper(opname, rtype, rtype, TRUE); - - if (HeapTupleIsValid(newop) && oprid(newop) != expr_op) - { - expr_op = get_commutator(oprid(newop)); - isIndexable = (expr_op != InvalidOid) && - op_class(expr_op, xclass, index->relam); - if (isIndexable) - ((Oper *) clause->oper)->opno = oprid(newop); - } - } - } -#endif + return false; +} - if (isIndexable) - { - /* - * In place list modification. (op const var/func) -> (op - * var/func const) - */ - CommuteClause((Node *) clause); - } - else +/* + * indexable_operator + * Does a binary opclause contain an operator matching the index's + * access method? + * + * If the indexkey is on the right, what we actually want to know + * is whether the operator has a commutator operator that matches + * the index's access method. + * + * We try both the straightforward match and matches that rely on + * recognizing binary-compatible datatypes. For example, if we have + * an expression like "oid = 123", the operator will be oideqint4, + * which we need to replace with oideq in order to recognize it as + * matching an oid_ops index on the oid field. + * + * NOTE: if a binary-compatible match is made, we destructively modify + * the given clause to use the binary-compatible substitute operator! + * This should be safe even if we don't end up using the index, but it's + * a tad ugly... + */ +static bool +indexable_operator(Expr *clause, int xclass, Oid relam, + bool indexkey_on_left) +{ + Oid expr_op = ((Oper *) clause->oper)->opno; + Oid commuted_op; + Oid ltype, + rtype; + + /* Get the commuted operator if necessary */ + if (indexkey_on_left) + commuted_op = expr_op; + else + commuted_op = get_commutator(expr_op); + if (commuted_op == InvalidOid) + return false; + + /* Done if the (commuted) operator is a member of the index's AM */ + if (op_class(commuted_op, xclass, relam)) + return true; + + /* + * Maybe the index uses a binary-compatible operator set. + */ + ltype = exprType((Node *) get_leftop(clause)); + rtype = exprType((Node *) get_rightop(clause)); + + /* + * make sure we have two different binary-compatible types... + */ + if (ltype != rtype && IS_BINARY_COMPATIBLE(ltype, rtype)) + { + char *opname = get_opname(expr_op); + Operator newop; + + if (opname == NULL) + return false; /* probably shouldn't happen */ + + /* Use the datatype of the index key */ + if (indexkey_on_left) + newop = oper(opname, ltype, ltype, TRUE); + else + newop = oper(opname, rtype, rtype, TRUE); + + if (HeapTupleIsValid(newop)) + { + Oid new_expr_op = oprid(newop); + + if (new_expr_op != expr_op) { /* - * If we didn't find a member of the index's opclass, - * see whether it is a "special" indexable operator. - * (match_special_index_operator must commute the - * clause itself, if it wants to.) + * OK, we found a binary-compatible operator of the same name; + * now does it match the index? */ - isIndexable = match_special_index_operator(clause, false); + if (indexkey_on_left) + commuted_op = new_expr_op; + else + commuted_op = get_commutator(new_expr_op); + if (commuted_op == InvalidOid) + return false; + + if (op_class(commuted_op, xclass, relam)) + { + /* + * Success! Change the opclause to use the + * binary-compatible operator. + */ + ((Oper *) clause->oper)->opno = new_expr_op; + return true; + } } } } - else - { - /* - * Check for an indexable scan on one of the join relations. - * clause is of the form (operator var/func var/func) - * XXX this does not seem right. Should check other side - * looks like var/func? do we really want to only consider - * this rel on lefthand side?? - */ - Oid join_op = InvalidOid; - - if (match_index_to_operand(indexkey, (Expr *) leftop, - rel, index)) - join_op = expr_op; - else if (match_index_to_operand(indexkey, (Expr *) rightop, - rel, index)) - join_op = get_commutator(expr_op); - - if (join_op && op_class(join_op, xclass, index->relam) && - is_joinable((Node *) clause)) - isIndexable = true; - } - return isIndexable; + return false; } /**************************************************************************** @@ -1315,7 +1339,7 @@ useful_for_mergejoin(RelOptInfo *index, */ static bool match_index_to_operand(int indexkey, - Expr *operand, + Var *operand, RelOptInfo *rel, RelOptInfo *index) { @@ -1324,19 +1348,19 @@ match_index_to_operand(int indexkey, /* * Normal index. */ - return match_indexkey_operand(indexkey, (Var *) operand, rel); + return match_indexkey_operand(indexkey, operand, rel); } /* * functional index check */ - return function_index_operand(operand, rel, index); + return function_index_operand((Expr *) operand, rel, index); } static bool function_index_operand(Expr *funcOpnd, RelOptInfo *rel, RelOptInfo *index) { - Oid heapRelid = (Oid) lfirsti(rel->relids); + int relvarno = lfirsti(rel->relids); Func *function; List *funcargs; int *indexKeys = index->indexkeys; @@ -1346,8 +1370,8 @@ function_index_operand(Expr *funcOpnd, RelOptInfo *rel, RelOptInfo *index) /* * sanity check, make sure we know what we're dealing with here. */ - if (funcOpnd == NULL || - nodeTag(funcOpnd) != T_Expr || funcOpnd->opType != FUNC_EXPR || + if (funcOpnd == NULL || ! IsA(funcOpnd, Expr) || + funcOpnd->opType != FUNC_EXPR || funcOpnd->oper == NULL || indexKeys == NULL) return false; @@ -1362,27 +1386,17 @@ function_index_operand(Expr *funcOpnd, RelOptInfo *rel, RelOptInfo *index) * create the functional index. To do this we must check that 1. * refer to the right relatiion. 2. the args have the right attr. * numbers in the right order. - * - * Check all args refer to the correct relation (i.e. the one with the - * functional index defined on it (rel). To do this we can simply - * compare range table entry numbers, they must be the same. - */ - foreach(arg, funcargs) - { - if (heapRelid != ((Var *) lfirst(arg))->varno) - return false; - } - - /* - * check attr numbers and order. */ i = 0; foreach(arg, funcargs) { + Var *var = (Var *) lfirst(arg); + + if (! IsA(var, Var)) + return false; if (indexKeys[i] == 0) return false; - - if (((Var *) lfirst(arg))->varattno != indexKeys[i]) + if (var->varno != relvarno || var->varattno != indexKeys[i]) return false; i++; diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 64213cde7b1c8c5c3836c56267cb01749fe466b1..281e36851608481c45ef2d8cb68bb84252c6a3c8 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.69 1999/08/10 02:58:56 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.70 1999/08/12 04:32:53 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -49,10 +49,10 @@ static HashJoin *create_hashjoin_node(HashPath *best_path, List *tlist, List *clauses, Plan *outer_node, List *outer_tlist, Plan *inner_node, List *inner_tlist); static List *fix_indxqual_references(List *indexquals, IndexPath *index_path); -static List *fix_one_indxqual_sublist(List *indexqual, IndexPath *index_path, - Form_pg_index index); -static Node *fix_one_indxqual_operand(Node *node, IndexPath *index_path, - Form_pg_index index); +static List *fix_indxqual_sublist(List *indexqual, IndexPath *index_path, + Form_pg_index index); +static Node *fix_indxqual_operand(Node *node, IndexPath *index_path, + Form_pg_index index); static Noname *make_noname(List *tlist, List *pathkeys, Oid *operators, Plan *plan_node, int nonametype); static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid, @@ -345,8 +345,8 @@ create_indexscan_node(IndexPath *best_path, /* * The qpqual list must contain all restrictions not automatically * handled by the index. Note that for non-lossy indices, the - * predicates in the indxqual are handled by the index, while for - * lossy indices the indxqual predicates need to be double-checked + * predicates in the indxqual are checked fully by the index, while + * for lossy indices the indxqual predicates need to be double-checked * after the index fetches the best-guess tuples. * * Since the indexquals were generated from the restriction clauses @@ -390,14 +390,17 @@ create_indexscan_node(IndexPath *best_path, else qpqual = NIL; + /* The executor needs a copy with the indexkey on the left of each clause + * and with index attrs substituted for table ones. + */ + fixed_indxqual = fix_indxqual_references(indxqual, best_path); + /* - * Fix opids in the completed indxqual. + * Fix opids in the completed indxquals. * XXX this ought to only happen at final exit from the planner... */ indxqual = fix_opids(indxqual); - - /* The executor needs a copy with index attrs substituted for table ones */ - fixed_indxqual = fix_indxqual_references(indxqual, best_path); + fixed_indxqual = fix_opids(fixed_indxqual); scan_node = make_indexscan(tlist, qpqual, @@ -445,11 +448,6 @@ create_nestloop_node(NestPath *best_path, * checked as qpquals in the indexscan. We can still remove them * from the nestloop's qpquals, but we gotta update the outer-rel * vars in the indexscan's qpquals too... - * - * XXX as of 8/99, removal of redundant joinclauses doesn't work - * all the time, since it will fail to recognize clauses that have - * been commuted in the indexqual. I hope to make this problem go - * away soon by not commuting indexqual clauses --- tgl. */ IndexScan *innerscan = (IndexScan *) inner_node; List *indxqualorig = innerscan->indxqualorig; @@ -459,7 +457,8 @@ create_nestloop_node(NestPath *best_path, { /* Remove redundant tests from my clauses, if possible. * Note we must compare against indxqualorig not the "fixed" - * indxqual (which has index attnos instead of relation attnos). + * indxqual (which has index attnos instead of relation attnos, + * and may have been commuted as well). */ if (length(indxqualorig) == 1) /* single indexscan? */ clauses = set_difference(clauses, lfirst(indxqualorig)); @@ -471,6 +470,7 @@ create_nestloop_node(NestPath *best_path, innerscan->indxqual = join_references(innerscan->indxqual, outer_tlist, NIL); + /* fix the inner qpqual too, if it has join clauses */ if (NumRelids((Node *) inner_node->qual) > 1) inner_node->qual = join_references(inner_node->qual, outer_tlist, @@ -638,8 +638,10 @@ create_hashjoin_node(HashPath *best_path, /* * fix_indxqual_references - * Adjust indexqual clauses to refer to index attributes instead of the - * attributes of the original relation. + * Adjust indexqual clauses to refer to index attributes instead of the + * attributes of the original relation. Also, commute clauses if needed + * to put the indexkey on the left. (Someday the executor might not need + * that, but for now it does.) * * This code used to be entirely bogus for multi-index scans. Now it keeps * track of which index applies to each subgroup of index qual clauses... @@ -671,9 +673,9 @@ fix_indxqual_references(List *indexquals, IndexPath *index_path) index = (Form_pg_index) GETSTRUCT(indexTuple); fixed_quals = lappend(fixed_quals, - fix_one_indxqual_sublist(indexqual, - index_path, - index)); + fix_indxqual_sublist(indexqual, + index_path, + index)); indexids = lnext(indexids); } @@ -683,41 +685,52 @@ fix_indxqual_references(List *indexquals, IndexPath *index_path) /* * Fix the sublist of indexquals to be used in a particular scan. * - * All that we need to do is change the left or right operand of the top-level - * operator of each qual clause. Those are the only places that the index - * attribute can appear in a valid indexqual. The other side of the indexqual - * might be a complex function of joined rels; we do not need or want to - * alter such an expression. + * For each qual clause, commute if needed to put the indexkey operand on the + * left, and then change its varno. We do not need to change the other side + * of the clause. */ static List * -fix_one_indxqual_sublist(List *indexqual, IndexPath *index_path, - Form_pg_index index) +fix_indxqual_sublist(List *indexqual, IndexPath *index_path, + Form_pg_index index) { List *fixed_qual = NIL; List *i; foreach(i, indexqual) { - Node *clause = lfirst(i); - List *args; + Expr *clause = (Expr *) lfirst(i); + int relid; + AttrNumber attno; + Datum constval; + int flag; Expr *newclause; - if (!is_opclause(clause)) - elog(ERROR, "fix_one_indxqual_sublist: indexqual clause is not opclause"); + if (!is_opclause((Node *) clause) || + length(clause->args) != 2) + elog(ERROR, "fix_indxqual_sublist: indexqual clause is not binary opclause"); + + /* Which side is the indexkey on? + * + * get_relattval sets flag&SEL_RIGHT if the indexkey is on the LEFT. + */ + get_relattval((Node *) clause, + lfirsti(index_path->path.parent->relids), + &relid, &attno, &constval, &flag); + + /* Copy enough structure to allow commuting and replacing an operand + * without changing original clause. + */ + newclause = make_clause(clause->opType, clause->oper, + listCopy(clause->args)); - /* Copy enough structure to allow replacing left or right operand */ - args = listCopy(((Expr *) clause)->args); - newclause = make_clause(((Expr *) clause)->opType, - ((Expr *) clause)->oper, - args); + /* If the indexkey is on the right, commute the clause. */ + if ((flag & SEL_RIGHT) == 0) + CommuteClause(newclause); - lfirst(args) = fix_one_indxqual_operand(lfirst(args), - index_path, - index); - if (lnext(args)) - lfirst(lnext(args)) = fix_one_indxqual_operand(lfirst(lnext(args)), - index_path, - index); + /* Now, change the indexkey operand as needed. */ + lfirst(newclause->args) = fix_indxqual_operand(lfirst(newclause->args), + index_path, + index); fixed_qual = lappend(fixed_qual, newclause); } @@ -725,11 +738,9 @@ fix_one_indxqual_sublist(List *indexqual, IndexPath *index_path, } static Node * -fix_one_indxqual_operand(Node *node, IndexPath *index_path, - Form_pg_index index) +fix_indxqual_operand(Node *node, IndexPath *index_path, + Form_pg_index index) { - if (node == NULL) - return NULL; if (IsA(node, Var)) { if (((Var *) node)->varno == lfirsti(index_path->path.parent->relids)) @@ -746,22 +757,17 @@ fix_one_indxqual_operand(Node *node, IndexPath *index_path, return newnode; } } - /* - * We should never see a reference to an attribute of the indexed - * relation that is not one of the indexed attributes. - */ - elog(ERROR, "fix_one_indxqual_operand: failed to find index pos of index attribute"); } /* - * The Var is not part of the indexed relation, leave it alone. - * This would normally only occur when looking at the other side - * of a join indexqual. + * Oops, this Var isn't the indexkey! */ - return node; + elog(ERROR, "fix_indxqual_operand: var is not index attribute"); } /* - * Note: currently, there is no need for us to do anything here for + * Else, it must be a func expression representing a functional index. + * + * Currently, there is no need for us to do anything here for * functional indexes. If nodeIndexscan.c sees a func clause as the left * or right-hand toplevel operand of an indexqual, it assumes that that is * a reference to the functional index's value and makes the appropriate @@ -789,7 +795,7 @@ switch_outer(List *clauses) foreach(i, clauses) { - Expr *clause = lfirst(i); + Expr *clause = (Expr *) lfirst(i); Node *op; Assert(is_opclause((Node *) clause)); @@ -808,11 +814,9 @@ switch_outer(List *clauses) Expr *temp; temp = make_clause(clause->opType, clause->oper, - lcons(get_leftop(clause), - lcons(get_rightop(clause), - NIL))); + listCopy(clause->args)); /* Commute it --- note this modifies the temp node in-place. */ - CommuteClause((Node *) temp); + CommuteClause(temp); t_list = lappend(t_list, temp); } else diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 322aeba1ce3db5bce5d257d92329dfb21973401e..f9906cffc04b708baf374783430bc7ac522db642 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.45 1999/08/10 03:00:15 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.46 1999/08/12 04:32:54 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -666,38 +666,39 @@ get_rels_atts(Node *clause, *-------------------- */ void -CommuteClause(Node *clause) +CommuteClause(Expr *clause) { - Node *temp; - Oper *commu; - Form_pg_operator commuTup; HeapTuple heapTup; + Form_pg_operator commuTup; + Oper *commu; + Node *temp; - if (!is_opclause(clause)) - elog(ERROR, "CommuteClause: applied to non-operator clause"); + if (!is_opclause((Node *) clause) || + length(clause->args) != 2) + elog(ERROR, "CommuteClause: applied to non-binary-operator clause"); heapTup = (HeapTuple) - get_operator_tuple(get_commutator(((Oper *) ((Expr *) clause)->oper)->opno)); + get_operator_tuple(get_commutator(((Oper *) clause->oper)->opno)); if (heapTup == (HeapTuple) NULL) elog(ERROR, "CommuteClause: no commutator for operator %u", - ((Oper *) ((Expr *) clause)->oper)->opno); + ((Oper *) clause->oper)->opno); commuTup = (Form_pg_operator) GETSTRUCT(heapTup); commu = makeOper(heapTup->t_data->t_oid, commuTup->oprcode, commuTup->oprresult, - ((Oper *) ((Expr *) clause)->oper)->opsize, + ((Oper *) clause->oper)->opsize, NULL); /* * re-form the clause in-place! */ - ((Expr *) clause)->oper = (Node *) commu; - temp = lfirst(((Expr *) clause)->args); - lfirst(((Expr *) clause)->args) = lsecond(((Expr *) clause)->args); - lsecond(((Expr *) clause)->args) = temp; + clause->oper = (Node *) commu; + temp = lfirst(clause->args); + lfirst(clause->args) = lsecond(clause->args); + lsecond(clause->args) = temp; } diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h index 3ebf442791222c9f6a83d7f6a34fc8e1c2ee3d80..381da4adaeb43f66eb57410004c9bdddf68ea6d3 100644 --- a/src/include/optimizer/clauses.h +++ b/src/include/optimizer/clauses.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: clauses.h,v 1.26 1999/08/10 03:00:12 tgl Exp $ + * $Id: clauses.h,v 1.27 1999/08/12 04:32:49 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -47,7 +47,7 @@ extern void get_relattval(Node *clause, int targetrelid, Datum *constval, int *flag); extern void get_rels_atts(Node *clause, int *relid1, AttrNumber *attno1, int *relid2, AttrNumber *attno2); -extern void CommuteClause(Node *clause); +extern void CommuteClause(Expr *clause); extern bool expression_tree_walker(Node *node, bool (*walker) (), void *context);