From d5096af2c42a78b2fccf24057a1eacb892399b40 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 18 Apr 2001 20:42:56 +0000 Subject: [PATCH] Make the world safe for passing whole rows of views to functions. This already worked fine for whole rows of tables, but not so well for views... --- src/backend/optimizer/plan/planner.c | 8 ++- src/backend/optimizer/util/var.c | 121 +++++++++++++++++++++++++++++++---- src/backend/rewrite/rewriteManip.c | 11 +++- src/include/optimizer/var.h | 9 +-- src/pl/plpgsql/src/gram.y | 15 ++--- src/pl/plpgsql/src/pl_comp.c | 33 ++++++---- src/pl/plpgsql/src/scan.l | 8 ++- 7 files changed, 161 insertions(+), 44 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 199d27973b..3026fdf058 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.103 2001/04/01 22:37:19 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.104 2001/04/18 20:42:55 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -271,8 +271,12 @@ pull_up_subqueries(Query *parse, Node *jtnode) /* * Is this a subquery RTE, and if so, is the subquery simple * enough to pull up? (If not, do nothing at this node.) + * + * Note: even if the subquery itself is simple enough, we can't + * pull it up if there is a reference to its whole tuple result. */ - if (subquery && is_simple_subquery(subquery)) + if (subquery && is_simple_subquery(subquery) && + !contain_whole_tuple_var((Node *) parse, varno, 0)) { int rtoffset; Node *subjointree; diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c index cac0eee827..30f02de5c7 100644 --- a/src/backend/optimizer/util/var.c +++ b/src/backend/optimizer/util/var.c @@ -8,12 +8,10 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.30 2001/03/22 03:59:40 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/var.c,v 1.31 2001/04/18 20:42:55 tgl Exp $ * *------------------------------------------------------------------------- */ -#include - #include "postgres.h" #include "nodes/plannodes.h" @@ -29,12 +27,20 @@ typedef struct typedef struct { + int varno; + int sublevels_up; +} contain_whole_tuple_var_context; + +typedef struct +{ List *varlist; bool includeUpperVars; } pull_var_clause_context; static bool pull_varnos_walker(Node *node, pull_varnos_context *context); +static bool contain_whole_tuple_var_walker(Node *node, + contain_whole_tuple_var_context *context); static bool contain_var_clause_walker(Node *node, void *context); static bool pull_var_clause_walker(Node *node, pull_var_clause_context *context); @@ -46,11 +52,10 @@ static bool pull_var_clause_walker(Node *node, * Create a list of all the distinct varnos present in a parsetree. * Only varnos that reference level-zero rtable entries are considered. * - * NOTE: unlike other routines in this file, pull_varnos() is used on - * not-yet-planned expressions. It may therefore find bare SubLinks, - * and if so it needs to recurse into them to look for uplevel references - * to the desired rtable level! But when we find a completed SubPlan, - * we only need to look at the parameters passed to the subplan. + * NOTE: this is used on not-yet-planned expressions. It may therefore find + * bare SubLinks, and if so it needs to recurse into them to look for uplevel + * references to the desired rtable level! But when we find a completed + * SubPlan, we only need to look at the parameters passed to the subplan. */ List * pull_varnos(Node *node) @@ -122,17 +127,105 @@ pull_varnos_walker(Node *node, pull_varnos_context *context) (void *) context); } + +/* + * contain_whole_tuple_var + * + * Detect whether a parsetree contains any references to the whole + * tuple of a given rtable entry (ie, a Var with varattno = 0). + * + * NOTE: this is used on not-yet-planned expressions. It may therefore find + * bare SubLinks, and if so it needs to recurse into them to look for uplevel + * references to the desired rtable entry! But when we find a completed + * SubPlan, we only need to look at the parameters passed to the subplan. + */ +bool +contain_whole_tuple_var(Node *node, int varno, int levelsup) +{ + contain_whole_tuple_var_context context; + + context.varno = varno; + context.sublevels_up = levelsup; + + /* + * Must be prepared to start with a Query or a bare expression tree; + * if it's a Query, go straight to query_tree_walker to make sure that + * sublevels_up doesn't get incremented prematurely. + */ + if (node && IsA(node, Query)) + return query_tree_walker((Query *) node, + contain_whole_tuple_var_walker, + (void *) &context, true); + else + return contain_whole_tuple_var_walker(node, &context); +} + +static bool +contain_whole_tuple_var_walker(Node *node, + contain_whole_tuple_var_context *context) +{ + if (node == NULL) + return false; + if (IsA(node, Var)) + { + Var *var = (Var *) node; + + if (var->varno == context->varno && + var->varlevelsup == context->sublevels_up && + var->varattno == InvalidAttrNumber) + return true; + return false; + } + if (is_subplan(node)) + { + + /* + * Already-planned subquery. Examine the args list (parameters to + * be passed to subquery), as well as the "oper" list which is + * executed by the outer query. But short-circuit recursion into + * the subquery itself, which would be a waste of effort. + */ + Expr *expr = (Expr *) node; + + if (contain_whole_tuple_var_walker((Node *) ((SubPlan *) expr->oper)->sublink->oper, + context)) + return true; + if (contain_whole_tuple_var_walker((Node *) expr->args, + context)) + return true; + return false; + } + if (IsA(node, Query)) + { + /* Recurse into RTE subquery or not-yet-planned sublink subquery */ + bool result; + + context->sublevels_up++; + result = query_tree_walker((Query *) node, + contain_whole_tuple_var_walker, + (void *) context, true); + context->sublevels_up--; + return result; + } + return expression_tree_walker(node, contain_whole_tuple_var_walker, + (void *) context); +} + + /* * contain_var_clause * Recursively scan a clause to discover whether it contains any Var nodes * (of the current query level). * * Returns true if any varnode found. + * + * Does not examine subqueries, therefore must only be used after reduction + * of sublinks to subplans! */ bool -contain_var_clause(Node *clause) +contain_var_clause(Node *node) { - return contain_var_clause_walker(clause, NULL); + return contain_var_clause_walker(node, NULL); } static bool @@ -150,6 +243,7 @@ contain_var_clause_walker(Node *node, void *context) return expression_tree_walker(node, contain_var_clause_walker, context); } + /* * pull_var_clause * Recursively pulls all var nodes from an expression clause. @@ -160,16 +254,19 @@ contain_var_clause_walker(Node *node, void *context) * * Returns list of varnodes found. Note the varnodes themselves are not * copied, only referenced. + * + * Does not examine subqueries, therefore must only be used after reduction + * of sublinks to subplans! */ List * -pull_var_clause(Node *clause, bool includeUpperVars) +pull_var_clause(Node *node, bool includeUpperVars) { pull_var_clause_context context; context.varlist = NIL; context.includeUpperVars = includeUpperVars; - pull_var_clause_walker(clause, &context); + pull_var_clause_walker(node, &context); return context.varlist; } diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 663b67708e..45115b8d04 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.56 2001/03/22 03:59:44 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.57 2001/04/18 20:42:55 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -765,8 +765,13 @@ ResolveNew_mutator(Node *node, ResolveNew_context *context) if (this_varno == context->target_varno && this_varlevelsup == context->sublevels_up) { - Node *n = FindMatchingNew(context->targetlist, - var->varattno); + Node *n; + + /* band-aid: don't do the wrong thing with a whole-tuple Var */ + if (var->varattno == InvalidAttrNumber) + elog(ERROR, "ResolveNew: can't handle whole-tuple reference"); + + n = FindMatchingNew(context->targetlist, var->varattno); if (n == NULL) { diff --git a/src/include/optimizer/var.h b/src/include/optimizer/var.h index d0840596f1..45048133eb 100644 --- a/src/include/optimizer/var.h +++ b/src/include/optimizer/var.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: var.h,v 1.12 2001/01/24 19:43:26 momjian Exp $ + * $Id: var.h,v 1.13 2001/04/18 20:42:55 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -16,8 +16,9 @@ #include "nodes/primnodes.h" -extern List *pull_varnos(Node *me); -extern bool contain_var_clause(Node *clause); -extern List *pull_var_clause(Node *clause, bool includeUpperVars); +extern List *pull_varnos(Node *node); +extern bool contain_whole_tuple_var(Node *node, int varno, int levelsup); +extern bool contain_var_clause(Node *node); +extern List *pull_var_clause(Node *node, bool includeUpperVars); #endif /* VAR_H */ diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y index 29e2dd24bb..1bfce18473 100644 --- a/src/pl/plpgsql/src/gram.y +++ b/src/pl/plpgsql/src/gram.y @@ -4,7 +4,7 @@ * procedural language * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/gram.y,v 1.16 2001/02/19 19:49:53 tgl Exp $ + * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/gram.y,v 1.17 2001/04/18 20:42:56 tgl Exp $ * * This software is copyrighted by Jan Wieck - Hamburg. * @@ -885,7 +885,7 @@ fori_lower : } } - expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - 1); + expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - sizeof(int)); expr->dtype = PLPGSQL_DTYPE_EXPR; expr->query = strdup(plpgsql_dstring_get(&ds)); expr->plan = NULL; @@ -1272,7 +1272,7 @@ read_sqlstmt (int until, char *s, char *sqlstart) } } - expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - 1); + expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - sizeof(int)); expr->dtype = PLPGSQL_DTYPE_EXPR; expr->query = strdup(plpgsql_dstring_get(&ds)); expr->plan = NULL; @@ -1310,7 +1310,7 @@ make_select_stmt() { PLpgSQL_stmt_execsql *execsql; - expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - 1); + expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - sizeof(int)); expr->dtype = PLPGSQL_DTYPE_EXPR; expr->query = strdup(plpgsql_dstring_get(&ds)); expr->plan = NULL; @@ -1449,14 +1449,13 @@ make_select_stmt() { PLpgSQL_stmt_execsql *execsql; - expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - 1); + expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - sizeof(int)); expr->dtype = PLPGSQL_DTYPE_EXPR; expr->query = strdup(plpgsql_dstring_get(&ds)); expr->plan = NULL; expr->nparams = nparams; - while(nparams-- > 0) { + while (nparams-- > 0) expr->params[nparams] = params[nparams]; - } plpgsql_dstring_free(&ds); execsql = malloc(sizeof(PLpgSQL_stmt_execsql)); @@ -1549,7 +1548,7 @@ make_select_stmt() } } - expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * (nparams - 1)); + expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - sizeof(int)); expr->dtype = PLPGSQL_DTYPE_EXPR; expr->query = strdup(plpgsql_dstring_get(&ds)); expr->plan = NULL; diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 22bb9e03a8..9cfa748241 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -3,7 +3,7 @@ * procedural language * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.29 2001/04/06 02:06:48 tgl Exp $ + * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.30 2001/04/18 20:42:56 tgl Exp $ * * This software is copyrighted by Jan Wieck - Hamburg. * @@ -552,7 +552,7 @@ plpgsql_parse_word(char *word) */ if (plpgsql_curr_compile->fn_functype == T_TRIGGER) { - if (!strcmp(cp, "tg_argv")) + if (strcmp(cp, "tg_argv") == 0) { int save_spacescanned = plpgsql_SpaceScanned; PLpgSQL_trigarg *trigarg; @@ -751,7 +751,7 @@ plpgsql_parse_dblword(char *string) row = (PLpgSQL_row *) (plpgsql_Datums[ns->itemno]); for (i = 0; i < row->nfields; i++) { - if (!strcmp(row->fieldnames[i], word2)) + if (strcmp(row->fieldnames[i], word2) == 0) { plpgsql_yylval.var = (PLpgSQL_var *) (plpgsql_Datums[row->varnos[i]]); pfree(word1); @@ -855,7 +855,7 @@ plpgsql_parse_tripword(char *string) row = (PLpgSQL_row *) (plpgsql_Datums[ns->itemno]); for (i = 0; i < row->nfields; i++) { - if (!strcmp(row->fieldnames[i], word3)) + if (strcmp(row->fieldnames[i], word3) == 0) { plpgsql_yylval.var = (PLpgSQL_var *) (plpgsql_Datums[row->varnos[i]]); pfree(word1); @@ -1139,14 +1139,17 @@ plpgsql_parse_wordrowtype(char *string) elog(ERROR, "%s: no such class", word1); } classStruct = (Form_pg_class) GETSTRUCT(classtup); - if (classStruct->relkind != 'r' && classStruct->relkind != 's') + /* accept relation, sequence, or view pg_class entries */ + if (classStruct->relkind != 'r' && + classStruct->relkind != 's' && + classStruct->relkind != 'v') { plpgsql_comperrinfo(); elog(ERROR, "%s isn't a table", word1); } /* - * Fetch the tables pg_type tuple too + * Fetch the table's pg_type tuple too */ typetup = SearchSysCache(TYPENAME, PointerGetDatum(word1), @@ -1205,15 +1208,17 @@ plpgsql_parse_wordrowtype(char *string) typeStruct = (Form_pg_type) GETSTRUCT(typetup); /* - * Create the internal variable We know if the table definitions - * contain a default value or if the field is declared in the - * table as NOT NULL. But it's possible to create a table field as - * NOT NULL without a default value and that would lead to - * problems later when initializing the variables due to entering - * a block at execution time. Thus we ignore this information for - * now. + * Create the internal variable + * + * We know if the table definitions contain a default value or if the + * field is declared in the table as NOT NULL. But it's possible to + * create a table field as NOT NULL without a default value and that + * would lead to problems later when initializing the variables due to + * entering a block at execution time. Thus we ignore this information + * for now. */ var = malloc(sizeof(PLpgSQL_var)); + memset(var, 0, sizeof(PLpgSQL_var)); var->dtype = PLPGSQL_DTYPE_VAR; var->refname = malloc(strlen(word1) + strlen(cp) + 2); strcpy(var->refname, word1); @@ -1241,7 +1246,7 @@ plpgsql_parse_wordrowtype(char *string) /* * Add the variable to the row. */ - row->fieldnames[i] = cp; + row->fieldnames[i] = strdup(cp); row->varnos[i] = var->varno; } diff --git a/src/pl/plpgsql/src/scan.l b/src/pl/plpgsql/src/scan.l index 5872a4eddb..ecde59df0c 100644 --- a/src/pl/plpgsql/src/scan.l +++ b/src/pl/plpgsql/src/scan.l @@ -4,7 +4,7 @@ * procedural language * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/Attic/scan.l,v 1.9 2001/02/19 19:49:53 tgl Exp $ + * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/Attic/scan.l,v 1.10 2001/04/18 20:42:56 tgl Exp $ * * This software is copyrighted by Jan Wieck - Hamburg. * @@ -145,6 +145,12 @@ dump { return O_DUMP; } {WS}{WC}*%ROWTYPE { return plpgsql_parse_wordrowtype(yytext); } \$[0-9]+ { return plpgsql_parse_word(yytext); } +\$[0-9]+\.{WS}{WC}* { return plpgsql_parse_dblword(yytext); } +\$[0-9]+\.{WS}{WC}*\.{WS}{WC}* { return plpgsql_parse_tripword(yytext); } +\$[0-9]+%TYPE { return plpgsql_parse_wordtype(yytext); } +\$[0-9]+\.{WS}{WC}*%TYPE { return plpgsql_parse_dblwordtype(yytext); } +\$[0-9]+%ROWTYPE { return plpgsql_parse_wordrowtype(yytext); } + [0-9]+ { return T_NUMBER; } /* ---------- -- 2.11.0