From: Tom Lane Date: Thu, 6 May 1999 23:07:33 +0000 (+0000) Subject: Fix oversights in flatten_tlistentry and replace_clause_joinvar_refs X-Git-Tag: REL6_5~303 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ec1f5f78b9ad9932073a9a054205c480155654ca;p=postgresql Fix oversights in flatten_tlistentry and replace_clause_joinvar_refs that led to CASE expressions not working very well in joined queries. --- diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 4506097af9..cf3c3edfc1 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.44 1999/05/03 00:38:43 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.45 1999/05/06 23:07:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -36,11 +36,12 @@ static void set_join_tlist_references(Join *join); static void set_nonamescan_tlist_references(SeqScan *nonamescan); static void set_noname_tlist_references(Noname *noname); -static List *replace_clause_joinvar_refs(Expr *clause, - List *outer_tlist, List *inner_tlist); -static List *replace_subclause_joinvar_refs(List *clauses, - List *outer_tlist, List *inner_tlist); -static Var *replace_joinvar_refs(Var *var, List *outer_tlist, List *inner_tlist); +static Node *replace_clause_joinvar_refs(Node *clause, + List *outer_tlist, + List *inner_tlist); +static Var *replace_joinvar_refs(Var *var, + List *outer_tlist, + List *inner_tlist); static List *tlist_noname_references(Oid nonameid, List *tlist); static bool OperandIsInner(Node *opnd, int inner_relid); static List *pull_agg_clause(Node *clause); @@ -104,27 +105,20 @@ set_join_tlist_references(Join *join) { Plan *outer = ((Plan *) join)->lefttree; Plan *inner = ((Plan *) join)->righttree; + List *outer_tlist = ((outer == NULL) ? NIL : outer->targetlist); + List *inner_tlist = ((inner == NULL) ? NIL : inner->targetlist); List *new_join_targetlist = NIL; - TargetEntry *temp = (TargetEntry *) NULL; - List *entry = NIL; - List *inner_tlist = NULL; - List *outer_tlist = NULL; - TargetEntry *xtl = (TargetEntry *) NULL; List *qptlist = ((Plan *) join)->targetlist; + List *entry; foreach(entry, qptlist) { - List *joinvar; - - xtl = (TargetEntry *) lfirst(entry); - inner_tlist = ((inner == NULL) ? NIL : inner->targetlist); - outer_tlist = ((outer == NULL) ? NIL : outer->targetlist); - joinvar = replace_clause_joinvar_refs((Expr *) get_expr(xtl), - outer_tlist, - inner_tlist); - - temp = makeTargetEntry(xtl->resdom, (Node *) joinvar); - new_join_targetlist = lappend(new_join_targetlist, temp); + TargetEntry *xtl = (TargetEntry *) lfirst(entry); + Node *joinvar = replace_clause_joinvar_refs(xtl->expr, + outer_tlist, + inner_tlist); + new_join_targetlist = lappend(new_join_targetlist, + makeTargetEntry(xtl->resdom, joinvar)); } ((Plan *) join)->targetlist = new_join_targetlist; @@ -182,15 +176,17 @@ set_noname_tlist_references(Noname *noname) /* * join_references - * Creates a new set of join clauses by replacing the varno/varattno + * Creates a new set of join clauses by changing the varno/varattno * values of variables in the clauses to reference target list values * from the outer and inner join relation target lists. + * This is just an external interface for replace_clause_joinvar_refs. * * 'clauses' is the list of join clauses * 'outer_tlist' is the target list of the outer join relation * 'inner_tlist' is the target list of the inner join relation * - * Returns the new join clauses. + * Returns the new join clauses. The original clause structure is + * not modified. * */ List * @@ -198,9 +194,9 @@ join_references(List *clauses, List *outer_tlist, List *inner_tlist) { - return (replace_subclause_joinvar_refs(clauses, - outer_tlist, - inner_tlist)); + return (List *) replace_clause_joinvar_refs((Node *) clauses, + outer_tlist, + inner_tlist); } /* @@ -239,9 +235,9 @@ index_outerjoin_references(List *inner_indxqual, if (OperandIsInner((Node *) get_rightop(clause), inner_relid)) { Var *joinvar = (Var *) - replace_clause_joinvar_refs((Expr *) get_leftop(clause), - outer_tlist, - NIL); + replace_clause_joinvar_refs((Node *) get_leftop(clause), + outer_tlist, + NIL); temp = make_opclause(replace_opid((Oper *) ((Expr *) clause)->oper), joinvar, @@ -252,9 +248,9 @@ index_outerjoin_references(List *inner_indxqual, { /* inner scan on left */ Var *joinvar = (Var *) - replace_clause_joinvar_refs((Expr *) get_rightop(clause), - outer_tlist, - NIL); + replace_clause_joinvar_refs((Node *) get_rightop(clause), + outer_tlist, + NIL); temp = make_opclause(replace_opid((Oper *) ((Expr *) clause)->oper), get_leftop(clause), @@ -268,7 +264,6 @@ index_outerjoin_references(List *inner_indxqual, /* * replace_clause_joinvar_refs - * replace_subclause_joinvar_refs * replace_joinvar_refs * * Replaces all variables within a join clause with a new var node @@ -280,169 +275,177 @@ index_outerjoin_references(List *inner_indxqual, * 'inner_tlist' is the target list of the inner join relation * * Returns the new join clause. + * NB: it is critical that the original clause structure not be modified! + * The changes must be applied to a copy. * + * XXX the current implementation does not copy unchanged primitive + * nodes; they remain shared with the original. Is this safe? */ -static List * -replace_clause_joinvar_refs(Expr *clause, +static Node * +replace_clause_joinvar_refs(Node *clause, List *outer_tlist, List *inner_tlist) { - List *temp = NULL; - if (clause == NULL) return NULL; if (IsA(clause, Var)) { - temp = (List *) replace_joinvar_refs((Var *) clause, - outer_tlist, inner_tlist); + Var *temp = replace_joinvar_refs((Var *) clause, + outer_tlist, inner_tlist); if (temp != NULL) - return temp; - else if (clause != NULL) - return (List *) clause; + return (Node *) temp; else - return NIL; + return clause; } - else if (single_node((Node *) clause)) - return (List *) clause; - else if (and_clause((Node *) clause)) + else if (single_node(clause)) + return clause; + else if (and_clause(clause)) { - List *andclause = replace_subclause_joinvar_refs(((Expr *) clause)->args, - outer_tlist, - inner_tlist); - - return (List *) make_andclause(andclause); + return (Node *) make_andclause((List *) + replace_clause_joinvar_refs((Node *) ((Expr *) clause)->args, + outer_tlist, + inner_tlist)); } - else if (or_clause((Node *) clause)) + else if (or_clause(clause)) { - List *orclause = replace_subclause_joinvar_refs(((Expr *) clause)->args, - outer_tlist, - inner_tlist); - - return (List *) make_orclause(orclause); + return (Node *) make_orclause((List *) + replace_clause_joinvar_refs((Node *) ((Expr *) clause)->args, + outer_tlist, + inner_tlist)); } else if (IsA(clause, ArrayRef)) { - ArrayRef *aref = (ArrayRef *) clause; - - temp = replace_subclause_joinvar_refs(aref->refupperindexpr, - outer_tlist, - inner_tlist); - aref->refupperindexpr = (List *) temp; - temp = replace_subclause_joinvar_refs(aref->reflowerindexpr, - outer_tlist, - inner_tlist); - aref->reflowerindexpr = (List *) temp; - temp = replace_clause_joinvar_refs((Expr *) aref->refexpr, - outer_tlist, - inner_tlist); - aref->refexpr = (Node *) temp; - - /* - * no need to set refassgnexpr. we only set that in the target - * list on replaces, and this is an array reference in the - * qualification. if we got this far, it's 0x0 in the ArrayRef - * structure 'clause'. - */ + ArrayRef *oldnode = (ArrayRef *) clause; + ArrayRef *newnode = makeNode(ArrayRef); + + newnode->refattrlength = oldnode->refattrlength; + newnode->refelemlength = oldnode->refelemlength; + newnode->refelemtype = oldnode->refelemtype; + newnode->refelembyval = oldnode->refelembyval; + newnode->refupperindexpr = (List *) + replace_clause_joinvar_refs((Node *) oldnode->refupperindexpr, + outer_tlist, + inner_tlist); + newnode->reflowerindexpr = (List *) + replace_clause_joinvar_refs((Node *) oldnode->reflowerindexpr, + outer_tlist, + inner_tlist); + newnode->refexpr = + replace_clause_joinvar_refs(oldnode->refexpr, + outer_tlist, + inner_tlist); + newnode->refassgnexpr = + replace_clause_joinvar_refs(oldnode->refassgnexpr, + outer_tlist, + inner_tlist); - return (List *) clause; + return (Node *) newnode; } - else if (is_funcclause((Node *) clause)) + else if (is_funcclause(clause)) { - List *funcclause = replace_subclause_joinvar_refs(((Expr *) clause)->args, - outer_tlist, - inner_tlist); - - return ((List *) make_funcclause((Func *) ((Expr *) clause)->oper, - funcclause)); + return (Node *) make_funcclause( + (Func *) ((Expr *) clause)->oper, + (List *) replace_clause_joinvar_refs( + (Node *) ((Expr *) clause)->args, + outer_tlist, + inner_tlist)); } - else if (not_clause((Node *) clause)) + else if (not_clause(clause)) { - List *notclause = replace_clause_joinvar_refs(get_notclausearg(clause), - outer_tlist, - inner_tlist); - - return (List *) make_notclause((Expr *) notclause); + return (Node *) make_notclause((Expr *) + replace_clause_joinvar_refs( + (Node *) get_notclausearg((Expr *) clause), + outer_tlist, + inner_tlist)); } - else if (is_opclause((Node *) clause)) + else if (is_opclause(clause)) { - Var *leftvar = (Var *) replace_clause_joinvar_refs((Expr *) get_leftop(clause), - outer_tlist, - inner_tlist); - Var *rightvar = (Var *) replace_clause_joinvar_refs((Expr *) get_rightop(clause), - outer_tlist, - inner_tlist); + return (Node *) make_opclause( + replace_opid((Oper *) ((Expr *) clause)->oper), + (Var *) replace_clause_joinvar_refs( + (Node *) get_leftop((Expr *) clause), + outer_tlist, + inner_tlist), + (Var *) replace_clause_joinvar_refs( + (Node *) get_rightop((Expr *) clause), + outer_tlist, + inner_tlist)); + } + else if (IsA(clause, List)) + { + List *t_list = NIL; + List *subclause; - return ((List *) make_opclause(replace_opid((Oper *) ((Expr *) clause)->oper), - leftvar, - rightvar)); + foreach(subclause, (List *) clause) + { + t_list = lappend(t_list, + replace_clause_joinvar_refs(lfirst(subclause), + outer_tlist, + inner_tlist)); + } + return (Node *) t_list; } else if (is_subplan(clause)) { - ((Expr *) clause)->args = replace_subclause_joinvar_refs(((Expr *) clause)->args, - outer_tlist, - inner_tlist); - ((SubPlan *) ((Expr *) clause)->oper)->sublink->oper = - replace_subclause_joinvar_refs(((SubPlan *) ((Expr *) clause)->oper)->sublink->oper, - outer_tlist, - inner_tlist); - return (List *) clause; + /* This is a tad wasteful of space, but it works... */ + Expr *newclause = (Expr *) copyObject(clause); + newclause->args = (List *) + replace_clause_joinvar_refs((Node *) newclause->args, + outer_tlist, + inner_tlist); + ((SubPlan *) newclause->oper)->sublink->oper = (List *) + replace_clause_joinvar_refs( + (Node *) ((SubPlan *) newclause->oper)->sublink->oper, + outer_tlist, + inner_tlist); + return (Node *) newclause; } else if (IsA(clause, CaseExpr)) { - ((CaseExpr *) clause)->args = - (List *) replace_subclause_joinvar_refs(((CaseExpr *) clause)->args, - outer_tlist, - inner_tlist); + CaseExpr *oldnode = (CaseExpr *) clause; + CaseExpr *newnode = makeNode(CaseExpr); + + newnode->casetype = oldnode->casetype; + newnode->arg = oldnode->arg; /* XXX should always be null anyway ... */ + newnode->args = (List *) + replace_clause_joinvar_refs((Node *) oldnode->args, + outer_tlist, + inner_tlist); + newnode->defresult = + replace_clause_joinvar_refs(oldnode->defresult, + outer_tlist, + inner_tlist); - ((CaseExpr *) clause)->defresult = - (Node *) replace_clause_joinvar_refs((Expr *) ((CaseExpr *) clause)->defresult, - outer_tlist, - inner_tlist); - return (List *) clause; + return (Node *) newnode; } else if (IsA(clause, CaseWhen)) { - ((CaseWhen *) clause)->expr = - (Node *) replace_clause_joinvar_refs((Expr *) ((CaseWhen *) clause)->expr, - outer_tlist, - inner_tlist); - - ((CaseWhen *) clause)->result = - (Node *) replace_clause_joinvar_refs((Expr *) ((CaseWhen *) clause)->result, - outer_tlist, - inner_tlist); - return (List *) clause; - } - - /* shouldn't reach here */ - elog(ERROR, "replace_clause_joinvar_refs: unsupported clause %d", - nodeTag(clause)); - return NULL; -} + CaseWhen *oldnode = (CaseWhen *) clause; + CaseWhen *newnode = makeNode(CaseWhen); -static List * -replace_subclause_joinvar_refs(List *clauses, - List *outer_tlist, - List *inner_tlist) -{ - List *t_list = NIL; - List *temp = NIL; - List *clause = NIL; + newnode->expr = + replace_clause_joinvar_refs(oldnode->expr, + outer_tlist, + inner_tlist); + newnode->result = + replace_clause_joinvar_refs(oldnode->result, + outer_tlist, + inner_tlist); - foreach(clause, clauses) + return (Node *) newnode; + } + else { - temp = replace_clause_joinvar_refs(lfirst(clause), - outer_tlist, - inner_tlist); - t_list = lappend(t_list, temp); + elog(ERROR, "replace_clause_joinvar_refs: unsupported clause %d", + nodeTag(clause)); + return NULL; } - return t_list; } static Var * replace_joinvar_refs(Var *var, List *outer_tlist, List *inner_tlist) { - Resdom *outer_resdom = (Resdom *) NULL; + Resdom *outer_resdom; outer_resdom = tlist_member(var, outer_tlist); diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index 283ac83a49..6d25950bca 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.28 1999/02/21 03:48:55 scrappy Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/tlist.c,v 1.29 1999/05/06 23:07:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -48,7 +48,7 @@ tlistentry_member(Var *var, List *targetlist) { if (var) { - List *temp = NIL; + List *temp; foreach(temp, targetlist) { @@ -95,7 +95,7 @@ matching_tlist_var(Var *var, List *targetlist) void add_var_to_tlist(RelOptInfo *rel, Var *var) { - Expr *oldvar = (Expr *) NULL; + Expr *oldvar; oldvar = matching_tlist_var(var, rel->targetlist); @@ -132,7 +132,6 @@ add_var_to_tlist(RelOptInfo *rel, Var *var) TargetEntry * create_tl_element(Var *var, int resdomno) { - return makeTargetEntry(makeResdom(resdomno, var->vartype, var->vartypmod, @@ -190,34 +189,23 @@ get_actual_tlist(List *tlist) * * 'var' is the var node * 'tlist' is the target list - * 'dots' is t if we must match dotfields to determine uniqueness * - * Returns the resdom entry of the matching var node. + * Returns the resdom entry of the matching var node, or NULL if no match. * */ Resdom * tlist_member(Var *var, List *tlist) { - List *i = NIL; - TargetEntry *temp_tle = (TargetEntry *) NULL; - TargetEntry *tl_elt = (TargetEntry *) NULL; - if (var) { + List *i; + foreach(i, tlist) { - temp_tle = (TargetEntry *) lfirst(i); - if (var_equal(var, get_expr(temp_tle))) - { - tl_elt = temp_tle; - break; - } + TargetEntry *tle = (TargetEntry *) lfirst(i); + if (var_equal(var, get_expr(tle))) + return tle->resdom; } - - if (tl_elt != NULL) - return tl_elt->resdom; - else - return (Resdom *) NULL; } return (Resdom *) NULL; } @@ -228,14 +216,12 @@ tlist_member(Var *var, List *tlist) Resdom * tlist_resdom(List *tlist, Resdom *resnode) { - Resdom *resdom = (Resdom *) NULL; - List *i = NIL; - TargetEntry *temp_tle = (TargetEntry *) NULL; + List *i; foreach(i, tlist) { - temp_tle = (TargetEntry *) lfirst(i); - resdom = temp_tle->resdom; + TargetEntry *tle = (TargetEntry *) lfirst(i); + Resdom *resdom = tle->resdom; /* Since resnos are supposed to be unique */ if (resnode->resno == resdom->resno) return resdom; @@ -288,7 +274,6 @@ match_varid(Var *test_var, List *tlist) if (tlvar->varnoold == test_var->varnoold && tlvar->varoattno == test_var->varoattno) { - if (tlvar->vartype == type_var) return entry; } @@ -312,7 +297,7 @@ List * new_unsorted_tlist(List *targetlist) { List *new_targetlist = (List *) copyObject((Node *) targetlist); - List *x = NIL; + List *x; foreach(x, new_targetlist) { @@ -374,13 +359,10 @@ flatten_tlist(List *tlist) foreach(temp, tlist) { - TargetEntry *temp_entry = NULL; - List *vars; + TargetEntry *temp_entry = (TargetEntry *) lfirst(temp); - temp_entry = lfirst(temp); - vars = pull_var_clause((Node *) get_expr(temp_entry)); - if (vars != NULL) - tlist_vars = nconc(tlist_vars, vars); + tlist_vars = nconc(tlist_vars, + pull_var_clause((Node *) get_expr(temp_entry))); } foreach(temp, tlist_vars) @@ -421,8 +403,8 @@ flatten_tlist(List *tlist) List * flatten_tlist_vars(List *full_tlist, List *flat_tlist) { - List *x = NIL; List *result = NIL; + List *x; foreach(x, full_tlist) { @@ -450,94 +432,78 @@ flatten_tlist_vars(List *full_tlist, List *flat_tlist) static Node * flatten_tlistentry(Node *tlistentry, List *flat_tlist) { + List *temp; + if (tlistentry == NULL) - { return NULL; - } else if (IsA(tlistentry, Var)) - { - return ((Node *) get_expr(match_varid((Var *) tlistentry, - flat_tlist))); - } + return (Node *) get_expr(match_varid((Var *) tlistentry, + flat_tlist)); + else if (single_node(tlistentry)) + return tlistentry; else if (IsA(tlistentry, Iter)) { - ((Iter *) tlistentry)->iterexpr = flatten_tlistentry((Node *) ((Iter *) tlistentry)->iterexpr, + ((Iter *) tlistentry)->iterexpr = + flatten_tlistentry((Node *) ((Iter *) tlistentry)->iterexpr, flat_tlist); return tlistentry; } - else if (single_node(tlistentry)) + else if (is_subplan(tlistentry)) { + /* do we need to support this case? */ + elog(ERROR, "flatten_tlistentry: subplan case not implemented"); return tlistentry; } - else if (is_funcclause(tlistentry)) + else if (IsA(tlistentry, Expr)) { - Expr *expr = (Expr *) tlistentry; - List *temp_result = NIL; - List *elt = NIL; - - foreach(elt, expr->args) - temp_result = lappend(temp_result, - flatten_tlistentry(lfirst(elt), flat_tlist)); - - return ((Node *) make_funcclause((Func *) expr->oper, temp_result)); + /* + * Recursively scan the arguments of an expression. + * NOTE: this must come after is_subplan() case since + * subplan is a kind of Expr node. + */ + foreach(temp, ((Expr *) tlistentry)->args) + lfirst(temp) = flatten_tlistentry(lfirst(temp), flat_tlist); + return tlistentry; } else if (IsA(tlistentry, Aggref)) { + /* XXX shouldn't this be recursing into the agg's target? + * Seems to work though, so will leave it alone ... tgl 5/99 + */ return tlistentry; } else if (IsA(tlistentry, ArrayRef)) { ArrayRef *aref = (ArrayRef *) tlistentry; - List *temp = NIL; - List *elt = NIL; - - foreach(elt, aref->refupperindexpr) - temp = lappend(temp, flatten_tlistentry(lfirst(elt), flat_tlist)); - aref->refupperindexpr = temp; - - temp = NIL; - foreach(elt, aref->reflowerindexpr) - temp = lappend(temp, flatten_tlistentry(lfirst(elt), flat_tlist)); - aref->reflowerindexpr = temp; + foreach(temp, aref->refupperindexpr) + lfirst(temp) = flatten_tlistentry(lfirst(temp), flat_tlist); + foreach(temp, aref->reflowerindexpr) + lfirst(temp) = flatten_tlistentry(lfirst(temp), flat_tlist); aref->refexpr = flatten_tlistentry(aref->refexpr, flat_tlist); - - aref->refassgnexpr = flatten_tlistentry(aref->refassgnexpr, flat_tlist); + aref->refassgnexpr = flatten_tlistentry(aref->refassgnexpr,flat_tlist); return tlistentry; } else if (case_clause(tlistentry)) { CaseExpr *cexpr = (CaseExpr *) tlistentry; - List *elt = NIL; - foreach(elt, cexpr->args) + foreach(temp, cexpr->args) { - CaseWhen *cwhen = (CaseWhen *)lfirst(elt); + CaseWhen *cwhen = (CaseWhen *) lfirst(temp); + cwhen->expr = flatten_tlistentry(cwhen->expr, flat_tlist); cwhen->result = flatten_tlistentry(cwhen->result, flat_tlist); } cexpr->defresult = flatten_tlistentry(cexpr->defresult, flat_tlist); - return ((Node *) cexpr); + return tlistentry; } else { - Expr *expr, *final; - Var *left, *right; - - Assert(IsA(tlistentry, Expr)); - - expr = (Expr *) tlistentry; - left = (Var *) flatten_tlistentry((Node *) get_leftop(expr), - flat_tlist); - right = (Var *) flatten_tlistentry((Node *) get_rightop(expr), - flat_tlist); - - final = make_opclause((Oper *) expr->oper, left, right); - final->opType = expr->opType; - final->typeOid = expr->typeOid; - - return (Node *)final; + elog(ERROR, "flatten_tlistentry: Cannot handle node type %d", + nodeTag(tlistentry)); + return tlistentry; } }