Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
On Mon, Sep 14, 2009 at 11:56 AM, Pavel Stehule wrote: > 2009/9/14 Steve Prentice : >> On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote: >> >>> 2009/9/14 Tom Lane : Robert Haas 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
2009/9/14 Steve Prentice : > On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote: > >> 2009/9/14 Tom Lane : >>> >>> Robert Haas 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 > -Steve > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
On Mon, Sep 14, 2009 at 11:02 AM, Steve Prentice wrote: > On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote: > >> 2009/9/14 Tom Lane : >>> >>> Robert Haas 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
On Sep 13, 2009, at 10:22 PM, Pavel Stehule wrote: 2009/9/14 Tom Lane : Robert Haas 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
2009/9/14 Tom Lane : > Robert Haas 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 > 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 > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
Robert Haas 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
On Thu, May 21, 2009 at 2:46 PM, Steve Prentice 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
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. plpgsql_keyword_as.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
--On Donnerstag, Mai 21, 2009 11:46:24 -0700 Steve Prentice 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] 8.5 plpgsql change for named notation: treat word following AS keyword as label v3
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 statement_name(text,
timestamp) AS
-The substitution mechanism will replace any token that
matches a
+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
-Variable substitution does not happen in the command string given
+There are two places where variable substitution does not happen.
+
+
+
+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,
+
+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;
+
+ All occurances of v_logtxt in the function are replaced except the
one
+ following "AS".
+
+
+
+Variable substitution also does not happen in the command string
given
to 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;}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
