Hello
2009/5/21 Tom Lane <t...@sss.pgh.pa.us>:
> Steve Prentice <prent...@cisco.com> writes:
>> This patch is intended to supplement Pavel's patch for named and mixed
>> notation support in 8.5. This patch makes it so a plpgsql function can
>> call another function with the same parameter names using the named
>> parameters notation.
>
> Well, plpgsql's parsing has always been a kluge, but I think this is
> really taking the kluge level a step too far. It's only because AS
> is used in so few contexts that this can even pretend to work --- but
> there are still an awful lot of contexts where AS is used, and will
> likely be more in the future. So I think it's pretty un-future-proof;
> and it certainly won't scale to any other contexts where we might wish
> that plpsql variables don't get substituted.
>
> It's probably time to bite the bullet and redo the parser as has been
> suggested in the past, ie fix things so that the main parser is used.
> Ideally I'd like to switch the name resolution priority to be more
> Oracle-like, but even if we don't do that it would be a great
> improvement to have actual syntactic knowledge behind the lookups.
I have fast hack, that is based on callback function from
transformExpr - it replace unknown ColumnRef node to Param node. It
works well, but I have two notes:
a) when we use main parser for identification, then valid_sql flag has
different sense. We have to valid all SQL statements - so we have to
change some logic or we have to develop levels of validations. This
have one big advantage - we are able to do complete validation.
Disadvantage - check is necessary before every first run.
b) Change priority (sql identifiers >> plpgsql identifiers) will have
two impacts:
** buggy applications will have different behave
** some an table's alters should change function's behave - so minimum
is reset plpgsql cache.
postgres=# create or replace function test(a int) returns int as $$
declare x int; y int; begin return a+20 as a; end; $$ language
plpgsql;
CREATE FUNCTION
Time: 2,485 ms
postgres=# select test(20);
test
------
40
(1 row)
Time: 1,473 ms
postgres=# create or replace function test(a int) returns int as $$
declare x int; y int; begin return a+20 as a from a; end; $$ language
plpgsql;
ERROR: relation "a" does not exist
LINE 1: SELECT a+20 as a from a
^ --< pointer is correct, look on it
some non proportional font
QUERY: SELECT a+20 as a from a
CONTEXT: SQL statement in PL/PgSQL function "test" near line 1
Attached patch is VERY UGLY, but it should to show possible direction.
I thing, so some similar should by in 8.5
??? Notes, Comments
Regards
Pavel Stehule
>
> Just for the record, you'd have to put the same kluge into the T_RECORD
> and T_ROW cases if we wanted to do it like this.
>
> regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
*** ./src/backend/parser/parse_expr.c.orig 2009-05-22 17:12:52.000000000 +0200
--- ./src/backend/parser/parse_expr.c 2009-05-22 17:44:54.000000000 +0200
***************
*** 458,466 ****
if (refnameRangeTblEntry(pstate, NULL, name1,
cref->location,
&levels_up) != NULL)
node = transformWholeRowRef(pstate, NULL, name1,
cref->location);
! else
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_COLUMN),
errmsg("column \"%s\" does not exist",
--- 458,473 ----
if (refnameRangeTblEntry(pstate, NULL, name1,
cref->location,
&levels_up) != NULL)
+ {
node = transformWholeRowRef(pstate, NULL, name1,
cref->location);
! break;
! }
!
! if (pstate->p_external_var_hook)
! node = (pstate->p_external_var_hook)(pstate, name1, cref->location);
!
! if (node == NULL)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_COLUMN),
errmsg("column \"%s\" does not exist",
*** ./src/include/parser/parse_node.h.orig 2009-05-22 17:33:37.000000000 +0200
--- ./src/include/parser/parse_node.h 2009-05-22 17:48:12.000000000 +0200
***************
*** 17,22 ****
--- 17,25 ----
#include "nodes/parsenodes.h"
#include "utils/relcache.h"
+ /* Hook for PL variable detection and substitution */
+ typedef Node *(*ParseExprExternalVariables_hook_type) (void *pstate, char *name, int location);
+
/*
* State information used during parse analysis
*
***************
*** 102,107 ****
--- 105,111 ----
bool p_is_update;
Relation p_target_relation;
RangeTblEntry *p_target_rangetblentry;
+ ParseExprExternalVariables_hook_type p_external_var_hook;
} ParseState;
/* Support for parser_errposition_callback function */
*** ./src/pl/plpgsql/src/gram.y.orig 2009-05-22 17:53:12.000000000 +0200
--- ./src/pl/plpgsql/src/gram.y 2009-05-22 21:14:03.000000000 +0200
***************
*** 18,23 ****
--- 18,25 ----
#include "catalog/pg_type.h"
#include "parser/parser.h"
+ #include "parser/parse_node.h"
+ #include "parser/parse_expr.h"
/*
***************
*** 31,36 ****
--- 33,42 ----
#define YYMALLOC palloc
#define YYFREE pfree
+ static int nparams = 0;
+ static int params[200];
+
+ static MemoryContext oldCxt;
static PLpgSQL_expr *read_sql_construct(int until,
int until2,
***************
*** 71,76 ****
--- 77,89 ----
int until, const char *expected);
static List *read_raise_options(void);
+ typedef struct {
+ int location;
+ char *token;
+ } ParserToken;
+
+ List *param_tokens;
+
%}
%expect 0
***************
*** 1953,1958 ****
--- 1966,1983 ----
sqlstart, false, true, NULL);
}
+ static List *
+ push_token(List *tokens, char *tokentxt, int location)
+ {
+
+ ParserToken *pt = palloc(sizeof(ParserToken));
+ pt->token = pstrdup(tokentxt);
+ pt->location = location;
+
+ return lappend(tokens, pt);
+ }
+
+
/*
* Read a SQL construct and build a PLpgSQL_expr for it.
*
***************
*** 1980,1994 ****
int lno;
PLpgSQL_dstring ds;
int parenlevel = 0;
- int nparams = 0;
- int params[MAX_EXPR_PARAMS];
char buf[32];
PLpgSQL_expr *expr;
lno = plpgsql_scanner_lineno();
plpgsql_dstring_init(&ds);
plpgsql_dstring_append(&ds, sqlstart);
for (;;)
{
tok = yylex();
--- 2005,2020 ----
int lno;
PLpgSQL_dstring ds;
int parenlevel = 0;
char buf[32];
PLpgSQL_expr *expr;
+ List *stacked_tokens = NIL;
lno = plpgsql_scanner_lineno();
plpgsql_dstring_init(&ds);
plpgsql_dstring_append(&ds, sqlstart);
+ stacked_tokens = push_token(stacked_tokens, sqlstart, 0);
+
for (;;)
{
tok = yylex();
***************
*** 2031,2036 ****
--- 2057,2064 ----
if (plpgsql_SpaceScanned)
plpgsql_dstring_append(&ds, " ");
+ stacked_tokens = push_token(stacked_tokens, yytext, ds.used - 1);
+ /*
switch (tok)
{
case T_SCALAR:
***************
*** 2058,2068 ****
--- 2086,2135 ----
plpgsql_dstring_append(&ds, yytext);
break;
}
+ */
+ plpgsql_dstring_append(&ds, yytext);
}
if (endtoken)
*endtoken = tok;
+ check_sql_expr(plpgsql_dstring_get(&ds));
+
+ if (param_tokens != NIL)
+ {
+ ListCell *l;
+ ListCell *l2;
+ char *token;
+ bool first = true;
+
+ /* generate parametrized query string */
+ plpgsql_dstring_init(&ds);
+
+ foreach(l, stacked_tokens)
+ {
+ ParserToken *pt = (ParserToken *) lfirst(l);
+ token = pt->token;
+
+ foreach(l2, param_tokens)
+ {
+ ParserToken *pt2 = (ParserToken *) lfirst(l2);
+
+ if (pt2->location == pt->location)
+ {
+ token = pt2->token;
+ break;
+ }
+ }
+
+ if (!first)
+ plpgsql_dstring_append_char(&ds, ' ');
+ else
+ first = false;
+
+ plpgsql_dstring_append(&ds, token);
+ }
+ }
+
expr = palloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - sizeof(int));
expr->dtype = PLPGSQL_DTYPE_EXPR;
expr->query = pstrdup(plpgsql_dstring_get(&ds));
***************
*** 2072,2079 ****
expr->params[nparams] = params[nparams];
plpgsql_dstring_free(&ds);
- if (valid_sql)
- check_sql_expr(expr->query);
return expr;
}
--- 2139,2144 ----
***************
*** 2689,2694 ****
--- 2754,2797 ----
return row;
}
+ Node *local_variables_finder(ParseState *pstate, char *name, int location)
+ {
+ PLpgSQL_nsitem *var;
+ Node *result = NULL;
+ MemoryContext cxt;
+
+ var = plpgsql_ns_lookup(name, NULL, NULL, NULL);
+ if (var != NULL)
+ {
+ PLpgSQL_type *dtype = plpgsql_get_dtype(var->itemno);
+ Param *p = makeNode(Param);
+ ParserToken *pt = palloc(sizeof(ParserToken));
+ char buf[32];
+
+ p->paramid = assign_expr_param(var->itemno,
+ params, &nparams);
+
+ p->paramtype = dtype->typoid;
+ p->paramtypmod = dtype->atttypmod;
+ p->location = location;
+
+ cxt = MemoryContextSwitchTo(oldCxt);
+
+ snprintf(buf, sizeof(buf), "$%d ", p->paramid);
+
+ pt->location = location;
+ pt->token = pstrdup(buf);
+
+ param_tokens = lappend(param_tokens, pt);
+
+ MemoryContextSwitchTo(cxt);
+
+ return (Node *) p;
+
+ }
+ return result;
+ }
+
/*
* When the PL/PgSQL parser expects to see a SQL statement, it is very
* liberal in what it accepts; for example, we often assume an
***************
*** 2711,2717 ****
{
ErrorContextCallback syntax_errcontext;
ErrorContextCallback *previous_errcontext;
! MemoryContext oldCxt;
if (!plpgsql_check_syntax)
return;
--- 2814,2831 ----
{
ErrorContextCallback syntax_errcontext;
ErrorContextCallback *previous_errcontext;
!
! ParseState *pstate = make_parsestate(NULL);
! pstate->p_external_var_hook = local_variables_finder;
! ListCell *l;
! List *list;
!
! pstate->p_sourcetext = stmt;
! pstate->p_paramtypes = NULL;
! pstate->p_variableparams = true;
!
!
!
if (!plpgsql_check_syntax)
return;
***************
*** 2732,2738 ****
error_context_stack = &syntax_errcontext;
oldCxt = MemoryContextSwitchTo(compile_tmp_cxt);
! (void) raw_parser(stmt);
MemoryContextSwitchTo(oldCxt);
/* Restore former ereport callback */
--- 2846,2862 ----
error_context_stack = &syntax_errcontext;
oldCxt = MemoryContextSwitchTo(compile_tmp_cxt);
!
! list = raw_parser(stmt);
!
! nparams = 0;
! param_tokens = NIL;
!
! foreach(l, list)
! {
! transformStmt(pstate, lfirst(l));
! }
!
MemoryContextSwitchTo(oldCxt);
/* Restore former ereport callback */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
