[PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v2
Hello,
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. Without this patch, the following example will
have a syntax errors:
CREATE FUNCTION fun1(a INT DEFAULT 1) RETURNS INT AS 'SELECT $1'
LANGUAGE SQL;
CREATE FUNCTION fun2(a INT) RETURNS INT AS $$
DECLARE
t INT;
BEGIN
t := fun1(1 as a); -- syntax error: "SELECT fun1(1 as $1 )"
t := fun1(a as a); -- syntax error: "SELECT fun1( $1 as $1 )"
RETURN 0;
END;
$$ LANGUAGE plpgsql;
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.
(v1 of this patch was in the "plpgsql + named parameters" thread, but
it didn't include the doc changes.)
-Steve
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 80dbf45..9b99314 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3510,7 +3510,7 @@ PREPARE <replaceable>statement_name</>(text,
timestamp) AS
</para> <para>
- <emphasis>The substitution mechanism will replace any token that
matches a
+ <emphasis>The substitution mechanism will replace most tokens
that match a
known variable's name.</> This poses various traps for the unwary.
For example, it is a bad idea
to use a variable name that is the same as any table or column name
@@ -3601,9 +3601,29 @@ CONTEXT: SQL statement in PL/PgSQL function
"logfunc2" near line 5
interpreted the <quote>wrong</> way. But it is useful for
clarifying
the intent of potentially-ambiguous code.
</para>
-
+
+ <para>
+ There are two places where variable substitution does not happen.
+ </para>
+
+ <para>
+ Any label following the "AS" keyword is not replace. This allows
passing
+ parameters by name to functions that have parameters of the same
name as
+ the calling function. For example,
+<programlisting>
+ CREATE FUNCTION logfunc(v_logtxt text) RETURNS void AS $$
+ BEGIN
+ INSERT INTO logtable (logtxt) VALUES (v_logtxt);
+ PERFORM tracefunc(v_logtxt AS v_logtxt);
+ END;
+ $$ LANGUAGE plpgsql;
+</programlisting>
+ All occurances of v_logtxt in the function are replaced except the
one
+ following "AS".
+ </para>
+
<para>
- Variable substitution does not happen in the command string given
+ Variable substitution also does not happen in the command string
given
to <command>EXECUTE</> or one of its variants. If you need to
insert a varying value into such a command, do so as part of
constructing the string value, as illustrated in
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 06704cf..647daab 100644
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -177,6 +177,7 @@ static List *read_raise_options(void);
* Keyword tokens
*/
%token K_ALIAS
+%token K_AS
%token K_ASSIGN
%token K_BEGIN
%token K_BY
@@ -1977,6 +1978,7 @@ read_sql_construct(int until,
int *endtoken)
{
int tok;
+ int prevtok = 0;
int lno;
PLpgSQL_dstring ds;
int parenlevel = 0;
@@ -1989,7 +1991,7 @@ read_sql_construct(int until,
plpgsql_dstring_init(&ds);
plpgsql_dstring_append(&ds, sqlstart);- for (;;)
+ for (;;prevtok = tok)
{
tok = yylex();
if (tok == until && parenlevel == 0)
@@ -2034,10 +2036,16 @@ read_sql_construct(int until,
switch (tok)
{
case T_SCALAR:
- snprintf(buf, sizeof(buf), " $%d ",
- assign_expr_param(yylval.scalar->dno,
- params, &nparams));
- plpgsql_dstring_append(&ds, buf);
+ /* A scalar following AS is treated as a label */
+ if (prevtok == K_AS)
+ plpgsql_dstring_append(&ds, yytext);
+ else
+ {
+ snprintf(buf, sizeof(buf), " $%d ",
+ assign_expr_param(yylval.scalar->dno,
+ params, &nparams));
+ plpgsql_dstring_append(&ds, buf);
+ }
break; case T_ROW:
diff --git a/src/pl/plpgsql/src/scan.l b/src/pl/plpgsql/src/scan.l
index 1917eef..e3a5c45 100644
--- a/src/pl/plpgsql/src/scan.l
+++ b/src/pl/plpgsql/src/scan.l
@@ -149,6 +149,7 @@ param \${digit}+
= { return K_ASSIGN; }
\.\. { return K_DOTDOT; }
alias { return K_ALIAS; }
+as { return K_AS; }
begin { return K_BEGIN; }
by { return K_BY; }
case { return K_CASE; }
Steve Prentice <prentice@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.
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
On May 21, 2009, at 10:52 AM, Tom Lane wrote:
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.
That kind of refactoring is beyond my experience-level with the code,
but I can't say I disagree with your analysis.
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.
Patch updated.
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 80dbf45..f8e8ce4 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3510,7 +3510,7 @@ PREPARE <replaceable>statement_name</>(text,
timestamp) AS
</para> <para>
- <emphasis>The substitution mechanism will replace any token that
matches a
+ <emphasis>The substitution mechanism will replace most tokens
that match a
known variable's name.</> This poses various traps for the
unwary.
For example, it is a bad idea
to use a variable name that is the same as any table or column
name
@@ -3603,7 +3603,27 @@ CONTEXT: SQL statement in PL/PgSQL function
"logfunc2" near line 5
</para> <para>
- Variable substitution does not happen in the command string given
+ There are two places where variable substitution does not happen.
+ </para>
+
+ <para>
+ Any label following the "AS" keyword is not replaced. This allows
passing
+ parameters by name to functions that have parameters of the same
name as
+ the calling function. For example,
+<programlisting>
+ CREATE FUNCTION logfunc(v_logtxt text) RETURNS void AS $$
+ BEGIN
+ INSERT INTO logtable (logtxt) VALUES (v_logtxt);
+ PERFORM tracefunc(v_logtxt AS v_logtxt);
+ END;
+ $$ LANGUAGE plpgsql;
+</programlisting>
+ All occurances of v_logtxt in the function are replaced except the
one
+ following "AS".
+ </para>
+
+ <para>
+ Variable substitution also does not happen in the command string
given
to <command>EXECUTE</> or one of its variants. If you need to
insert a varying value into such a command, do so as part of
constructing the string value, as illustrated in
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 06704cf..3b4e9b8 100644
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -177,6 +177,7 @@ static List
*read_raise_options(void);
* Keyword tokens
*/
%token K_ALIAS
+%token K_AS
%token K_ASSIGN
%token K_BEGIN
%token K_BY
@@ -1977,6 +1978,7 @@ read_sql_construct(int until,
int *endtoken)
{
int tok;
+ int prevtok = 0;
int lno;
PLpgSQL_dstring ds;
int parenlevel = 0;
@@ -1989,7 +1991,7 @@ read_sql_construct(int until,
plpgsql_dstring_init(&ds);
plpgsql_dstring_append(&ds, sqlstart);- for (;;)
+ for (;;prevtok = tok)
{
tok = yylex();
if (tok == until && parenlevel == 0)
@@ -2031,6 +2033,16 @@ read_sql_construct(int until,
if (plpgsql_SpaceScanned)
plpgsql_dstring_append(&ds, " ");
+ /* A variable following AS is treated as a label */
+ if (prevtok == K_AS &&
+ (tok == T_SCALAR || tok == T_ROW ||
tok == T_RECORD))
+ {
+ plpgsql_dstring_append(&ds, yytext);
+ continue;
+ }
+
switch (tok)
{
case T_SCALAR:
diff --git a/src/pl/plpgsql/src/scan.l b/src/pl/plpgsql/src/scan.l
index 1917eef..e3a5c45 100644
--- a/src/pl/plpgsql/src/scan.l
+++ b/src/pl/plpgsql/src/scan.l
@@ -149,6 +149,7 @@ param \${digit}+
= { return
K_ASSIGN; }
\.\. { return K_DOTDOT; }
alias { return K_ALIAS; }
+as { return
K_AS; }
begin { return K_BEGIN; }
by { return
K_BY; }
case { return K_CASE; }
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.
Would it be possible to also support "=" as well as "as"? I believe
that SQL Server uses "=" exclusively, and supporting that syntax would
help people port TSQL-based applications.
--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com
2009/5/21 Josh Berkus <josh@agliodbs.com>:
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.Would it be possible to also support "=" as well as "as"? I believe that
SQL Server uses "=" exclusively, and supporting that syntax would help
people port TSQL-based applications.
No, it not possible, or not simply . This is ambiguous. a = 10 should
be normal boolean expression. I didn't use T-SQL many years, but I
thing, so in T-SQL this problem is solved with variables prefixes @.
regards
Pavel Stehule
Show quoted text
--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2009/5/21 Pavel Stehule <pavel.stehule@gmail.com>:
2009/5/21 Josh Berkus <josh@agliodbs.com>:
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.Would it be possible to also support "=" as well as "as"? I believe that
SQL Server uses "=" exclusively, and supporting that syntax would help
people port TSQL-based applications.No, it not possible, or not simply . This is ambiguous. a = 10 should
be normal boolean expression. I didn't use T-SQL many years, but I
thing, so in T-SQL this problem is solved with variables prefixes @.
there are not technical problem - there are semantic problem
Pavel
Show quoted text
regards
Pavel Stehule--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
"=" is more common than "as" for sure, but this has been talked about
before starting at about this message:
http://archives.postgresql.org/message-id/14319.1228833321@sss.pgh.pa.us
-Steve
On May 21, 2009, at 11:51 AM, Josh Berkus wrote:
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.
Would it be possible to also support "=" as well as "as"? I believe
that SQL Server uses "=" exclusively, and supporting that syntax would
help people port TSQL-based applications.
--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com
Josh Berkus wrote:
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.Would it be possible to also support "=" as well as "as"? I believe
that SQL Server uses "=" exclusively, and supporting that syntax would
help people port TSQL-based applications.
Would this be best served by implementing PL/TSQL?
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On 5/21/09 6:44 PM, Alvaro Herrera wrote:
Josh Berkus wrote:
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.Would it be possible to also support "=" as well as "as"? I believe
that SQL Server uses "=" exclusively, and supporting that syntax would
help people port TSQL-based applications.Would this be best served by implementing PL/TSQL?
Passing variables takes place *outside* of the PL.
--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com
Josh Berkus wrote:
On 5/21/09 6:44 PM, Alvaro Herrera wrote:
Josh Berkus wrote:
The patch adds the "AS" keyword to the plpgsql grammar and doesn't
assign an expression parameter to the sql construct if the scalar
follows the AS keyword.Would it be possible to also support "=" as well as "as"? I believe
that SQL Server uses "=" exclusively, and supporting that syntax would
help people port TSQL-based applications.Would this be best served by implementing PL/TSQL?
Passing variables takes place *outside* of the PL.
Hmm, sure, but I was actually thinking on a higher level ... I mean I've always
thought that there's so much effort directed at porting procedures from TSQL to
plpgsql, but it would be better to just write a dedicated PL. (Of course,
the set of people who can actually write such a PL is a lot smaller than those
who can write plpgsql).
I mean if all you had to do was change = to "as" to make your TSQL procedures
supported, you'd be a lot happier than if that was the only work you could
_save_.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Hello
2009/5/21 Tom Lane <tgl@sss.pgh.pa.us>:
Steve Prentice <prentice@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
Show quoted text
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
Attachments:
mainparser.difftext/x-patch; charset=US-ASCII; name=mainparser.diffDownload
*** ./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 */
--On Donnerstag, Mai 21, 2009 11:46:24 -0700 Steve Prentice
<prentice@cisco.com> wrote:
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.Patch updated.
Steve,
it seems there's something broken, patch complains about a broken format.
Can you please provide a new diff file?
Thanks
Bernd
On Jul 17, 2009, at 11:56 AM, Bernd Helmle wrote:
it seems there's something broken, patch complains about a broken
format. Can you please provide a new diff file?
Sorry about that--probably got messed up as I pasted it into the
message. I've attached the patch this time.
Attachments:
plpgsql_keyword_as.patchapplication/octet-stream; name=plpgsql_keyword_as.patch; x-unix-mode=0644Download
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 07078e6..ce1676b 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3510,7 +3510,7 @@ PREPARE <replaceable>statement_name</>(text, timestamp) AS
</para>
<para>
- <emphasis>The substitution mechanism will replace any token that matches a
+ <emphasis>The substitution mechanism will replace most tokens that match a
known variable's name.</> This poses various traps for the unwary.
For example, it is a bad idea
to use a variable name that is the same as any table or column name
@@ -3603,7 +3603,27 @@ CONTEXT: SQL statement in PL/PgSQL function "logfunc2" near line 5
</para>
<para>
- Variable substitution does not happen in the command string given
+ There are two places where variable substitution does not happen.
+ </para>
+
+ <para>
+ Any label following the "AS" keyword is not replaced. This allows passing
+ parameters by name to functions that have parameters of the same name as
+ the calling function. For example,
+<programlisting>
+ CREATE FUNCTION logfunc(v_logtxt text) RETURNS void AS $$
+ BEGIN
+ INSERT INTO logtable (logtxt) VALUES (v_logtxt);
+ PERFORM tracefunc(v_logtxt AS v_logtxt);
+ END;
+ $$ LANGUAGE plpgsql;
+</programlisting>
+ All occurances of v_logtxt in the function are replaced except the one
+ following "AS".
+ </para>
+
+ <para>
+ Variable substitution also does not happen in the command string given
to <command>EXECUTE</> or one of its variants. If you need to
insert a varying value into such a command, do so as part of
constructing the string value, as illustrated in
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 5572cf7..fe432d7 100644
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -177,6 +177,7 @@ static List *read_raise_options(void);
* Keyword tokens
*/
%token K_ALIAS
+%token K_AS
%token K_ASSIGN
%token K_BEGIN
%token K_BY
@@ -1977,6 +1978,7 @@ read_sql_construct(int until,
int *endtoken)
{
int tok;
+ int prevtok = 0;
int lno;
PLpgSQL_dstring ds;
int parenlevel = 0;
@@ -1989,7 +1991,7 @@ read_sql_construct(int until,
plpgsql_dstring_init(&ds);
plpgsql_dstring_append(&ds, sqlstart);
- for (;;)
+ for (;;prevtok = tok)
{
tok = yylex();
if (tok == until && parenlevel == 0)
@@ -2031,6 +2033,14 @@ read_sql_construct(int until,
if (plpgsql_SpaceScanned)
plpgsql_dstring_append(&ds, " ");
+ /* A variable following AS is treated as a label */
+ if (prevtok == K_AS &&
+ (tok == T_SCALAR || tok == T_ROW || tok == T_RECORD))
+ {
+ plpgsql_dstring_append(&ds, yytext);
+ continue;
+ }
+
switch (tok)
{
case T_SCALAR:
diff --git a/src/pl/plpgsql/src/scan.l b/src/pl/plpgsql/src/scan.l
index 5d7f497..d67bd3d 100644
--- a/src/pl/plpgsql/src/scan.l
+++ b/src/pl/plpgsql/src/scan.l
@@ -147,6 +147,7 @@ param \${digit}+
= { return K_ASSIGN; }
\.\. { return K_DOTDOT; }
alias { return K_ALIAS; }
+as { return K_AS; }
begin { return K_BEGIN; }
by { return K_BY; }
case { return K_CASE; }
On Thu, May 21, 2009 at 2:46 PM, Steve Prentice <prentice@cisco.com> wrote:
On May 21, 2009, at 10:52 AM, Tom Lane wrote:
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.That kind of refactoring is beyond my experience-level with the code, but I
can't say I disagree with your analysis.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.Patch updated.
I played around a bit with the latest version of this patch tonight,
but I'm replying to this previous version for the sake of being able
to quote more of the relevant discussion.
First, I applied this patch, which resulted in a successful compile,
but PL/pgsql wouldn't load. After scratching my head for a minute, I
recalled that this was supposed to be dependent on named and mixed
notation, so I applied both patches, which resulted in a failed
compile. Further experimentation revealed that named and mixed
notation alone also lead to a failed compile. I replied to the
named/mixed notation thread so hopefully Pavel will fix whatever the
problem is with that patch.
However... even assuming I can get this to work at all, it seems like
it's only going to help in a pretty limited range of cases. Since
this is just looking for occurrences of "AS", it has a chance of
working (of course I can't test at the moment) for something like
this:
select foo as bar from generate_series(1,10) foo;
...but I think it will certainly fail for something like this:
select foo bar from generate_series(1,10) foo;
As much as I'm annoyed by the stupidity of PL/pgsql in this regard
(and I really am - I use it constantly and this is a real pain in the
neck), I think it makes more sense to wait for a more comprehensive
solution. Also, besides the fact that this doesn't (and can't) handle
all cases, as Tom points out, this would create a real possibility
that some future use of the word AS could cause breakage at a
distance.
So, I guess I'm sadly left feeling that we should probably reject this
patch. Anyone want to argue otherwise?
...Robert
Robert Haas <robertmhaas@gmail.com> writes:
So, I guess I'm sadly left feeling that we should probably reject this
patch. Anyone want to argue otherwise?
+1. I'm really hoping to get something done about the plpgsql parsing
situation before 8.5 is out, so this should be a dead end anyway.
regards, tom lane
2009/9/14 Tom Lane <tgl@sss.pgh.pa.us>:
Robert Haas <robertmhaas@gmail.com> writes:
So, I guess I'm sadly left feeling that we should probably reject this
patch. Anyone want to argue otherwise?+1. I'm really hoping to get something done about the plpgsql parsing
situation before 8.5 is out, so this should be a dead end anyway.
I have a WIP patch for integration main SQL parser to plpgsql. I'll
send it to this weekend.
regards
Pavel Stehule
Show quoted text
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
On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote:
2009/9/14 Tom Lane <tgl@sss.pgh.pa.us>:
Robert Haas <robertmhaas@gmail.com> writes:
So, I guess I'm sadly left feeling that we should probably reject
this
patch. Anyone want to argue otherwise?+1. I'm really hoping to get something done about the plpgsql
parsing
situation before 8.5 is out, so this should be a dead end anyway.I have a WIP patch for integration main SQL parser to plpgsql. I'll
send it to this weekend.
I certainly don't mind the patch getting rejected and agree that
refactoring the plpgsql parser is probably the best approach to this
issue. However, I think it would be more than a little strange to ship
the named notation feature without a solution for this problem. For
reference, the problem is that the function below causes a compile
error because of the way plpgsql blindly does variable replacement:
create function fun1(pDisplayName text) returns void as $$
begin
perform fun2(pDisplayName as pDisplayName);
-- Above line compiles as:
-- SELECT fun2( $1 as $1 )
end
$$ language plpgsql;
-Steve
On Mon, Sep 14, 2009 at 11:02 AM, Steve Prentice <prentice@cisco.com> wrote:
On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote:
2009/9/14 Tom Lane <tgl@sss.pgh.pa.us>:
Robert Haas <robertmhaas@gmail.com> writes:
So, I guess I'm sadly left feeling that we should probably reject this
patch. Anyone want to argue otherwise?+1. I'm really hoping to get something done about the plpgsql parsing
situation before 8.5 is out, so this should be a dead end anyway.I have a WIP patch for integration main SQL parser to plpgsql. I'll
send it to this weekend.I certainly don't mind the patch getting rejected and agree that refactoring
the plpgsql parser is probably the best approach to this issue. However, I
think it would be more than a little strange to ship the named notation
feature without a solution for this problem. For reference, the problem is
that the function below causes a compile error because of the way plpgsql
blindly does variable replacement:create function fun1(pDisplayName text) returns void as $$
begin
perform fun2(pDisplayName as pDisplayName);
-- Above line compiles as:
-- SELECT fun2( $1 as $1 )
end
$$ language plpgsql;
Yeah but we already have this problem. Right now, it typically
happens because of some statement of the form SELECT ... AS ...; this
just adds one more case where it can happen, and I doubt it's any more
common than the case we already struggle with.
But at any rate Tom is planning a fix for 8.5, so I don't think
there's any need to get excited just yet. If Tom doesn't get his
stuff finished by January, we can revisit the issue then.
...Robert
2009/9/14 Steve Prentice <prentice@cisco.com>:
On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote:
2009/9/14 Tom Lane <tgl@sss.pgh.pa.us>:
Robert Haas <robertmhaas@gmail.com> writes:
So, I guess I'm sadly left feeling that we should probably reject this
patch. Anyone want to argue otherwise?+1. I'm really hoping to get something done about the plpgsql parsing
situation before 8.5 is out, so this should be a dead end anyway.I have a WIP patch for integration main SQL parser to plpgsql. I'll
send it to this weekend.I certainly don't mind the patch getting rejected and agree that refactoring
the plpgsql parser is probably the best approach to this issue. However, I
think it would be more than a little strange to ship the named notation
feature without a solution for this problem. For reference, the problem is
that the function below causes a compile error because of the way plpgsql
blindly does variable replacement:create function fun1(pDisplayName text) returns void as $$
begin
perform fun2(pDisplayName as pDisplayName);
-- Above line compiles as:
-- SELECT fun2( $1 as $1 )
end
$$ language plpgsql;
I am sure, so this this will be solved in next commitfest. This
problem is related only to plpgsql. Other PL languages are well,
because doesn't try to emulate SQL parser.
Pavel
Show quoted text
-Steve
On Mon, Sep 14, 2009 at 11:56 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2009/9/14 Steve Prentice <prentice@cisco.com>:
On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote:
2009/9/14 Tom Lane <tgl@sss.pgh.pa.us>:
Robert Haas <robertmhaas@gmail.com> writes:
So, I guess I'm sadly left feeling that we should probably reject this
patch. Anyone want to argue otherwise?+1. I'm really hoping to get something done about the plpgsql parsing
situation before 8.5 is out, so this should be a dead end anyway.I have a WIP patch for integration main SQL parser to plpgsql. I'll
send it to this weekend.I certainly don't mind the patch getting rejected and agree that refactoring
the plpgsql parser is probably the best approach to this issue. However, I
think it would be more than a little strange to ship the named notation
feature without a solution for this problem. For reference, the problem is
that the function below causes a compile error because of the way plpgsql
blindly does variable replacement:create function fun1(pDisplayName text) returns void as $$
begin
perform fun2(pDisplayName as pDisplayName);
-- Above line compiles as:
-- SELECT fun2( $1 as $1 )
end
$$ language plpgsql;I am sure, so this this will be solved in next commitfest. This
problem is related only to plpgsql. Other PL languages are well,
because doesn't try to emulate SQL parser.
And the emphasis here is on "try".
...Robert