agora inbox for [email protected]
help / color / mirror / Atom feedPatch for cursor calling with named parameters
38+ messages / 10 participants
[nested] [flat]
* Patch for cursor calling with named parameters
@ 2011-09-15 08:18 Yeb Havinga <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Yeb Havinga @ 2011-09-15 08:18 UTC (permalink / raw)
To: pgsql-hackers
Hello list,
The following patch implements cursor calling with named parameters in
addition to the standard positional argument lists.
c1 cursor (param1 int, param2 int) for select * from rc_test where a >
param1 and b > param2;
open c1($1, $2); -- this is currently possible
open c1(param2 := $2, param1 := $1); -- this is the new feature
Especially for cursors with a lot of arguments, this increases
readability of code. This was discussed previously in
http://archives.postgresql.org/pgsql-hackers/2010-09/msg01433.php. We
actually made two patches: one with => and then one with := notation.
Attached is the patch with := notation.
Is it ok to add it to the next commitfest?
regards,
Yeb Havinga, Willem Dijkstra
--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data
Attachments:
[text/x-patch] cursornamedparameter-v1.patch (14.0K, ../../[email protected]/2-cursornamedparameter-v1.patch)
download | inline diff:
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
new file mode 100644
index 92b54dd..192f278
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
*************** read_sql_expression(int until, const cha
*** 2335,2340 ****
--- 2335,2352 ----
"SELECT ", true, true, NULL, NULL);
}
+ /*
+ * Convenience routine to read a single unchecked expression with two possible
+ * terminators, returning an expression with an empty sql prefix.
+ */
+ static PLpgSQL_expr *
+ read_sql_one_expression(int until, int until2, const char *expected,
+ int *endtoken)
+ {
+ return read_sql_construct(until, until2, 0, expected,
+ "", true, false, NULL, endtoken);
+ }
+
/* Convenience routine to read an expression with two possible terminators */
static PLpgSQL_expr *
read_sql_expression2(int until, int until2, const char *expected,
*************** check_labels(const char *start_label, co
*** 3384,3399 ****
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL.
! * If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token. Consume all that and return a SELECT query that
! * evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
! int tok;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
--- 3396,3418 ----
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL. If it
! * does have args, we expect to see "( expr [, expr ...] )" followed by the
! * until token, where expr may be a plain expression, or a named parameter
! * assignment of the form IDENT := expr. Consume all that and return a SELECT
! * query that evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
! PLpgSQL_row *row;
! int tok;
! int argc = 0;
! char **argv;
! StringInfoData ds;
! char *sqlstart = "SELECT ";
! int startlocation = yylloc;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3412,3417 ****
--- 3431,3439 ----
return NULL;
}
+ row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+ argv = (char **) palloc0(sizeof(char *) * row->nfields);
+
/* Else better provide arguments */
if (tok != '(')
ereport(ERROR,
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3420,3429 ****
cursor->refname),
parser_errposition(yylloc)));
! /*
! * Read expressions until the matching ')'.
! */
! expr = read_sql_expression(')', ")");
/* Next we'd better find the until token */
tok = yylex();
--- 3442,3527 ----
cursor->refname),
parser_errposition(yylloc)));
! for (argc = 0; argc < row->nfields; argc++)
! {
! int argpos;
! int endtoken;
! PLpgSQL_expr *item;
!
! if (plpgsql_isidentassign())
! {
! /* Named parameter assignment */
! for (argpos = 0; argpos < row->nfields; argpos++)
! if (strncmp(row->fieldnames[argpos], yylval.str, strlen(row->fieldnames[argpos])) == 0)
! break;
!
! if (argpos == row->nfields)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" has no argument named \"%s\"",
! cursor->refname, yylval.str),
! parser_errposition(yylloc)));
! }
! else
! {
! /* Positional parameter assignment */
! argpos = argc;
! }
!
! /*
! * Read one expression at a time until the matching endtoken. Checking
! * the expressions is postponed until the positional argument list is
! * made.
! */
! item = read_sql_one_expression(',', ')', ",\" or \")", &endtoken);
!
! if (endtoken == ')' && !(argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("not enough arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (endtoken == ',' && (argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("too many arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (argv[argpos] != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" argument %d \"%s\" provided multiple times",
! cursor->refname, argpos + 1, row->fieldnames[argpos]),
! parser_errposition(yylloc)));
!
! argv[argpos] = item->query;
! }
!
! /* Make positional argument list */
! initStringInfo(&ds);
! appendStringInfoString(&ds, sqlstart);
! for (argc = 0; argc < row->nfields; argc++)
! {
! Assert(argv[argc] != NULL);
! appendStringInfoString(&ds, argv[argc]);
!
! if (argc < row->nfields - 1)
! appendStringInfoString(&ds, "\n,"); /* use newline to end possible -- comment in arg */
! }
! appendStringInfoChar(&ds, ';');
!
! expr = palloc0(sizeof(PLpgSQL_expr));
! expr->dtype = PLPGSQL_DTYPE_EXPR;
! expr->query = pstrdup(ds.data);
! expr->plan = NULL;
! expr->paramnos = NULL;
! expr->ns = plpgsql_ns_top();
! pfree(ds.data);
!
! /* Check if sql is valid */
! check_sql_expr(expr->query, startlocation, strlen(sqlstart));
/* Next we'd better find the until token */
tok = yylex();
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
new file mode 100644
index 76e8436..9c233c4
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
*************** plpgsql_scanner_finish(void)
*** 583,585 ****
--- 583,617 ----
yyscanner = NULL;
scanorig = NULL;
}
+
+ /*
+ * Return true if 'IDENT' ':=' are the next two tokens
+ */
+ bool
+ plpgsql_isidentassign(void)
+ {
+ int tok1, tok2;
+ TokenAuxData aux1, aux2;
+ bool result = false;
+
+ tok1 = internal_yylex(&aux1);
+ if (tok1 == IDENT)
+ {
+ tok2 = internal_yylex(&aux2);
+
+ if (tok2 == COLON_EQUALS)
+ result = true;
+ else
+ push_back_token(tok2, &aux2);
+ }
+
+ if (!result)
+ push_back_token(tok1, &aux1);
+
+ plpgsql_yylval = aux1.lval;
+ plpgsql_yylloc = aux1.lloc;
+ plpgsql_yyleng = aux1.leng;
+
+ return result;
+ }
+
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
new file mode 100644
index ed8fcad..2ecb82f
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** extern int plpgsql_location_to_lineno(in
*** 950,955 ****
--- 950,956 ----
extern int plpgsql_latest_lineno(void);
extern void plpgsql_scanner_init(const char *str);
extern void plpgsql_scanner_finish(void);
+ extern bool plpgsql_isidentassign(void);
/* ----------
* Externs in gram.y
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
new file mode 100644
index bed34c8..bf6fba6
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** select refcursor_test2(20000, 20000) as
*** 2292,2297 ****
--- 2292,2412 ----
(1 row)
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param2 := $2, -- command after ,
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test2(20000, 20000) as "Should be false",
+ namedparmcursor_test2(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ create function namedparmcursor_test3(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1 -- comment before ,
+ , $2);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test3(20000, 20000) as "Should be false",
+ namedparmcursor_test3(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ -- should fail
+ create function namedparmcursor_test4(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1($1, param1 := $1);
+ end
+ $$ language plpgsql;
+ ERROR: cursor "c1" argument 1 "param1" provided multiple times
+ LINE 5: open c1($1, param1 := $1);
+ ^
+ -- should fail
+ create function namedparmcursor_test5(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2, param1 := $1, 3);
+ end
+ $$ language plpgsql;
+ ERROR: too many arguments for cursor "c1"
+ LINE 5: open c1(param2 := $2, param1 := $1, 3);
+ ^
+ -- should fail
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+ ERROR: not enough arguments for cursor "c1"
+ LINE 5: open c1(param2 := $2);
+ ^
+ -- should fail
+ create function namedparmcursor_test7(int, int) returns void as $$
+ declare
+ c1 cursor for select * from rc_test;
+ begin
+ open c1(param1 := $1);
+ end
+ $$ language plpgsql;
+ ERROR: cursor "c1" has no arguments
+ LINE 5: open c1(param1 := $1);
+ ^
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
new file mode 100644
index 05f0315..59db90f
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select refcursor_test2(20000, 20000) as
*** 1946,1951 ****
--- 1946,2049 ----
refcursor_test2(20, 20) as "Should be true";
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param2 := $2, -- command after ,
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test2(20000, 20000) as "Should be false",
+ namedparmcursor_test2(20, 20) as "Should be true";
+
+ create function namedparmcursor_test3(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1 -- comment before ,
+ , $2);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test3(20000, 20000) as "Should be false",
+ namedparmcursor_test3(20, 20) as "Should be true";
+
+ -- should fail
+ create function namedparmcursor_test4(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1($1, param1 := $1);
+ end
+ $$ language plpgsql;
+
+ -- should fail
+ create function namedparmcursor_test5(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2, param1 := $1, 3);
+ end
+ $$ language plpgsql;
+
+ -- should fail
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+
+ -- should fail
+ create function namedparmcursor_test7(int, int) returns void as $$
+ declare
+ c1 cursor for select * from rc_test;
+ begin
+ open c1(param1 := $1);
+ end
+ $$ language plpgsql;
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: Patch for cursor calling with named parameters
@ 2011-09-15 14:31 Cédric Villemain <[email protected]>
parent: Yeb Havinga <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Cédric Villemain @ 2011-09-15 14:31 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; +Cc: pgsql-hackers
2011/9/15 Yeb Havinga <[email protected]>:
> Hello list,
>
> The following patch implements cursor calling with named parameters in
> addition to the standard positional argument lists.
>
> c1 cursor (param1 int, param2 int) for select * from rc_test where a >
> param1 and b > param2;
> open c1($1, $2); -- this is currently possible
> open c1(param2 := $2, param1 := $1); -- this is the new feature
>
> Especially for cursors with a lot of arguments, this increases readability
> of code. This was discussed previously in
> http://archives.postgresql.org/pgsql-hackers/2010-09/msg01433.php. We
> actually made two patches: one with => and then one with := notation.
> Attached is the patch with := notation.
>
> Is it ok to add it to the next commitfest?
I think it is, as you have provided a patch.
There exist also a mecanism to order the parameters of 'EXECUTE ...
USING ...' (it's using a cursor), can the current work benefit to
EXECUTE USING to use named parameters ?
> regards,
> Yeb Havinga, Willem Dijkstra
>
> --
> Yeb Havinga
> http://www.mgrid.net/
> Mastering Medical Data
>
>
>
> --
> Sent via pgsql-hackers mailing list ([email protected])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: Patch for cursor calling with named parameters
@ 2011-09-15 15:30 Yeb Havinga <[email protected]>
parent: Cédric Villemain <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Yeb Havinga @ 2011-09-15 15:30 UTC (permalink / raw)
To: Cédric Villemain <[email protected]>; +Cc: pgsql-hackers
On 2011-09-15 16:31, Cédric Villemain wrote:
> There exist also a mecanism to order the parameters of 'EXECUTE ...
> USING ...' (it's using a cursor), can the current work benefit to
> EXECUTE USING to use named parameters ?
I looked at it a bit but it seems there is no benefit, since the dynamic
sql handling vs the cursor declaration and opening touch different code
paths in both the plpgsql grammar and the SPI functions that are called.
regards,
Yeb
^ permalink raw reply [nested|flat] 38+ messages in thread
* [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 14:04 Royce Ausburn <[email protected]>
0 siblings, 2 replies; 38+ messages in thread
From: Royce Ausburn @ 2011-10-06 14:04 UTC (permalink / raw)
To: pgsql-hackers; +Cc: [email protected]
Initial Review for patch:
http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php
Submission review
The patch is in context diff format and applies cleanly to the git master. The patch includes an update to regression tests. The regression tests pass. The patch does not include updates to the documentation reflecting the new functionality.
Usability review
The patch adds a means of specifying named cursor parameter arguments in pg/plsql.
The syntax is straight forward:
cur1 CURSOR (param1 int, param2 int, param3 int) for select …;
…
open cur1(param3 := 4, param2 := 1, param1 := 5);
The old syntax continues to work:
cur1 CURSOR (param1 int, param2 int, param3 int) for select …;
…
open cur1(5, 1, 3);
• Does the patch actually implement that?
Yes, the feature works as advertised.
• Do we want that?
I very rarely use pg/plsql, so I won't speak to its utility. However there has been some discussion about the idea:
http://archives.postgresql.org/pgsql-hackers/2010-09/msg01440.php
• Do we already have it?
Not AFAIK
• Does it follow SQL spec, or the community-agreed behavior?
There's some discussion about the syntax ( := or => ), I'm not sure there's a consensus yet.
• Does it include pg_dump support (if applicable)?
Yes -- pgplsql functions using named parameters are correctly dumped.
• Are there dangers?
Potentially. See below.
• Have all the bases been covered?
I don't think so. The new feature accepts opening a cursor with some parameter names not specified:
open cur1(param3 := 4, 1, param1 := 5);
It seems that if a parameter is not named, its position is used to bind to a variable. For example, the following fail:
psql:plsqltest.sql:26: ERROR: cursor "cur1" argument 2 "param2" provided multiple times
LINE 10: open cur1(param3 := 4, 1, param2 := 5);
and
psql:plsqltest.sql:26: ERROR: cursor "cur1" argument 2 "param2" provided multiple times
LINE 10: open cur1(param2 := 4, 2, param1 := 5);
I think that postgres ought to enforce some consistency here. Use one way or the other, never both.
I can also produce some unhelpful errors when I give bad syntax. For example:
psql:plsqltest.sql:28: ERROR: cursor "cur1" argument 1 "param1" provided multiple times
LINE 11: open cur1( param3 : = 4, 2, param1 := 5);
(notice the space between the : and =)
--
psql:plsqltest.sql:28: ERROR: cursor "cur1" argument 1 "param1" provided multiple times
LINE 11: open cur1( param3 => 4, 2, param1 := 5);
(Wrong assignment operator)
--
psql:plsqltest.sql:27: ERROR: cursor "cur1" argument 3 "param3" provided multiple times
LINE 10: open cur1( 1 , param3 := 2, param2 = 3 );
(Wrong assignment operator)
--
psql:plsqltest.sql:27: ERROR: cursor "cur1" argument 3 "param3" provided multiple times
LINE 10: ... open cur1( param3 = param3 , param3 := 2, param2 = 3 );
--
open cur1( param3 := param3 , param2 = 3, param1 := 1 );
psql:plsqltest.sql:29: ERROR: column "param2" does not exist
LINE 2: ,param2 = 3
^
QUERY: SELECT 1
,param2 = 3
,param3;
CONTEXT: PL/pgSQL function "named_para_test" line 7 at OPEN
I haven't been able to make something syntactically incorrect that doesn't produce an error, even if the error isn't spot on. I haven't been able to make it crash, and when the syntax is correct, it has always produced correct results.
Performance review
I've done some limited performance testing and I can't really see a difference between the unpatched and patched master.
• Does it follow the project coding guidelines?
I believe so, but someone more familiar with them will probably spot violations better than me =)
• Are there portability issues?
I wouldn't know -- I don't have any experience with C portability.
• Will it work on Windows/BSD etc?
Tested under OS X, so BSD is presumably okay. No idea about other unixes nor windows.
• Are the comments sufficient and accurate?
I'm happy enough with them.
• Does it do what it says, correctly?
Yes, excepting my comments above.
• Does it produce compiler warnings?
Yes:
In file included from gram.y:12962:
scan.c: In function ‘yy_try_NUL_trans’:
scan.c:16243: warning: unused variable ‘yyg’
But this was not added by this patch -- it's also in the unpatched master.
• Can you make it crash?
No.
--Royce
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 16:23 Tom Lane <[email protected]>
parent: Royce Ausburn <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Tom Lane @ 2011-10-06 16:23 UTC (permalink / raw)
To: Royce Ausburn <[email protected]>; +Cc: pgsql-hackers; [email protected]
Royce Ausburn <[email protected]> writes:
> Initial Review for patch:
> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php
> The patch adds a means of specifying named cursor parameter arguments in pg/plsql.
> � Do we want that?
> I very rarely use pg/plsql, so I won't speak to its utility. However there has been some discussion about the idea:
> http://archives.postgresql.org/pgsql-hackers/2010-09/msg01440.php
I still think what I said in that message, which is that it's premature
to add this syntax to plpgsql cursors when we have thoughts of changing
it. There is not any groundswell of demand from the field for named
parameters to cursors, so I think we can just leave this in abeyance
until the function case has settled.
regards, tom lane
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 16:38 Robert Haas <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Robert Haas @ 2011-10-06 16:38 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
On Thu, Oct 6, 2011 at 12:23 PM, Tom Lane <[email protected]> wrote:
> Royce Ausburn <[email protected]> writes:
>> Initial Review for patch:
>> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php
>> The patch adds a means of specifying named cursor parameter arguments in pg/plsql.
>
>> • Do we want that?
>
>> I very rarely use pg/plsql, so I won't speak to its utility. However there has been some discussion about the idea:
>> http://archives.postgresql.org/pgsql-hackers/2010-09/msg01440.php
>
> I still think what I said in that message, which is that it's premature
> to add this syntax to plpgsql cursors when we have thoughts of changing
> it. There is not any groundswell of demand from the field for named
> parameters to cursors, so I think we can just leave this in abeyance
> until the function case has settled.
+1. However, if that's the route we're traveling down, I think we had
better go ahead and remove the one remaining => operator from hstore
in 9.2:
CREATE OPERATOR => (
LEFTARG = text,
RIGHTARG = text,
PROCEDURE = hstore
);
We've been warning that this operator name was deprecated since 9.0,
so it's probably about time to take the next step, if we want to have
a chance of getting this sorted out in finite time.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 16:46 Tom Lane <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Tom Lane @ 2011-10-06 16:46 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
Robert Haas <[email protected]> writes:
> +1. However, if that's the route we're traveling down, I think we had
> better go ahead and remove the one remaining => operator from hstore
> in 9.2:
Fair enough.
regards, tom lane
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 17:29 David E. Wheeler <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: David E. Wheeler @ 2011-10-06 17:29 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
On Oct 6, 2011, at 9:46 AM, Tom Lane wrote:
>> +1. However, if that's the route we're traveling down, I think we had
>> better go ahead and remove the one remaining => operator from hstore
>> in 9.2:
>
> Fair enough.
Would it then be added as an alias for := for named function parameters? Or would that come still later?
Best,
David
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 17:37 Tom Lane <[email protected]>
parent: David E. Wheeler <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Tom Lane @ 2011-10-06 17:37 UTC (permalink / raw)
To: David E. Wheeler <[email protected]>; +Cc: Robert Haas <[email protected]>; Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
"David E. Wheeler" <[email protected]> writes:
> On Oct 6, 2011, at 9:46 AM, Tom Lane wrote:
>>> +1. However, if that's the route we're traveling down, I think we had
>>> better go ahead and remove the one remaining => operator from hstore
>>> in 9.2:
>> Fair enough.
> Would it then be added as an alias for := for named function parameters? Or would that come still later?
Once we do that, it will be impossible not merely deprecated to use =>
as an operator name. I think that has to wait at least another release
cycle or two past where we're using it ourselves.
regards, tom lane
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 17:39 David E. Wheeler <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: David E. Wheeler @ 2011-10-06 17:39 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
On Oct 6, 2011, at 10:37 AM, Tom Lane wrote:
>> Would it then be added as an alias for := for named function parameters? Or would that come still later?
>
> Once we do that, it will be impossible not merely deprecated to use =>
> as an operator name. I think that has to wait at least another release
> cycle or two past where we're using it ourselves.
Okay. I kind of like := so there's no rush AFAIC. :-)
David
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 17:46 Tom Lane <[email protected]>
parent: David E. Wheeler <[email protected]>
0 siblings, 2 replies; 38+ messages in thread
From: Tom Lane @ 2011-10-06 17:46 UTC (permalink / raw)
To: David E. Wheeler <[email protected]>; +Cc: Robert Haas <[email protected]>; Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
"David E. Wheeler" <[email protected]> writes:
>>> Would it then be added as an alias for := for named function parameters? Or would that come still later?
>> Once we do that, it will be impossible not merely deprecated to use =>
>> as an operator name. I think that has to wait at least another release
>> cycle or two past where we're using it ourselves.
> Okay. I kind of like := so there's no rush AFAIC. :-)
Hmm ... actually, that raises another issue that I'm not sure whether
there's consensus for or not. Are we intending to keep name := value
syntax forever, as an alternative to the standard name => value syntax?
I can't immediately see a reason not to, other than the "it's not
standard" argument.
Because if we *are* going to keep it forever, there's no very good
reason why we shouldn't accept this plpgsql cursor patch now. We'd
just have to remember to extend plpgsql to take => at the same time
we do that for core function calls.
regards, tom lane
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 17:51 David E. Wheeler <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 0 replies; 38+ messages in thread
From: David E. Wheeler @ 2011-10-06 17:51 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
On Oct 6, 2011, at 10:46 AM, Tom Lane wrote:
>> Okay. I kind of like := so there's no rush AFAIC. :-)
>
> Hmm ... actually, that raises another issue that I'm not sure whether
> there's consensus for or not. Are we intending to keep name := value
> syntax forever, as an alternative to the standard name => value syntax?
> I can't immediately see a reason not to, other than the "it's not
> standard" argument.
The only reason it would be required, I think, is if the SQL standard developed some other use for that operator.
> Because if we *are* going to keep it forever, there's no very good
> reason why we shouldn't accept this plpgsql cursor patch now. We'd
> just have to remember to extend plpgsql to take => at the same time
> we do that for core function calls.
Makes sense.
Best,
David
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 17:52 Robert Haas <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Robert Haas @ 2011-10-06 17:52 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: David E. Wheeler <[email protected]>; Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane <[email protected]> wrote:
> "David E. Wheeler" <[email protected]> writes:
>>>> Would it then be added as an alias for := for named function parameters? Or would that come still later?
>
>>> Once we do that, it will be impossible not merely deprecated to use =>
>>> as an operator name. I think that has to wait at least another release
>>> cycle or two past where we're using it ourselves.
>
>> Okay. I kind of like := so there's no rush AFAIC. :-)
>
> Hmm ... actually, that raises another issue that I'm not sure whether
> there's consensus for or not. Are we intending to keep name := value
> syntax forever, as an alternative to the standard name => value syntax?
> I can't immediately see a reason not to, other than the "it's not
> standard" argument.
>
> Because if we *are* going to keep it forever, there's no very good
> reason why we shouldn't accept this plpgsql cursor patch now. We'd
> just have to remember to extend plpgsql to take => at the same time
> we do that for core function calls.
It's hard to see adding support for => and dropping support for := in
the same release. That would be a compatibility nightmare.
If := is used by the standard for some other, incompatible purpose,
then I suppose we would want to add support for =>, wait a few
releases, deprecate :=, wait a couple of releases, remove :=
altogether. But IIRC we picked := precisely because the standard
didn't use it at all, or at least not for anything related... in which
case we may as well keep it around more or less indefinitely.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-06 20:15 Pavel Stehule <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Pavel Stehule @ 2011-10-06 20:15 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; David E. Wheeler <[email protected]>; Royce Ausburn <[email protected]>; pgsql-hackers; [email protected]
2011/10/6 Robert Haas <[email protected]>:
> On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane <[email protected]> wrote:
>> "David E. Wheeler" <[email protected]> writes:
>>>>> Would it then be added as an alias for := for named function parameters? Or would that come still later?
>>
>>>> Once we do that, it will be impossible not merely deprecated to use =>
>>>> as an operator name. I think that has to wait at least another release
>>>> cycle or two past where we're using it ourselves.
>>
>>> Okay. I kind of like := so there's no rush AFAIC. :-)
>>
>> Hmm ... actually, that raises another issue that I'm not sure whether
>> there's consensus for or not. Are we intending to keep name := value
>> syntax forever, as an alternative to the standard name => value syntax?
>> I can't immediately see a reason not to, other than the "it's not
>> standard" argument.
>>
>> Because if we *are* going to keep it forever, there's no very good
>> reason why we shouldn't accept this plpgsql cursor patch now. We'd
>> just have to remember to extend plpgsql to take => at the same time
>> we do that for core function calls.
>
> It's hard to see adding support for => and dropping support for := in
> the same release. That would be a compatibility nightmare.
>
> If := is used by the standard for some other, incompatible purpose,
> then I suppose we would want to add support for =>, wait a few
> releases, deprecate :=, wait a couple of releases, remove :=
> altogether. But IIRC we picked := precisely because the standard
> didn't use it at all, or at least not for anything related... in which
> case we may as well keep it around more or less indefinitely.
+1
Pavel
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> --
> Sent via pgsql-hackers mailing list ([email protected])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-07 02:54 Royce Ausburn <[email protected]>
parent: Pavel Stehule <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Royce Ausburn @ 2011-10-07 02:54 UTC (permalink / raw)
To: pgsql-hackers
Forgive my ignorance -- do I need to be doing anything else now seeing as I started the review?
On 07/10/2011, at 7:15 AM, Pavel Stehule wrote:
> 2011/10/6 Robert Haas <[email protected]>:
>> On Thu, Oct 6, 2011 at 1:46 PM, Tom Lane <[email protected]> wrote:
>>> "David E. Wheeler" <[email protected]> writes:
>>>>>> Would it then be added as an alias for := for named function parameters? Or would that come still later?
>>>
>>>>> Once we do that, it will be impossible not merely deprecated to use =>
>>>>> as an operator name. I think that has to wait at least another release
>>>>> cycle or two past where we're using it ourselves.
>>>
>>>> Okay. I kind of like := so there's no rush AFAIC. :-)
>>>
>>> Hmm ... actually, that raises another issue that I'm not sure whether
>>> there's consensus for or not. Are we intending to keep name := value
>>> syntax forever, as an alternative to the standard name => value syntax?
>>> I can't immediately see a reason not to, other than the "it's not
>>> standard" argument.
>>>
>>> Because if we *are* going to keep it forever, there's no very good
>>> reason why we shouldn't accept this plpgsql cursor patch now. We'd
>>> just have to remember to extend plpgsql to take => at the same time
>>> we do that for core function calls.
>>
>> It's hard to see adding support for => and dropping support for := in
>> the same release. That would be a compatibility nightmare.
>>
>> If := is used by the standard for some other, incompatible purpose,
>> then I suppose we would want to add support for =>, wait a few
>> releases, deprecate :=, wait a couple of releases, remove :=
>> altogether. But IIRC we picked := precisely because the standard
>> didn't use it at all, or at least not for anything related... in which
>> case we may as well keep it around more or less indefinitely.
>
> +1
>
> Pavel
>
>>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> --
>> Sent via pgsql-hackers mailing list ([email protected])
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>
> --
> Sent via pgsql-hackers mailing list ([email protected])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-07 10:21 Yeb Havinga <[email protected]>
parent: Royce Ausburn <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Yeb Havinga @ 2011-10-07 10:21 UTC (permalink / raw)
To: Royce Ausburn <[email protected]>; +Cc: pgsql-hackers
On 2011-10-06 16:04, Royce Ausburn wrote:
> Initial Review for patch:
>
> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php
Hello Royce,
Thank you for your review.
>
> I don't think so. The new feature accepts opening a cursor with some
> parameter names not specified:
>
> open cur1(param3 := 4, 1, param1 := 5);
>
> It seems that if a parameter is not named, its position is used to
> bind to a variable. For example, the following fail:
>
> psql:plsqltest.sql:26: ERROR: cursor "cur1" argument 2 "param2"
> provided multiple times
> LINE 10: open cur1(param3 := 4, 1, param2 := 5);
>
> and
>
> psql:plsqltest.sql:26: ERROR: cursor "cur1" argument 2 "param2"
> provided multiple times
> LINE 10: open cur1(param2 := 4, 2, param1 := 5);
>
>
> I think that postgres ought to enforce some consistency here. Use one
> way or the other, never both.
This was meant as a feature, but I can remove it.
>
>
> I can also produce some unhelpful errors when I give bad syntax. For
> example:
>
> psql:plsqltest.sql:28: ERROR: cursor "cur1" argument 1 "param1"
> provided multiple times
> LINE 11: open cur1( param3 : = 4, 2, param1 := 5);
> (notice the space between the : and =)
Yes, the whole of the expression before the first comma, 'param3 : = 4'
is not recognized as <parametername> <:= symbol> <expression>, so that
is taken as the value of the first parameter. This value is parsed after
all named arguments are read, and hence no meaningful error is given. If
there was no param1 parameter name at the end, the 'multiple times'
error would not have caused the processing to stop, and a syntax error
at the correct : would have been given.
The same reasoning also explains the other 'multiple times' errors you
could get, by putting a syntax error in some value.
>
> --
>
> open cur1( param3 := param3 , param2 = 3, param1 := 1 );
>
> psql:plsqltest.sql:29: ERROR: column "param2" does not exist
> LINE 2: ,param2 = 3
> ^
> QUERY: SELECT 1
> ,param2 = 3
> ,param3;
> CONTEXT: PL/pgSQL function "named_para_test" line 7 at OPEN
This is a valid error, since the parser / SQL will try to evaluate the
boolean expression param2 = 3, while param2 is not a defined variabele.
Again, thank you very much for your thorough review. I'll update the
patch so mixing positional and named parameters are removed, add
documentation, and give syntax errors before an error message indicating
that positional and named parameters were mixed.
--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-07 14:56 Yeb Havinga <[email protected]>
parent: Yeb Havinga <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Yeb Havinga @ 2011-10-07 14:56 UTC (permalink / raw)
To: Royce Ausburn <[email protected]>; +Cc: pgsql-hackers
On 2011-10-07 12:21, Yeb Havinga wrote:
> On 2011-10-06 16:04, Royce Ausburn wrote:
>> Initial Review for patch:
>>
>> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php
>
>
> Again, thank you very much for your thorough review. I'll update the
> patch so mixing positional and named parameters are removed, add
> documentation, and give syntax errors before an error message
> indicating that positional and named parameters were mixed.
>
Attach is v2 of the patch.
Mixed notation now raises an error.
In contrast with what I said above, named parameter related errors are
thrown before any syntax errors. I tested with raising syntax errors
first but the resulting code was a bit more ugly and the sql checking
under a error condition (i.e. double named parameter error means there
is one parameter in short) was causing serious errors.
Documentation was also added, regression tests updated.
regards,
--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data
Attachments:
[text/x-patch] cursornamedparameter-v2.patch (14.9K, ../../[email protected]/2-cursornamedparameter-v2.patch)
download | inline diff:
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index c14c34c..45081f8
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** END;
*** 2699,2718 ****
Another way is to use the cursor declaration syntax,
which in general is:
<synopsis>
! <replaceable>name</replaceable> <optional> <optional> NO </optional> SCROLL </optional> CURSOR <optional> ( <replaceable>arguments</replaceable> ) </optional> FOR <replaceable>query</replaceable>;
</synopsis>
(<literal>FOR</> can be replaced by <literal>IS</> for
! <productname>Oracle</productname> compatibility.)
! If <literal>SCROLL</> is specified, the cursor will be capable of
! scrolling backward; if <literal>NO SCROLL</> is specified, backward
! fetches will be rejected; if neither specification appears, it is
! query-dependent whether backward fetches will be allowed.
! <replaceable>arguments</replaceable>, if specified, is a
! comma-separated list of pairs <literal><replaceable>name</replaceable>
! <replaceable>datatype</replaceable></literal> that define names to be
! replaced by parameter values in the given query. The actual
! values to substitute for these names will be specified later,
! when the cursor is opened.
</para>
<para>
Some examples:
--- 2699,2717 ----
Another way is to use the cursor declaration syntax,
which in general is:
<synopsis>
! <replaceable>name</replaceable> <optional> <optional> NO </optional> SCROLL </optional> CURSOR <optional> ( <optional> <replaceable>argname</replaceable> </optional> <replaceable>argtype</replaceable> <optional>, ...</optional>) </optional> FOR <replaceable>query</replaceable>;
</synopsis>
(<literal>FOR</> can be replaced by <literal>IS</> for
! <productname>Oracle</productname> compatibility.) If <literal>SCROLL</>
! is specified, the cursor will be capable of scrolling backward; if
! <literal>NO SCROLL</> is specified, backward fetches will be rejected; if
! neither specification appears, it is query-dependent whether backward
! fetches will be allowed. <replaceable>argname</replaceable>, if
! specified, defines the name to be replaced by parameter values in the
! given query. The actual values to substitute for these names will be
! specified later, when the cursor is opened.
! <literal><replaceable>argtype</replaceable></literal> defines the datatype
! of the parameter.
</para>
<para>
Some examples:
*************** OPEN curs1 FOR EXECUTE 'SELECT * FROM '
*** 2827,2833 ****
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional>;
</synopsis>
<para>
--- 2826,2832 ----
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional>;
</synopsis>
<para>
*************** OPEN <replaceable>bound_cursorvar</repla
*** 2854,2864 ****
--- 2853,2875 ----
<command>OPEN</>.
</para>
+ <para>
+ Cursors that have named parameters may be opened using either
+ <firstterm>named</firstterm> or <firstterm>positional</firstterm>
+ notation. In contrast with calling functions, described in <xref
+ linkend="sql-syntax-calling-funcs">, it is not allowed to mix
+ positional and named notation. In positional notation, all arguments
+ are specified in order. In named notation, each argument's name is
+ specified using <literal>:=</literal> to separate it from the
+ argument expression.
+ </para>
+
<para>
Examples:
<programlisting>
OPEN curs2;
OPEN curs3(42);
+ OPEN curs3(key := 42);
</programlisting>
</para>
</sect3>
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
new file mode 100644
index f8e956b..b9bf888
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
*************** read_sql_expression(int until, const cha
*** 2337,2342 ****
--- 2337,2354 ----
"SELECT ", true, true, NULL, NULL);
}
+ /*
+ * Convenience routine to read a single unchecked expression with two possible
+ * terminators, returning an expression with an empty sql prefix.
+ */
+ static PLpgSQL_expr *
+ read_sql_one_expression(int until, int until2, const char *expected,
+ int *endtoken)
+ {
+ return read_sql_construct(until, until2, 0, expected,
+ "", true, false, NULL, endtoken);
+ }
+
/* Convenience routine to read an expression with two possible terminators */
static PLpgSQL_expr *
read_sql_expression2(int until, int until2, const char *expected,
*************** check_labels(const char *start_label, co
*** 3386,3401 ****
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL.
! * If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token. Consume all that and return a SELECT query that
! * evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
! int tok;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
--- 3398,3422 ----
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL. If it
! * does have args, we expect to see "( expr [, expr ...] )" followed by the
! * until token, where expr may be a plain expression, or a named parameter
! * assignment of the form IDENT := expr. Consume all that and return a SELECT
! * query that evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
! PLpgSQL_row *row;
! int tok;
! int argc = 0;
! char **argv;
! StringInfoData ds;
! char *sqlstart = "SELECT ";
! int startlocation = yylloc;
! bool named = false;
! bool positional = false;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3414,3419 ****
--- 3435,3443 ----
return NULL;
}
+ row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+ argv = (char **) palloc0(sizeof(char *) * row->nfields);
+
/* Else better provide arguments */
if (tok != '(')
ereport(ERROR,
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3422,3431 ****
cursor->refname),
parser_errposition(yylloc)));
! /*
! * Read expressions until the matching ')'.
! */
! expr = read_sql_expression(')', ")");
/* Next we'd better find the until token */
tok = yylex();
--- 3446,3540 ----
cursor->refname),
parser_errposition(yylloc)));
! for (argc = 0; argc < row->nfields; argc++)
! {
! int argpos;
! int endtoken;
! PLpgSQL_expr *item;
!
! if (plpgsql_isidentassign())
! {
! /* Named parameter assignment */
! named = true;
! for (argpos = 0; argpos < row->nfields; argpos++)
! if (strncmp(row->fieldnames[argpos], yylval.str, strlen(row->fieldnames[argpos])) == 0)
! break;
!
! if (argpos == row->nfields)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" has no argument named \"%s\"",
! cursor->refname, yylval.str),
! parser_errposition(yylloc)));
! }
! else
! {
! /* Positional parameter assignment */
! positional = true;
! argpos = argc;
! }
!
! if (named && positional)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("mixing positional and named parameter assignment not allowed in cursor \"%s\"",
! cursor->refname),
! parser_errposition(startlocation)));
!
! /*
! * Read one expression at a time until the matching endtoken. Checking
! * the expressions is postponed until the positional argument list is
! * made.
! */
! item = read_sql_one_expression(',', ')', ",\" or \")", &endtoken);
!
! if (endtoken == ')' && !(argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("not enough arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (endtoken == ',' && (argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("too many arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (argv[argpos] != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" argument %d \"%s\" provided multiple times",
! cursor->refname, argpos + 1, row->fieldnames[argpos]),
! parser_errposition(yylloc)));
!
! argv[argpos] = item->query;
! }
!
! /* Make positional argument list */
! initStringInfo(&ds);
! appendStringInfoString(&ds, sqlstart);
! for (argc = 0; argc < row->nfields; argc++)
! {
! Assert(argv[argc] != NULL);
! appendStringInfoString(&ds, argv[argc]);
!
! if (argc < row->nfields - 1)
! appendStringInfoString(&ds, "\n,"); /* use newline to end possible -- comment in arg */
! }
! appendStringInfoChar(&ds, ';');
!
! expr = palloc0(sizeof(PLpgSQL_expr));
! expr->dtype = PLPGSQL_DTYPE_EXPR;
! expr->query = pstrdup(ds.data);
! expr->plan = NULL;
! expr->paramnos = NULL;
! expr->ns = plpgsql_ns_top();
! pfree(ds.data);
!
! /* Check if sql is valid */
! check_sql_expr(expr->query, startlocation, strlen(sqlstart));
/* Next we'd better find the until token */
tok = yylex();
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
new file mode 100644
index 76e8436..9c233c4
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
*************** plpgsql_scanner_finish(void)
*** 583,585 ****
--- 583,617 ----
yyscanner = NULL;
scanorig = NULL;
}
+
+ /*
+ * Return true if 'IDENT' ':=' are the next two tokens
+ */
+ bool
+ plpgsql_isidentassign(void)
+ {
+ int tok1, tok2;
+ TokenAuxData aux1, aux2;
+ bool result = false;
+
+ tok1 = internal_yylex(&aux1);
+ if (tok1 == IDENT)
+ {
+ tok2 = internal_yylex(&aux2);
+
+ if (tok2 == COLON_EQUALS)
+ result = true;
+ else
+ push_back_token(tok2, &aux2);
+ }
+
+ if (!result)
+ push_back_token(tok1, &aux1);
+
+ plpgsql_yylval = aux1.lval;
+ plpgsql_yylloc = aux1.lloc;
+ plpgsql_yyleng = aux1.leng;
+
+ return result;
+ }
+
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
new file mode 100644
index 61503f1..9c25d24
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** extern int plpgsql_location_to_lineno(in
*** 960,965 ****
--- 960,966 ----
extern int plpgsql_latest_lineno(void);
extern void plpgsql_scanner_init(const char *str);
extern void plpgsql_scanner_finish(void);
+ extern bool plpgsql_isidentassign(void);
/* ----------
* Externs in gram.y
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
new file mode 100644
index 238bf5f..b986899
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** select refcursor_test2(20000, 20000) as
*** 2292,2297 ****
--- 2292,2346 ----
(1 row)
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param2 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ -- should fail
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+ ERROR: mixing positional and named parameter assignment not allowed in cursor "c1"
+ LINE 6: open c1(param1 := $1, $2);
+ ^
+ -- should fail
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+ ERROR: not enough arguments for cursor "c1"
+ LINE 5: open c1(param2 := $2);
+ ^
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
new file mode 100644
index b47c2de..70033f8
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select refcursor_test2(20000, 20000) as
*** 1946,1951 ****
--- 1946,1993 ----
refcursor_test2(20, 20) as "Should be true";
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param2 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+
+ -- should fail
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+
+ -- should fail
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-11 11:55 Royce Ausburn <[email protected]>
parent: Yeb Havinga <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Royce Ausburn @ 2011-10-11 11:55 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; +Cc: pgsql-hackers
On 08/10/2011, at 1:56 AM, Yeb Havinga wrote:
> Attach is v2 of the patch.
>
> Mixed notation now raises an error.
>
> In contrast with what I said above, named parameter related errors are thrown before any syntax errors. I tested with raising syntax errors first but the resulting code was a bit more ugly and the sql checking under a error condition (i.e. double named parameter error means there is one parameter in short) was causing serious errors.
>
> Documentation was also added, regression tests updated.
I've tested this patch out and can confirm mixed notation produces an error:
psql:plsqltest.sql:27: ERROR: mixing positional and named parameter assignment not allowed in cursor "cur1"
LINE 10: open cur1( param2 := 4, 2, 5);
Just one small thing: it'd be nice to have an example for cursor declaration with named parameters. Your patch adds one for opening a cursor, but not for declaring one.
Other than that, I think the patch is good. Everything works as advertised =)
--Royce
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-11 12:38 Yeb Havinga <[email protected]>
parent: Royce Ausburn <[email protected]>
0 siblings, 2 replies; 38+ messages in thread
From: Yeb Havinga @ 2011-10-11 12:38 UTC (permalink / raw)
To: Royce Ausburn <[email protected]>; +Cc: pgsql-hackers
Hello Royce,
Thanks again for testing.
On 2011-10-11 13:55, Royce Ausburn wrote:
> Just one small thing: it'd be nice to have an example for cursor declaration with named parameters. Your patch adds one for opening a cursor, but not for declaring one.
Declaration of cursors with named parameters is already part of
PostgreSQL (so it is possible to use the parameter names in the cursor
query instead of $1, $2, etc.) and it also already documented with an
example, just a few lines above the open examples. See curs3 on
http://developer.postgresql.org/pgdocs/postgres/plpgsql-cursors.html
> Other than that, I think the patch is good. Everything works as advertised =)
Thanks!
--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-11 12:40 Royce Ausburn <[email protected]>
parent: Yeb Havinga <[email protected]>
1 sibling, 0 replies; 38+ messages in thread
From: Royce Ausburn @ 2011-10-11 12:40 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; +Cc: pgsql-hackers
On 11/10/2011, at 11:38 PM, Yeb Havinga wrote:
> Declaration of cursors with named parameters is already part of PostgreSQL (so it is possible to use the parameter names in the cursor query instead of $1, $2, etc.) and it also already documented with an example, just a few lines above the open examples. See curs3 on http://developer.postgresql.org/pgdocs/postgres/plpgsql-cursors.html
Doh - my apologies!
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-15 05:41 Tom Lane <[email protected]>
parent: Yeb Havinga <[email protected]>
1 sibling, 2 replies; 38+ messages in thread
From: Tom Lane @ 2011-10-15 05:41 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; +Cc: Royce Ausburn <[email protected]>; pgsql-hackers
Yeb Havinga <[email protected]> writes:
> Hello Royce,
> Thanks again for testing.
I looked this patch over but concluded that it's not ready to apply,
mainly because there are too many weird behaviors around error
reporting.
The biggest problem is that the patch cuts up and reassembles the source
text and then hands that off to check_sql_expr, ignoring the latter's
comment that says
* It is assumed that "stmt" represents a copy of the function source text
* beginning at offset "location", with leader text of length "leaderlen"
* (typically "SELECT ") prefixed to the source text. We use this assumption
* to transpose any error cursor position back to the function source text.
This means that syntax error positioning for errors in the argument
expressions is generally pretty wacko, but especially so if the
arguments are supplied in other than left-to-right order. An example is
create or replace function fooey() returns void as $$
declare
c1 cursor (p1 int, p2 int) for
select * from tenk1 where thousand = p1 and tenthous = p2;
begin
open c1 ( p2 := 42/, p1 := 77);
end $$ language plpgsql;
which gives this:
ERROR: syntax error at or near ";"
LINE 6: open c1 ( p2 := 42/, p1 := 77);
^
which is not going to impress anybody as helpful, either as to the message
text (the user didn't write any ";" nearby) or as to the cursor
positioning. And it doesn't look very much more professional if the error
is run-time rather than parse-time:
create or replace function fooey() returns void as $$
declare
c1 cursor (p1 int, p2 int) for
select * from tenk1 where thousand = p1 and tenthous = p2;
begin
open c1 ( p2 := 42/0, p1 := 77);
end $$ language plpgsql;
select fooey();
ERROR: division by zero
CONTEXT: SQL statement "SELECT 77
,42/0;"
PL/pgSQL function "fooey" line 6 at OPEN
where again you're displaying something almost completely unlike what the
user wrote.
I don't have any great suggestions about how to fix this :-(. Perhaps
it'd be acceptable to copy the argument-list text into the query string,
carefully replacing the parameters and := marks with appropriate amounts
of whitespace (the same number of characters, not the same number of
bytes). This would allow syntax errors from check_sql_expr to match up
correctly, and runtime errors would at least show a string that doesn't
look totally unlike what the user wrote. But it would be painful to get
the values assigned to the cursor parameters in the right order --- I
think most likely you'd need to generate a new "row" list having the
cursor parameter values in the order written in the OPEN.
Also, I concur with Royce that it would be a whole lot better to apply
check_sql_expr to each argument expression as soon as you've parsed it
off, so that typos like "p1 : = 42" get detected soon enough to be
helpful, rather than ending up with misleading error messages. Very
possibly that would also simplify getting parse-time syntax errors to
point to the right places, since you'd be calling check_sql_expr on text
not far separated from the source query. (In fact, personally I'd use
read_sql_expression2(',', ')', ...) to parse off each expression and check
it immediately.) Maybe with that fix, it'd be all right if the run-time
query rearranged the expressions into the order required by the cursor
... though I'd still counsel spending more sweat on making the run-time
string look nicer. Something like
ERROR: division by zero
CONTEXT: SQL statement "SELECT /* p1 := */ 77 , /* p2 := */ 42/0"
PL/pgSQL function "fooey" line 6 at OPEN
would be easier for users to make sense of, I think.
By accident I noticed that the parameter name matching is inaccurate,
for instance this fails to fail:
create or replace function fooey() returns void as $$
declare
c1 cursor (p1 int, p2 int) for
select * from tenk1 where thousand = p1 and tenthous = p2;
begin
open c1 ( p1 := 42, p2z := 77);
end $$ language plpgsql;
select fooey();
I think this is because of the use of strncmp() --- is that really needed
rather than just plain strcmp()?
Cursor positioning for some errors is a bit flaky, observe these cases:
ERROR: mixing positional and named parameter assignment not allowed in cursor "c1"
LINE 6: open c1 ( p2 : = 42, p2 := 77);
^
ERROR: cursor "c1" argument 2 "p2" provided multiple times
LINE 6: open c1 ( p2 := 42, p2 := 77);
^
In both of these cases I'd have expected the syntax cursor to point at
the second "p2". I'd lose the "2" in that second message text, as well
--- the cursor argument name is sufficient, and as-is it does not read
very nicely.
On the documentation front, the patch includes a hunk that changes the
description of DECLARE to claim that the argument names are optional,
something I see no support for in the code. It also fails to document
that this patch affects the behavior of cursor FOR loops as well as OPEN,
since both of those places use read_cursor_args().
regards, tom lane
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-10-17 08:44 Yeb Havinga <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 0 replies; 38+ messages in thread
From: Yeb Havinga @ 2011-10-17 08:44 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Royce Ausburn <[email protected]>; pgsql-hackers
On 2011-10-15 07:41, Tom Lane wrote:
> Yeb Havinga<[email protected]> writes:
>> Hello Royce,
>> Thanks again for testing.
> I looked this patch over but concluded that it's not ready to apply,
> mainly because there are too many weird behaviors around error
> reporting.
Tom, thanks for reviewing - getting the syntax errors to be at the exact
location was indeed something that I thought would be near impossible,
however the whitespace suggestion together with the others you made seem
like a good path to go forward. Thanks for taking the time to write your
comments, it will be a great help with making an improved version.
regards,
Yeb Havinga
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-11-14 14:45 Yeb Havinga <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Yeb Havinga @ 2011-11-14 14:45 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Royce Ausburn <[email protected]>; pgsql-hackers
On 2011-10-15 07:41, Tom Lane wrote:
> Yeb Havinga<[email protected]> writes:
>> Hello Royce,
>> Thanks again for testing.
> I looked this patch over but concluded that it's not ready to apply,
> mainly because there are too many weird behaviors around error
> reporting.
Thanks again for the review and comments. Attached is v3 of the patch
that addresses all of the points made by Tom. In the regression test I
added a section under --- START ADDITIONAL TESTS that might speedup testing.
> On the documentation front, the patch includes a hunk that changes the
> description of DECLARE to claim that the argument names are optional,
> something I see no support for in the code. It also fails to document
> that this patch affects the behavior of cursor FOR loops as well as OPEN,
> since both of those places use read_cursor_args().
The declare section was removed. The cursor for loop section was changed
to include a reference to named parameters, however I was unsure about
OPEN as I was under the impression that was already altered.
regards,
Yeb Havinga
Attachments:
[text/x-patch] cursornamedparameter-v3.patch (21.6K, ../../[email protected]/2-cursornamedparameter-v3.patch)
download | inline diff:
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index f33cef5..6a77b75
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** OPEN curs1 FOR EXECUTE 'SELECT * FROM '
*** 2823,2833 ****
</para>
</sect3>
! <sect3>
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional>;
</synopsis>
<para>
--- 2823,2833 ----
</para>
</sect3>
! <sect3 id="plpgsql-open-bound-cursor">
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional>;
</synopsis>
<para>
*************** OPEN <replaceable>bound_cursorvar</repla
*** 2847,2856 ****
--- 2847,2868 ----
</para>
<para>
+ Cursors that have named parameters may be opened using either
+ <firstterm>named</firstterm> or <firstterm>positional</firstterm>
+ notation. In contrast with calling functions, described in <xref
+ linkend="sql-syntax-calling-funcs">, it is not allowed to mix
+ positional and named notation. In positional notation, all arguments
+ are specified in order. In named notation, each argument's name is
+ specified using <literal>:=</literal> to separate it from the
+ argument expression.
+ </para>
+
+ <para>
Examples (these use the cursor declaration examples above):
<programlisting>
OPEN curs2;
OPEN curs3(42);
+ OPEN curs3(key := 42);
</programlisting>
</para>
*************** COMMIT;
*** 3169,3175 ****
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
--- 3181,3187 ----
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
*************** END LOOP <optional> <replaceable>label</
*** 3187,3192 ****
--- 3199,3209 ----
Each row returned by the cursor is successively assigned to this
record variable and the loop body is executed.
</para>
+
+ <para>
+ See <xref linkend="plpgsql-open-bound-cursor"> on calling cursors with
+ named notation.
+ </para>
</sect2>
</sect1>
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
new file mode 100644
index 8c4c2f7..5271ab5
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
*************** static PLpgSQL_expr *read_sql_construct(
*** 67,72 ****
--- 67,73 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken);
static PLpgSQL_expr *read_sql_expression(int until,
*************** for_control : for_variable K_IN
*** 1313,1318 ****
--- 1314,1320 ----
"SELECT ",
true,
false,
+ true,
&expr1loc,
&tok);
*************** stmt_raise : K_RAISE
*** 1692,1698 ****
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
--- 1694,1700 ----
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
*************** stmt_dynexecute : K_EXECUTE
*** 1790,1796 ****
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
--- 1792,1798 ----
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
*************** stmt_dynexecute : K_EXECUTE
*** 1829,1835 ****
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
--- 1831,1837 ----
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
*************** static PLpgSQL_expr *
*** 2322,2328 ****
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
--- 2324,2342 ----
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, true, NULL, NULL);
! }
!
! /*
! * Convenience routine to read a single unchecked expression with two possible
! * terminators, returning an expression with an empty sql prefix.
! */
! static PLpgSQL_expr *
! read_sql_one_expression(int until, int until2, const char *expected,
! int *endtoken)
! {
! return read_sql_construct(until, until2, 0, expected,
! "", true, false, true, NULL, endtoken);
}
/* Convenience routine to read an expression with two possible terminators */
*************** read_sql_expression2(int until, int unti
*** 2331,2337 ****
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
--- 2345,2351 ----
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
*************** static PLpgSQL_expr *
*** 2339,2345 ****
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, NULL, NULL);
}
/*
--- 2353,2359 ----
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, true, NULL, NULL);
}
/*
*************** read_sql_stmt(const char *sqlstart)
*** 2352,2357 ****
--- 2366,2372 ----
* sqlstart: text to prefix to the accumulated SQL text
* isexpression: whether to say we're reading an "expression" or a "statement"
* valid_sql: whether to check the syntax of the expr (prefixed with sqlstart)
+ * bool: trim trailing whitespace
* startloc: if not NULL, location of first token is stored at *startloc
* endtoken: if not NULL, ending token is stored at *endtoken
* (this is only interesting if until2 or until3 isn't zero)
*************** read_sql_construct(int until,
*** 2364,2369 ****
--- 2379,2385 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken)
{
*************** read_sql_construct(int until,
*** 2443,2450 ****
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
--- 2459,2467 ----
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! if (trim)
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
*************** check_labels(const char *start_label, co
*** 3374,3389 ****
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL.
! * If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token. Consume all that and return a SELECT query that
! * evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
! int tok;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
--- 3391,3415 ----
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL. If it
! * does have args, we expect to see "( expr [, expr ...] )" followed by the
! * until token, where expr may be a plain expression, or a named parameter
! * assignment of the form IDENT := expr. Consume all that and return a SELECT
! * query that evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
! PLpgSQL_row *row;
! int tok;
! int argc = 0;
! char **argv;
! StringInfoData ds;
! char *sqlstart = "SELECT ";
! int startlocation = yylloc;
! bool named = false;
! bool positional = false;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3402,3407 ****
--- 3428,3436 ----
return NULL;
}
+ row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+ argv = (char **) palloc0(sizeof(char *) * row->nfields);
+
/* Else better provide arguments */
if (tok != '(')
ereport(ERROR,
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3410,3420 ****
--- 3439,3545 ----
cursor->refname),
parser_errposition(yylloc)));
+ if (false)
+ {
/*
* Read expressions until the matching ')'.
*/
expr = read_sql_expression(')', ")");
+ }
+ else
+ {
+ for (argc = 0; argc < row->nfields; argc++)
+ {
+ int argpos;
+ int endtoken;
+ PLpgSQL_expr *item;
+ if (plpgsql_isidentassign())
+ {
+ /* Named parameter assignment */
+ named = true;
+ for (argpos = 0; argpos < row->nfields; argpos++)
+ if (strcmp(row->fieldnames[argpos], yylval.str) == 0)
+ break;
+
+ if (argpos == row->nfields)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cursor \"%s\" has no argument named \"%s\"",
+ cursor->refname, yylval.str),
+ parser_errposition(yylloc)));
+ }
+ else
+ {
+ /* Positional parameter assignment */
+ positional = true;
+ argpos = argc;
+ }
+
+ /* Read one expression at a time until the matching endtoken. */
+ item = read_sql_construct(',', ')', 0,
+ ",\" or \")",
+ sqlstart,
+ true, true,
+ false, /* do not trim trailing whitespace to keep possible newlines */
+ NULL, &endtoken);
+
+ if (endtoken == ')' && !(argc == row->nfields - 1))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("not enough arguments for cursor \"%s\"",
+ cursor->refname),
+ parser_errposition(yylloc)));
+
+ if (endtoken == ',' && (argc == row->nfields - 1))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("too many arguments for cursor \"%s\"",
+ cursor->refname),
+ parser_errposition(yylloc)));
+
+ if (argv[argpos] != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cursor \"%s\" argument \"%s\" provided multiple times",
+ cursor->refname, row->fieldnames[argpos]),
+ parser_errposition(yylloc)));
+
+ if (named && positional)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("mixing positional and named parameter assignment not allowed in cursor \"%s\"",
+ cursor->refname),
+ parser_errposition(startlocation)));
+
+ argv[argpos] = item->query + strlen(sqlstart);
+ }
+
+ /* Make positional argument list */
+ initStringInfo(&ds);
+ appendStringInfoString(&ds, sqlstart);
+ for (argc = 0; argc < row->nfields; argc++)
+ {
+ Assert(argv[argc] != NULL);
+ if (named)
+ {
+ appendStringInfo(&ds, "/* %s := */ ", row->fieldnames[argc]);
+ }
+ appendStringInfoString(&ds, argv[argc]);
+
+ if (argc < row->nfields - 1)
+ appendStringInfoString(&ds, ", ");
+ }
+ appendStringInfoChar(&ds, ';');
+
+ expr = palloc0(sizeof(PLpgSQL_expr));
+ expr->dtype = PLPGSQL_DTYPE_EXPR;
+ expr->query = pstrdup(ds.data);
+ expr->plan = NULL;
+ expr->paramnos = NULL;
+ expr->ns = plpgsql_ns_top();
+ pfree(ds.data);
+ }
/* Next we'd better find the until token */
tok = yylex();
if (tok != until)
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
new file mode 100644
index 76e8436..9c233c4
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
*************** plpgsql_scanner_finish(void)
*** 583,585 ****
--- 583,617 ----
yyscanner = NULL;
scanorig = NULL;
}
+
+ /*
+ * Return true if 'IDENT' ':=' are the next two tokens
+ */
+ bool
+ plpgsql_isidentassign(void)
+ {
+ int tok1, tok2;
+ TokenAuxData aux1, aux2;
+ bool result = false;
+
+ tok1 = internal_yylex(&aux1);
+ if (tok1 == IDENT)
+ {
+ tok2 = internal_yylex(&aux2);
+
+ if (tok2 == COLON_EQUALS)
+ result = true;
+ else
+ push_back_token(tok2, &aux2);
+ }
+
+ if (!result)
+ push_back_token(tok1, &aux1);
+
+ plpgsql_yylval = aux1.lval;
+ plpgsql_yylloc = aux1.lloc;
+ plpgsql_yyleng = aux1.leng;
+
+ return result;
+ }
+
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
new file mode 100644
index c638f43..0f742e6
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** extern int plpgsql_location_to_lineno(in
*** 968,973 ****
--- 968,974 ----
extern int plpgsql_latest_lineno(void);
extern void plpgsql_scanner_init(const char *str);
extern void plpgsql_scanner_finish(void);
+ extern bool plpgsql_isidentassign(void);
/* ----------
* Externs in gram.y
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
new file mode 100644
index fc9d401..fe1db4c
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** select refcursor_test2(20000, 20000) as
*** 2292,2297 ****
--- 2292,2398 ----
(1 row)
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ -- should fail
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+ ERROR: mixing positional and named parameter assignment not allowed in cursor "c1"
+ LINE 6: open c1(param1 := $1, $2);
+ ^
+ -- should fail
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+ ERROR: not enough arguments for cursor "c1"
+ LINE 5: open c1(param2 := $2);
+ ^
+ -- START ADDITIONAL TESTS
+ -- test runtime error
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test comment and newline structure
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 7: , 42/);
+ ^
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test syntax error
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77/);
+ end $$ language plpgsql;
+ ERROR: syntax error at or near ":"
+ LINE 6: open c1 ( p2 := 42, p1 : = 77/);
+ ^
+ -- test another syntax error
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77/);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 6: open c1 ( p2 := 42/, p1 : = 77/);
+ ^
+ -- END ADDITIONAL TESTS
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
new file mode 100644
index 2906943..9233aa7
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select refcursor_test2(20000, 20000) as
*** 1946,1951 ****
--- 1946,2035 ----
refcursor_test2(20, 20) as "Should be true";
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+
+ -- should fail
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+
+ -- should fail
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+
+ -- START ADDITIONAL TESTS
+ -- test runtime error
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test comment and newline structure, will not give runtime error when read_sql_construct trims trailing whitespace
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test syntax error
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77);
+ end $$ language plpgsql;
+
+ -- test another syntax error
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77);
+ end $$ language plpgsql;
+
+ -- END ADDITIONAL TESTS
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-11-15 10:17 Yeb Havinga <[email protected]>
parent: Yeb Havinga <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Yeb Havinga @ 2011-11-15 10:17 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Royce Ausburn <[email protected]>; pgsql-hackers
On 2011-11-14 15:45, Yeb Havinga wrote:
> On 2011-10-15 07:41, Tom Lane wrote:
>> Yeb Havinga<[email protected]> writes:
>>> Hello Royce,
>>> Thanks again for testing.
>> I looked this patch over but concluded that it's not ready to apply,
>> mainly because there are too many weird behaviors around error
>> reporting.
>
> Thanks again for the review and comments. Attached is v3 of the patch
> that addresses all of the points made by Tom. In the regression test I
> added a section under --- START ADDITIONAL TESTS that might speedup
> testing.
Please disregard the previous patch: besides that it contained an unused
function, it turned out my statement that all of Tom's points were
addressed was not true - the attached patch fixes the remaining issue of
putting two kinds of errors at the correct start of the current argument
location.
I also put some more comments in the regression test section: mainly to
assist providing testcases for review, not for permanent inclusion.
To address a corner case of the form 'p1 := 1 -- comments\n, p2 := 2' it
was necessary to have read_sql_construct not trim trailing whitespace,
since that results in an expression of the form '1 -- comments, 2' which
is wrong.
regards,
Yeb Havinga
Attachments:
[text/x-patch] cursornamedparameter-v4.patch (24.4K, ../../[email protected]/2-cursornamedparameter-v4.patch)
download | inline diff:
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index f33cef5..17c04d2
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** OPEN curs1 FOR EXECUTE 'SELECT * FROM '
*** 2823,2833 ****
</para>
</sect3>
! <sect3>
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional>;
</synopsis>
<para>
--- 2823,2833 ----
</para>
</sect3>
! <sect3 id="plpgsql-open-bound-cursor">
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional>;
</synopsis>
<para>
*************** OPEN <replaceable>bound_cursorvar</repla
*** 2847,2856 ****
--- 2847,2867 ----
</para>
<para>
+ Cursors may be opened using either <firstterm>named</firstterm> or
+ <firstterm>positional</firstterm> notation. In contrast with calling
+ functions, described in <xref linkend="sql-syntax-calling-funcs">, it
+ is not allowed to mix positional and named notation. In positional
+ notation, all arguments are specified in order. In named notation,
+ each argument's name is specified using <literal>:=</literal> to
+ separate it from the argument expression.
+ </para>
+
+ <para>
Examples (these use the cursor declaration examples above):
<programlisting>
OPEN curs2;
OPEN curs3(42);
+ OPEN curs3(key := 42);
</programlisting>
</para>
*************** COMMIT;
*** 3169,3186 ****
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
The cursor variable must have been bound to some query when it was
! declared, and it <emphasis>cannot</> be open already. The
! <command>FOR</> statement automatically opens the cursor, and it closes
! the cursor again when the loop exits. A list of actual argument value
! expressions must appear if and only if the cursor was declared to take
! arguments. These values will be substituted in the query, in just
! the same way as during an <command>OPEN</>.
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
--- 3180,3203 ----
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
The cursor variable must have been bound to some query when it was
! declared, and it <emphasis>cannot</> be open already. The <command>FOR</>
! statement automatically opens the cursor, and it closes the cursor again
! when the loop exits. The cursors arguments may be supplied in either
! <firstterm>named</firstterm> or <firstterm>positional</firstterm>
! notation. A list of actual argument value expressions must appear if and
! only if the cursor was declared to take arguments. These values will be
! substituted in the query, in just the same way as during an
! <command>OPEN</command> described in <xref
! linkend="plpgsql-open-bound-cursor">.
! </para>
!
! <para>
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
new file mode 100644
index 8c4c2f7..19f8da8
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
*************** static PLpgSQL_expr *read_sql_construct(
*** 67,72 ****
--- 67,73 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken);
static PLpgSQL_expr *read_sql_expression(int until,
*************** for_control : for_variable K_IN
*** 1313,1318 ****
--- 1314,1320 ----
"SELECT ",
true,
false,
+ true,
&expr1loc,
&tok);
*************** stmt_raise : K_RAISE
*** 1692,1698 ****
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
--- 1694,1700 ----
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
*************** stmt_dynexecute : K_EXECUTE
*** 1790,1796 ****
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
--- 1792,1798 ----
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
*************** stmt_dynexecute : K_EXECUTE
*** 1829,1835 ****
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
--- 1831,1837 ----
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
*************** static PLpgSQL_expr *
*** 2322,2328 ****
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
--- 2324,2330 ----
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
*************** read_sql_expression2(int until, int unti
*** 2331,2337 ****
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
--- 2333,2339 ----
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
*************** static PLpgSQL_expr *
*** 2339,2345 ****
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, NULL, NULL);
}
/*
--- 2341,2347 ----
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, true, NULL, NULL);
}
/*
*************** read_sql_stmt(const char *sqlstart)
*** 2352,2357 ****
--- 2354,2360 ----
* sqlstart: text to prefix to the accumulated SQL text
* isexpression: whether to say we're reading an "expression" or a "statement"
* valid_sql: whether to check the syntax of the expr (prefixed with sqlstart)
+ * bool: trim trailing whitespace
* startloc: if not NULL, location of first token is stored at *startloc
* endtoken: if not NULL, ending token is stored at *endtoken
* (this is only interesting if until2 or until3 isn't zero)
*************** read_sql_construct(int until,
*** 2364,2369 ****
--- 2367,2373 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken)
{
*************** read_sql_construct(int until,
*** 2443,2450 ****
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
--- 2447,2455 ----
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! if (trim)
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
*************** check_labels(const char *start_label, co
*** 3374,3389 ****
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL.
! * If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token. Consume all that and return a SELECT query that
! * evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
! PLpgSQL_expr *expr;
! int tok;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
--- 3379,3402 ----
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL. If it
! * does have args, we expect to see "( expr [, expr ...] )" followed by the
! * until token, where expr may be a plain expression, or a named parameter
! * assignment of the form IDENT := expr. Consume all that and return a SELECT
! * query that evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
! PLpgSQL_expr *expr;
! PLpgSQL_row *row;
! int tok;
! int argc = 0;
! char **argv;
! StringInfoData ds;
! char *sqlstart = "SELECT ";
! bool named = false;
! bool positional = false;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3402,3407 ****
--- 3415,3423 ----
return NULL;
}
+ row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+ argv = (char **) palloc0(sizeof(char *) * row->nfields);
+
/* Else better provide arguments */
if (tok != '(')
ereport(ERROR,
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3410,3419 ****
cursor->refname),
parser_errposition(yylloc)));
! /*
! * Read expressions until the matching ')'.
! */
! expr = read_sql_expression(')', ")");
/* Next we'd better find the until token */
tok = yylex();
--- 3426,3530 ----
cursor->refname),
parser_errposition(yylloc)));
! for (argc = 0; argc < row->nfields; argc++)
! {
! int argpos;
! int endtoken;
! PLpgSQL_expr *item;
! int arglocation;
!
! if (plpgsql_isidentassign(&arglocation))
! {
! /* Named parameter assignment */
! named = true;
! for (argpos = 0; argpos < row->nfields; argpos++)
! if (strcmp(row->fieldnames[argpos], yylval.str) == 0)
! break;
!
! if (argpos == row->nfields)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" has no argument named \"%s\"",
! cursor->refname, yylval.str),
! parser_errposition(yylloc)));
! }
! else
! {
! /* Positional parameter assignment */
! positional = true;
! argpos = argc;
! }
!
! /*
! * Read one expression at a time until the matching endtoken. To
! * provide the user with meaningful parse error positions, we check the
! * syntax immediately, instead of checking the final expression that
! * may have a permutated argument list. Also trailing whitespace may
! * not be trimmed, since otherwise input of the form (param --
! * comment\n, param) is translated into a form that comments the
! * consecutive parameters.
! */
! item = read_sql_construct(',', ')', 0,
! ",\" or \")",
! sqlstart,
! true, true,
! false, /* do not trim */
! NULL, &endtoken);
!
! if (endtoken == ')' && !(argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("not enough arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (endtoken == ',' && (argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("too many arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (argv[argpos] != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" argument \"%s\" provided multiple times",
! cursor->refname, row->fieldnames[argpos]),
! parser_errposition(arglocation)));
!
! if (named && positional)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("mixing positional and named parameter assignment not allowed in cursor \"%s\"",
! cursor->refname),
! parser_errposition(arglocation)));
!
! argv[argpos] = item->query + strlen(sqlstart);
! }
!
! /* Make positional argument list */
! initStringInfo(&ds);
! appendStringInfoString(&ds, sqlstart);
! for (argc = 0; argc < row->nfields; argc++)
! {
! Assert(argv[argc] != NULL);
! /* Argument name included for meaningful runtime errors */
! if (named)
! appendStringInfo(&ds, "/* %s := */ ", row->fieldnames[argc]);
!
! appendStringInfoString(&ds, argv[argc]);
! if (argc < row->nfields - 1)
! appendStringInfoString(&ds, ", ");
! }
! appendStringInfoChar(&ds, ';');
!
! expr = palloc0(sizeof(PLpgSQL_expr));
! expr->dtype = PLPGSQL_DTYPE_EXPR;
! expr->query = pstrdup(ds.data);
! expr->plan = NULL;
! expr->paramnos = NULL;
! expr->ns = plpgsql_ns_top();
! pfree(ds.data);
/* Next we'd better find the until token */
tok = yylex();
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
new file mode 100644
index 76e8436..ce3161c
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
*************** plpgsql_scanner_finish(void)
*** 583,585 ****
--- 583,622 ----
yyscanner = NULL;
scanorig = NULL;
}
+
+ /*
+ * Return true if 'IDENT' ':=' are the next two tokens
+ *
+ * startloc: if not NULL, location of first token is stored at *startloc
+ */
+ bool
+ plpgsql_isidentassign(int *startloc)
+ {
+ int tok1, tok2;
+ TokenAuxData aux1, aux2;
+ bool result = false;
+
+ tok1 = internal_yylex(&aux1);
+ if (startloc)
+ *startloc = aux1.lloc;
+
+ if (tok1 == IDENT)
+ {
+ tok2 = internal_yylex(&aux2);
+
+ if (tok2 == COLON_EQUALS)
+ result = true;
+ else
+ push_back_token(tok2, &aux2);
+ }
+
+ if (!result)
+ push_back_token(tok1, &aux1);
+
+ plpgsql_yylval = aux1.lval;
+ plpgsql_yylloc = aux1.lloc;
+ plpgsql_yyleng = aux1.leng;
+
+ return result;
+ }
+
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
new file mode 100644
index c638f43..b3ff847
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** extern int plpgsql_location_to_lineno(in
*** 968,973 ****
--- 968,974 ----
extern int plpgsql_latest_lineno(void);
extern void plpgsql_scanner_init(const char *str);
extern void plpgsql_scanner_finish(void);
+ extern bool plpgsql_isidentassign(int *startloc);
/* ----------
* Externs in gram.y
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
new file mode 100644
index fc9d401..bf31a9a
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** select refcursor_test2(20000, 20000) as
*** 2292,2297 ****
--- 2292,2412 ----
(1 row)
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ -- may not mix named and positional, parse time error at $2
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+ ERROR: mixing positional and named parameter assignment not allowed in cursor "c1"
+ LINE 6: open c1(param1 := $1, $2);
+ ^
+ -- not enough parameters, parse time error at );
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+ ERROR: not enough arguments for cursor "c1"
+ LINE 5: open c1(param2 := $2);
+ ^
+ -- START ADDITIONAL TESTS
+ -- multiple same parameter, parse time error at second p2
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end $$ language plpgsql;
+ ERROR: cursor "c1" argument "p2" provided multiple times
+ LINE 6: open c1 (p2 := 77, p2 := 42);
+ ^
+ -- division by zero runtime error,
+ -- provides context users can make sense of:
+ -- CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test comment and newline structure, will not give runtime error when
+ -- read_sql_construct trims trailing whitespace
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 7: , 42/);
+ ^
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test syntax error at :
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77);
+ end $$ language plpgsql;
+ ERROR: syntax error at or near ":"
+ LINE 6: open c1 ( p2 := 42, p1 : = 77);
+ ^
+ -- test another syntax error at ,
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 6: open c1 ( p2 := 42/, p1 : = 77);
+ ^
+ -- END ADDITIONAL TESTS
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
new file mode 100644
index 2906943..bad7f05
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select refcursor_test2(20000, 20000) as
*** 1946,1951 ****
--- 1946,2047 ----
refcursor_test2(20, 20) as "Should be true";
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+
+ -- may not mix named and positional, parse time error at $2
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+
+ -- not enough parameters, parse time error at );
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+
+ -- START ADDITIONAL TESTS
+ -- multiple same parameter, parse time error at second p2
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end $$ language plpgsql;
+
+ -- division by zero runtime error,
+ -- provides context users can make sense of:
+ -- CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test comment and newline structure, will not give runtime error when
+ -- read_sql_construct trims trailing whitespace
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test syntax error at :
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77);
+ end $$ language plpgsql;
+
+ -- test another syntax error at ,
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77);
+ end $$ language plpgsql;
+
+ -- END ADDITIONAL TESTS
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-03 20:53 Kevin Grittner <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Kevin Grittner @ 2011-12-03 20:53 UTC (permalink / raw)
To: [email protected]; pgsql-hackers
The patch is in context format, includes docs and additional
regression tests, applies cleanly, passes "world" regression tests.
The parser changes don't cause any "backing up".
Discussion on the list seems to have reached a consensus that this
patch implements a useful feature. A previous version was reviewed.
This version attempts to respond to points previously raised.
Overall, it looked good, but there were a few things I would like Yeb
to address before mark it is marked ready for committer. I don't
think any of these will take long.
(1) Doc changes:
(a) These contain some unnecessary whitespace changes which make it
harder to see exactly what changed.
(b) There's one point where "cursors" should be possessive
"cursor's".
(c) I think it would be less confusing to be consistent about
mentioning "positional" and "named" in the same order each
time. Mixing it up like that is harder to read, at least for
me.
(2) The error for mixing positional and named parameters should be
moved up. That seems more important than "too many arguments" or
"provided multiple times" if both are true.
(3) The "-- END ADDITIONAL TESTS" comment shouldn't be added to the
regression tests.
The way this parses out the named parameters and then builds the
positional list, with some code from read_sql_construct() repeated in
read_cursor_args() seems a little bit clumsy to me, but I couldn't
think of a better way to do it.
-Kevin
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-05 10:06 Yeb Havinga <[email protected]>
parent: Kevin Grittner <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Yeb Havinga @ 2011-12-05 10:06 UTC (permalink / raw)
To: Kevin Grittner <[email protected]>; +Cc: pgsql-hackers
Kevin,
Thank you for your review!
On 2011-12-03 21:53, Kevin Grittner wrote:
>
> (1) Doc changes:
>
> (a) These contain some unnecessary whitespace changes which make it
> harder to see exactly what changed.
This is probably caused because I used emacs's fill-paragraph (alt+q)
command, after some changes. If this is against policy, I could change
the additions in such a way as to cause minor differences, however I
believe that the benefit of that ends immediately after review.
> (b) There's one point where "cursors" should be possessive
> "cursor's".
>
> (c) I think it would be less confusing to be consistent about
> mentioning "positional" and "named" in the same order each
> time. Mixing it up like that is harder to read, at least for
> me.
Good point, both changed.
> (2) The error for mixing positional and named parameters should be
> moved up. That seems more important than "too many arguments" or
> "provided multiple times" if both are true.
I moved the error up, though I personally tend to believe it doesn't
even need to be an error. There is no technical reason not to allow it.
All the user needs to do is make sure that the combination of named
parameters and the positional ones together are complete and not
overlapping. This is also in line with calling functions, where mixing
named and positional is allowed, as long as the positional arguments are
first. Personally I have no opinion what is best, include the feature or
give an error, and I removed the possibility during the previous commitfest.
> (3) The "-- END ADDITIONAL TESTS" comment shouldn't be added to the
> regression tests.
This is removed, also the -- START ADDITIONAL TESTS, though I kept the
tests between those to comments.
> The way this parses out the named parameters and then builds the
> positional list, with some code from read_sql_construct() repeated in
> read_cursor_args() seems a little bit clumsy to me, but I couldn't
> think of a better way to do it.
Same here.
The new patch is attached.
regards
Yeb Havinga
Attachments:
[text/x-patch] cursornamedparameter-v5.patch (24.3K, ../../[email protected]/2-cursornamedparameter-v5.patch)
download | inline diff:
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index f33cef5..a3e6540
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** OPEN curs1 FOR EXECUTE 'SELECT * FROM '
*** 2823,2833 ****
</para>
</sect3>
! <sect3>
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional>;
</synopsis>
<para>
--- 2823,2833 ----
</para>
</sect3>
! <sect3 id="plpgsql-open-bound-cursor">
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional>;
</synopsis>
<para>
*************** OPEN <replaceable>bound_cursorvar</repla
*** 2847,2856 ****
--- 2847,2867 ----
</para>
<para>
+ Cursors may be opened using either <firstterm>positional</firstterm>
+ or <firstterm>named</firstterm> notation. In contrast with calling
+ functions, described in <xref linkend="sql-syntax-calling-funcs">, it
+ is not allowed to mix positional and named notation. In positional
+ notation, all arguments are specified in order. In named notation,
+ each argument's name is specified using <literal>:=</literal> to
+ separate it from the argument expression.
+ </para>
+
+ <para>
Examples (these use the cursor declaration examples above):
<programlisting>
OPEN curs2;
OPEN curs3(42);
+ OPEN curs3(key := 42);
</programlisting>
</para>
*************** COMMIT;
*** 3169,3186 ****
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
The cursor variable must have been bound to some query when it was
! declared, and it <emphasis>cannot</> be open already. The
! <command>FOR</> statement automatically opens the cursor, and it closes
! the cursor again when the loop exits. A list of actual argument value
! expressions must appear if and only if the cursor was declared to take
! arguments. These values will be substituted in the query, in just
! the same way as during an <command>OPEN</>.
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
--- 3180,3203 ----
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
The cursor variable must have been bound to some query when it was
! declared, and it <emphasis>cannot</> be open already. The <command>FOR</>
! statement automatically opens the cursor, and it closes the cursor again
! when the loop exits. The cursor's arguments may be supplied in either
! <firstterm>positional</firstterm> or <firstterm>named</firstterm>
! notation. A list of actual argument value expressions must appear if and
! only if the cursor was declared to take arguments. These values will be
! substituted in the query, in just the same way as during an
! <command>OPEN</command> described in <xref
! linkend="plpgsql-open-bound-cursor">.
! </para>
!
! <para>
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
new file mode 100644
index 8c4c2f7..f7d6b9d
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
*************** static PLpgSQL_expr *read_sql_construct(
*** 67,72 ****
--- 67,73 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken);
static PLpgSQL_expr *read_sql_expression(int until,
*************** for_control : for_variable K_IN
*** 1313,1318 ****
--- 1314,1320 ----
"SELECT ",
true,
false,
+ true,
&expr1loc,
&tok);
*************** stmt_raise : K_RAISE
*** 1692,1698 ****
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
--- 1694,1700 ----
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
*************** stmt_dynexecute : K_EXECUTE
*** 1790,1796 ****
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
--- 1792,1798 ----
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
*************** stmt_dynexecute : K_EXECUTE
*** 1829,1835 ****
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
--- 1831,1837 ----
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
*************** static PLpgSQL_expr *
*** 2322,2328 ****
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
--- 2324,2330 ----
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
*************** read_sql_expression2(int until, int unti
*** 2331,2337 ****
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
--- 2333,2339 ----
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
*************** static PLpgSQL_expr *
*** 2339,2345 ****
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, NULL, NULL);
}
/*
--- 2341,2347 ----
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, true, NULL, NULL);
}
/*
*************** read_sql_stmt(const char *sqlstart)
*** 2352,2357 ****
--- 2354,2360 ----
* sqlstart: text to prefix to the accumulated SQL text
* isexpression: whether to say we're reading an "expression" or a "statement"
* valid_sql: whether to check the syntax of the expr (prefixed with sqlstart)
+ * bool: trim trailing whitespace
* startloc: if not NULL, location of first token is stored at *startloc
* endtoken: if not NULL, ending token is stored at *endtoken
* (this is only interesting if until2 or until3 isn't zero)
*************** read_sql_construct(int until,
*** 2364,2369 ****
--- 2367,2373 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken)
{
*************** read_sql_construct(int until,
*** 2443,2450 ****
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
--- 2447,2455 ----
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! if (trim)
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
*************** check_labels(const char *start_label, co
*** 3374,3389 ****
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL.
! * If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token. Consume all that and return a SELECT query that
! * evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
! PLpgSQL_expr *expr;
! int tok;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
--- 3379,3402 ----
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL. If it
! * does have args, we expect to see "( expr [, expr ...] )" followed by the
! * until token, where expr may be a plain expression, or a named parameter
! * assignment of the form IDENT := expr. Consume all that and return a SELECT
! * query that evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
! PLpgSQL_expr *expr;
! PLpgSQL_row *row;
! int tok;
! int argc = 0;
! char **argv;
! StringInfoData ds;
! char *sqlstart = "SELECT ";
! bool positional = false;
! bool named = false;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3402,3407 ****
--- 3415,3423 ----
return NULL;
}
+ row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+ argv = (char **) palloc0(sizeof(char *) * row->nfields);
+
/* Else better provide arguments */
if (tok != '(')
ereport(ERROR,
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3410,3419 ****
cursor->refname),
parser_errposition(yylloc)));
! /*
! * Read expressions until the matching ')'.
! */
! expr = read_sql_expression(')', ")");
/* Next we'd better find the until token */
tok = yylex();
--- 3426,3531 ----
cursor->refname),
parser_errposition(yylloc)));
! for (argc = 0; argc < row->nfields; argc++)
! {
! int argpos;
! int endtoken;
! PLpgSQL_expr *item;
! int arglocation;
!
! if (plpgsql_isidentassign(&arglocation))
! {
! /* Named parameter assignment */
! named = true;
! for (argpos = 0; argpos < row->nfields; argpos++)
! if (strcmp(row->fieldnames[argpos], yylval.str) == 0)
! break;
!
! if (argpos == row->nfields)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" has no argument named \"%s\"",
! cursor->refname, yylval.str),
! parser_errposition(yylloc)));
! }
! else
! {
! /* Positional parameter assignment */
! positional = true;
! argpos = argc;
! }
!
! /*
! * Read one expression at a time until the matching endtoken. To
! * provide the user with meaningful parse error positions, we check the
! * syntax immediately, instead of checking the final expression that
! * may have a permutated argument list. Also trailing whitespace may
! * not be trimmed, since otherwise input of the form (param --
! * comment\n, param) is translated into a form that comments the
! * remaining parameters.
! */
! item = read_sql_construct(',', ')', 0,
! ",\" or \")",
! sqlstart,
! true, true,
! false, /* do not trim */
! NULL, &endtoken);
!
! if (endtoken == ')' && !(argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("not enough arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (named && positional)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("mixing positional and named parameter assignment not allowed in cursor \"%s\"",
! cursor->refname),
! parser_errposition(arglocation)));
!
! if (endtoken == ',' && (argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("too many arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (argv[argpos] != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" argument \"%s\" provided multiple times",
! cursor->refname, row->fieldnames[argpos]),
! parser_errposition(arglocation)));
!
! argv[argpos] = item->query + strlen(sqlstart);
! }
!
! /* Make positional argument list */
! initStringInfo(&ds);
! appendStringInfoString(&ds, sqlstart);
! for (argc = 0; argc < row->nfields; argc++)
! {
! Assert(argv[argc] != NULL);
!
! /* Argument name included for meaningful runtime errors */
! if (named)
! appendStringInfo(&ds, "/* %s := */ ", row->fieldnames[argc]);
!
! appendStringInfoString(&ds, argv[argc]);
! if (argc < row->nfields - 1)
! appendStringInfoString(&ds, ", ");
! }
! appendStringInfoChar(&ds, ';');
!
! expr = palloc0(sizeof(PLpgSQL_expr));
! expr->dtype = PLPGSQL_DTYPE_EXPR;
! expr->query = pstrdup(ds.data);
! expr->plan = NULL;
! expr->paramnos = NULL;
! expr->ns = plpgsql_ns_top();
! pfree(ds.data);
/* Next we'd better find the until token */
tok = yylex();
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
new file mode 100644
index 76e8436..ce3161c
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
*************** plpgsql_scanner_finish(void)
*** 583,585 ****
--- 583,622 ----
yyscanner = NULL;
scanorig = NULL;
}
+
+ /*
+ * Return true if 'IDENT' ':=' are the next two tokens
+ *
+ * startloc: if not NULL, location of first token is stored at *startloc
+ */
+ bool
+ plpgsql_isidentassign(int *startloc)
+ {
+ int tok1, tok2;
+ TokenAuxData aux1, aux2;
+ bool result = false;
+
+ tok1 = internal_yylex(&aux1);
+ if (startloc)
+ *startloc = aux1.lloc;
+
+ if (tok1 == IDENT)
+ {
+ tok2 = internal_yylex(&aux2);
+
+ if (tok2 == COLON_EQUALS)
+ result = true;
+ else
+ push_back_token(tok2, &aux2);
+ }
+
+ if (!result)
+ push_back_token(tok1, &aux1);
+
+ plpgsql_yylval = aux1.lval;
+ plpgsql_yylloc = aux1.lloc;
+ plpgsql_yyleng = aux1.leng;
+
+ return result;
+ }
+
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
new file mode 100644
index c638f43..b3ff847
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** extern int plpgsql_location_to_lineno(in
*** 968,973 ****
--- 968,974 ----
extern int plpgsql_latest_lineno(void);
extern void plpgsql_scanner_init(const char *str);
extern void plpgsql_scanner_finish(void);
+ extern bool plpgsql_isidentassign(int *startloc);
/* ----------
* Externs in gram.y
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
new file mode 100644
index 238bf5f..8516466
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** select refcursor_test2(20000, 20000) as
*** 2292,2297 ****
--- 2292,2410 ----
(1 row)
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ -- may not mix named and positional, parse time error at $2
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+ ERROR: mixing positional and named parameter assignment not allowed in cursor "c1"
+ LINE 6: open c1(param1 := $1, $2);
+ ^
+ -- not enough parameters, parse time error at );
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+ ERROR: not enough arguments for cursor "c1"
+ LINE 5: open c1(param2 := $2);
+ ^
+ -- multiple same parameter, parse time error at second p2
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end $$ language plpgsql;
+ ERROR: cursor "c1" argument "p2" provided multiple times
+ LINE 6: open c1 (p2 := 77, p2 := 42);
+ ^
+ -- division by zero runtime error,
+ -- provides context users can make sense of:
+ -- CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test comment and newline structure, will not give runtime error when
+ -- read_sql_construct trims trailing whitespace
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 7: , 42/);
+ ^
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test syntax error at :
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77);
+ end $$ language plpgsql;
+ ERROR: syntax error at or near ":"
+ LINE 6: open c1 ( p2 := 42, p1 : = 77);
+ ^
+ -- test another syntax error at ,
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 6: open c1 ( p2 := 42/, p1 : = 77);
+ ^
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
new file mode 100644
index b47c2de..749aa46
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select refcursor_test2(20000, 20000) as
*** 1946,1951 ****
--- 1946,2044 ----
refcursor_test2(20, 20) as "Should be true";
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+
+ -- may not mix named and positional, parse time error at $2
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+
+ -- not enough parameters, parse time error at );
+ create function namedparmcursor_test6(int, int) returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+
+ -- multiple same parameter, parse time error at second p2
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end $$ language plpgsql;
+
+ -- division by zero runtime error,
+ -- provides context users can make sense of:
+ -- CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test comment and newline structure, will not give runtime error when
+ -- read_sql_construct trims trailing whitespace
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test syntax error at :
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77);
+ end $$ language plpgsql;
+
+ -- test another syntax error at ,
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77);
+ end $$ language plpgsql;
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-05 15:34 Kevin Grittner <[email protected]>
parent: Yeb Havinga <[email protected]>
0 siblings, 2 replies; 38+ messages in thread
From: Kevin Grittner @ 2011-12-05 15:34 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; +Cc: pgsql-hackers
Yeb Havinga <[email protected]> wrote:
> On 2011-12-03 21:53, Kevin Grittner wrote:
>> (1) Doc changes:
>>
>> (a) These contain some unnecessary whitespace changes which
>> make it harder to see exactly what changed.
>
> This is probably caused because I used emacs's fill-paragraph
> (alt+q) command, after some changes. If this is against policy, I
> could change the additions in such a way as to cause minor
> differences, however I believe that the benefit of that ends
> immediately after review.
I don't know whether it's a hard policy, but people usually minimize
whitespace changes in such situations, to make it easier for the
reviewer and committer to tell what really changed. The committer
can always reformat after looking that over, if they feel that's
useful. The issue is small enough here that I'm not inclined to
consider it a blocker for sending to the committer.
>> (2) The error for mixing positional and named parameters should
>> be moved up. That seems more important than "too many arguments"
>> or "provided multiple times" if both are true.
>
> I moved the error up, though I personally tend to believe it
> doesn't even need to be an error. There is no technical reason not
> to allow it. All the user needs to do is make sure that the
> combination of named parameters and the positional ones together
> are complete and not overlapping. This is also in line with
> calling functions, where mixing named and positional is allowed,
> as long as the positional arguments are first. Personally I have
> no opinion what is best, include the feature or give an error, and
> I removed the possibility during the previous commitfest.
If there's no technical reason not to allow them to be mixed, I
would tend to favor consistency with parameter calling rules. Doing
otherwise seems like to result in confusion and "bug" reports.
How do others feel?
-Kevin
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-05 17:09 Kevin Grittner <[email protected]>
parent: Kevin Grittner <[email protected]>
1 sibling, 0 replies; 38+ messages in thread
From: Kevin Grittner @ 2011-12-05 17:09 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; Kevin Grittner <[email protected]>; +Cc: pgsql-hackers
"Kevin Grittner" <[email protected]> wrote:
> Yeb Havinga <[email protected]> wrote:
>> I personally tend to believe it doesn't even need to be an error.
>> There is no technical reason not to allow it. All the user needs
>> to do is make sure that the combination of named parameters and
>> the positional ones together are complete and not overlapping.
>> This is also in line with calling functions, where mixing named
>> and positional is allowed, as long as the positional arguments
>> are first. Personally I have no opinion what is best, include the
>> feature or give an error, and I removed the possibility during
>> the previous commitfest.
>
> If there's no technical reason not to allow them to be mixed, I
> would tend to favor consistency with parameter calling rules.
> Doing otherwise seems like to result in confusion and "bug"
> reports.
Er, that was supposed to read "I would tend to favor consistency
with function calling rules." As stated here:
http://www.postgresql.org/docs/9.1/interactive/sql-syntax-calling-funcs.html
| PostgreSQL also supports mixed notation, which combines positional
| and named notation. In this case, positional parameters are
| written first and named parameters appear after them.
> How do others feel?
If there are no objections, I suggest that Yeb implement the mixed
notation for cursor parameters.
-Kevin
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-06 16:58 Kevin Grittner <[email protected]>
parent: Kevin Grittner <[email protected]>
1 sibling, 2 replies; 38+ messages in thread
From: Kevin Grittner @ 2011-12-06 16:58 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; +Cc: pgsql-hackers
Kevin Grittner <[email protected]> wrote:
> Yeb Havinga <[email protected]> wrote:
>> I personally tend to believe it doesn't even need to be an error.
>> There is no technical reason not to allow it. All the user needs
>> to do is make sure that the combination of named parameters and
>> the positional ones together are complete and not overlapping.
> If there are no objections, I suggest that Yeb implement the mixed
> notation for cursor parameters.
Hearing no objections -- Yeb, are you OK with doing this, and do you
feel this is doable for this CF?
-Kevin
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-07 09:00 Yeb Havinga <[email protected]>
parent: Kevin Grittner <[email protected]>
1 sibling, 0 replies; 38+ messages in thread
From: Yeb Havinga @ 2011-12-07 09:00 UTC (permalink / raw)
To: Kevin Grittner <[email protected]>; +Cc: pgsql-hackers
On 2011-12-06 17:58, Kevin Grittner wrote:
> Kevin Grittner<[email protected]> wrote:
>> If there are no objections, I suggest that Yeb implement the mixed
>> notation for cursor parameters.
>
> Hearing no objections -- Yeb, are you OK with doing this, and do you
> feel this is doable for this CF?
It is not a large change, I'll be able to do it tomorrow if nothing
unexpected happens, or monday otherwise.
regards,
Yeb
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-11 15:26 Yeb Havinga <[email protected]>
parent: Kevin Grittner <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Yeb Havinga @ 2011-12-11 15:26 UTC (permalink / raw)
To: Kevin Grittner <[email protected]>; +Cc: pgsql-hackers
On 2011-12-06 17:58, Kevin Grittner wrote:
> Kevin Grittner<[email protected]> wrote:
>> Yeb Havinga<[email protected]> wrote:
>
>>> I personally tend to believe it doesn't even need to be an error.
>>> There is no technical reason not to allow it. All the user needs
>>> to do is make sure that the combination of named parameters and
>>> the positional ones together are complete and not overlapping.
>
>> If there are no objections, I suggest that Yeb implement the mixed
>> notation for cursor parameters.
>
> Hearing no objections -- Yeb, are you OK with doing this, and do you
> feel this is doable for this CF?
Attach is v6 of the patch, allowing mixed mode and with new test cases
in the regression tests. One difference with calling functions remains:
it is allowed to place positional arguments after named parameters.
Including that would add code, but nothing would be gained.
regards,
Yeb Havinga
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-12 12:32 Yeb Havinga <[email protected]>
parent: Yeb Havinga <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Yeb Havinga @ 2011-12-12 12:32 UTC (permalink / raw)
To: Kevin Grittner <[email protected]>; +Cc: pgsql-hackers
On 2011-12-11 16:26, Yeb Havinga wrote:
> On 2011-12-06 17:58, Kevin Grittner wrote:
>> Kevin Grittner<[email protected]> wrote:
>>> Yeb Havinga<[email protected]> wrote:
>>
>>>> I personally tend to believe it doesn't even need to be an error.
>>>> There is no technical reason not to allow it. All the user needs
>>>> to do is make sure that the combination of named parameters and
>>>> the positional ones together are complete and not overlapping.
>>
>>> If there are no objections, I suggest that Yeb implement the mixed
>>> notation for cursor parameters.
>>
>> Hearing no objections -- Yeb, are you OK with doing this, and do you
>> feel this is doable for this CF?
>
> Attach is v6 of the patch, allowing mixed mode and with new test cases
> in the regression tests. One difference with calling functions
> remains: it is allowed to place positional arguments after named
> parameters. Including that would add code, but nothing would be gained.
Forgot to copy regression output to expected - attached v7 fixes that.
-- Yeb
Attachments:
[text/x-patch] cursornamedparameter-v7.patch (25.4K, ../../[email protected]/2-cursornamedparameter-v7.patch)
download | inline diff:
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index f33cef5..ee2e3a3
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** OPEN curs1 FOR EXECUTE 'SELECT * FROM '
*** 2823,2833 ****
</para>
</sect3>
! <sect3>
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional>;
</synopsis>
<para>
--- 2823,2833 ----
</para>
</sect3>
! <sect3 id="plpgsql-open-bound-cursor">
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional>;
</synopsis>
<para>
*************** OPEN <replaceable>bound_cursorvar</repla
*** 2847,2856 ****
--- 2847,2867 ----
</para>
<para>
+ Cursors may be opened using either <firstterm>positional</firstterm>
+ or <firstterm>named</firstterm> notation. Similar to calling
+ functions, described in <xref linkend="sql-syntax-calling-funcs">, it
+ is also allowed to mix positional and named notation. In positional
+ notation, all arguments are specified in order. In named notation,
+ each argument's name is specified using <literal>:=</literal> to
+ separate it from the argument expression.
+ </para>
+
+ <para>
Examples (these use the cursor declaration examples above):
<programlisting>
OPEN curs2;
OPEN curs3(42);
+ OPEN curs3(key := 42);
</programlisting>
</para>
*************** COMMIT;
*** 3169,3186 ****
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
The cursor variable must have been bound to some query when it was
! declared, and it <emphasis>cannot</> be open already. The
! <command>FOR</> statement automatically opens the cursor, and it closes
! the cursor again when the loop exits. A list of actual argument value
! expressions must appear if and only if the cursor was declared to take
! arguments. These values will be substituted in the query, in just
! the same way as during an <command>OPEN</>.
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
--- 3180,3203 ----
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
The cursor variable must have been bound to some query when it was
! declared, and it <emphasis>cannot</> be open already. The <command>FOR</>
! statement automatically opens the cursor, and it closes the cursor again
! when the loop exits. The cursor's arguments may be supplied in either
! <firstterm>positional</firstterm> or <firstterm>named</firstterm>
! notation. A list of actual argument value expressions must appear if and
! only if the cursor was declared to take arguments. These values will be
! substituted in the query, in just the same way as during an
! <command>OPEN</command> described in <xref
! linkend="plpgsql-open-bound-cursor">.
! </para>
!
! <para>
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
new file mode 100644
index 8c4c2f7..107007a
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
*************** static PLpgSQL_expr *read_sql_construct(
*** 67,72 ****
--- 67,73 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken);
static PLpgSQL_expr *read_sql_expression(int until,
*************** for_control : for_variable K_IN
*** 1313,1318 ****
--- 1314,1320 ----
"SELECT ",
true,
false,
+ true,
&expr1loc,
&tok);
*************** stmt_raise : K_RAISE
*** 1692,1698 ****
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
--- 1694,1700 ----
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
*************** stmt_dynexecute : K_EXECUTE
*** 1790,1796 ****
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
--- 1792,1798 ----
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
*************** stmt_dynexecute : K_EXECUTE
*** 1829,1835 ****
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
--- 1831,1837 ----
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
*************** static PLpgSQL_expr *
*** 2322,2328 ****
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
--- 2324,2330 ----
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
*************** read_sql_expression2(int until, int unti
*** 2331,2337 ****
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
--- 2333,2339 ----
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
*************** static PLpgSQL_expr *
*** 2339,2345 ****
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, NULL, NULL);
}
/*
--- 2341,2347 ----
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, true, NULL, NULL);
}
/*
*************** read_sql_stmt(const char *sqlstart)
*** 2352,2357 ****
--- 2354,2360 ----
* sqlstart: text to prefix to the accumulated SQL text
* isexpression: whether to say we're reading an "expression" or a "statement"
* valid_sql: whether to check the syntax of the expr (prefixed with sqlstart)
+ * bool: trim trailing whitespace
* startloc: if not NULL, location of first token is stored at *startloc
* endtoken: if not NULL, ending token is stored at *endtoken
* (this is only interesting if until2 or until3 isn't zero)
*************** read_sql_construct(int until,
*** 2364,2369 ****
--- 2367,2373 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken)
{
*************** read_sql_construct(int until,
*** 2443,2450 ****
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
--- 2447,2455 ----
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! if (trim)
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
*************** check_labels(const char *start_label, co
*** 3374,3389 ****
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL.
! * If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token. Consume all that and return a SELECT query that
! * evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
! PLpgSQL_expr *expr;
! int tok;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
--- 3379,3401 ----
/*
* Read the arguments (if any) for a cursor, followed by the until token
*
! * If cursor has no args, just swallow the until token and return NULL. If it
! * does have args, we expect to see "( expr [, expr ...] )" followed by the
! * until token, where expr may be a plain expression, or a named parameter
! * assignment of the form IDENT := expr. Consume all that and return a SELECT
! * query that evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
! PLpgSQL_expr *expr;
! PLpgSQL_row *row;
! int tok;
! int argc = 0;
! char **argv;
! StringInfoData ds;
! char *sqlstart = "SELECT ";
! bool named = false;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3402,3407 ****
--- 3414,3422 ----
return NULL;
}
+ row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+ argv = (char **) palloc0(sizeof(char *) * row->nfields);
+
/* Else better provide arguments */
if (tok != '(')
ereport(ERROR,
*************** read_cursor_args(PLpgSQL_var *cursor, in
*** 3410,3419 ****
cursor->refname),
parser_errposition(yylloc)));
! /*
! * Read expressions until the matching ')'.
! */
! expr = read_sql_expression(')', ")");
/* Next we'd better find the until token */
tok = yylex();
--- 3425,3525 ----
cursor->refname),
parser_errposition(yylloc)));
! for (argc = 0; argc < row->nfields; argc++)
! {
! int argpos;
! int endtoken;
! PLpgSQL_expr *item;
! int arglocation;
!
! if (plpgsql_isidentassign(&arglocation))
! {
! /* Named parameter assignment */
! named = true;
! for (argpos = 0; argpos < row->nfields; argpos++)
! if (strcmp(row->fieldnames[argpos], yylval.str) == 0)
! break;
!
! if (argpos == row->nfields)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" has no argument named \"%s\"",
! cursor->refname, yylval.str),
! parser_errposition(yylloc)));
! }
! else
! {
! /* Positional parameter assignment */
! argpos = argc;
! }
!
! /*
! * Read one expression at a time until the matching endtoken. To
! * provide the user with meaningful parse error positions, we check the
! * syntax immediately, instead of checking the final expression that
! * may have a permutated argument list. Also trailing whitespace may
! * not be trimmed, since otherwise input of the form (param --
! * comment\n, param) is translated into a form that comments the
! * remaining parameters.
! */
! item = read_sql_construct(',', ')', 0,
! ",\" or \")",
! sqlstart,
! true, true,
! false, /* do not trim */
! NULL, &endtoken);
!
! if (endtoken == ')' && !(argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("not enough arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (endtoken == ',' && (argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("too many arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (argv[argpos] != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("duplicate value for cursor \"%s\" parameter \"%s\"",
! cursor->refname, row->fieldnames[argpos]),
! parser_errposition(arglocation)));
!
! argv[argpos] = item->query + strlen(sqlstart);
! }
!
! /* Make positional argument list */
! initStringInfo(&ds);
! appendStringInfoString(&ds, sqlstart);
! for (argc = 0; argc < row->nfields; argc++)
! {
! Assert(argv[argc] != NULL);
!
! /*
! * Because named notation allows permutated argument lists, include
! * the parameter name for meaningful runtime errors.
! */
! if (named)
! appendStringInfo(&ds, "/* %s := */ ", row->fieldnames[argc]);
!
! appendStringInfoString(&ds, argv[argc]);
! if (argc < row->nfields - 1)
! appendStringInfoString(&ds, ", ");
! }
! appendStringInfoChar(&ds, ';');
!
! expr = palloc0(sizeof(PLpgSQL_expr));
! expr->dtype = PLPGSQL_DTYPE_EXPR;
! expr->query = pstrdup(ds.data);
! expr->plan = NULL;
! expr->paramnos = NULL;
! expr->ns = plpgsql_ns_top();
! pfree(ds.data);
/* Next we'd better find the until token */
tok = yylex();
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
new file mode 100644
index 76e8436..ce3161c
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
*************** plpgsql_scanner_finish(void)
*** 583,585 ****
--- 583,622 ----
yyscanner = NULL;
scanorig = NULL;
}
+
+ /*
+ * Return true if 'IDENT' ':=' are the next two tokens
+ *
+ * startloc: if not NULL, location of first token is stored at *startloc
+ */
+ bool
+ plpgsql_isidentassign(int *startloc)
+ {
+ int tok1, tok2;
+ TokenAuxData aux1, aux2;
+ bool result = false;
+
+ tok1 = internal_yylex(&aux1);
+ if (startloc)
+ *startloc = aux1.lloc;
+
+ if (tok1 == IDENT)
+ {
+ tok2 = internal_yylex(&aux2);
+
+ if (tok2 == COLON_EQUALS)
+ result = true;
+ else
+ push_back_token(tok2, &aux2);
+ }
+
+ if (!result)
+ push_back_token(tok1, &aux1);
+
+ plpgsql_yylval = aux1.lval;
+ plpgsql_yylloc = aux1.lloc;
+ plpgsql_yyleng = aux1.leng;
+
+ return result;
+ }
+
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
new file mode 100644
index c638f43..b3ff847
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** extern int plpgsql_location_to_lineno(in
*** 968,973 ****
--- 968,974 ----
extern int plpgsql_latest_lineno(void);
extern void plpgsql_scanner_init(const char *str);
extern void plpgsql_scanner_finish(void);
+ extern bool plpgsql_isidentassign(int *startloc);
/* ----------
* Externs in gram.y
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
new file mode 100644
index 238bf5f..1df0f23
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** select refcursor_test2(20000, 20000) as
*** 2292,2297 ****
--- 2292,2428 ----
(1 row)
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ -- mixing named and positional, ok
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+ -- mixing named and positional, double param2, parse time error at second value
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $1, $2);
+ end
+ $$ language plpgsql;
+ ERROR: duplicate value for cursor "c1" parameter "param2"
+ LINE 5: open c1(param2 := $1, $2);
+ ^
+ -- mixing named and positional, double param1, parse time error at second value
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1($1, param1 := $2);
+ end
+ $$ language plpgsql;
+ ERROR: duplicate value for cursor "c1" parameter "param1"
+ LINE 5: open c1($1, param1 := $2);
+ ^
+ -- not enough parameters, parse time error at );
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+ ERROR: not enough arguments for cursor "c1"
+ LINE 5: open c1(param2 := $2);
+ ^
+ -- double parameter, parse time error at second p2
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end $$ language plpgsql;
+ ERROR: duplicate value for cursor "c1" parameter "p2"
+ LINE 6: open c1 (p2 := 77, p2 := 42);
+ ^
+ -- division by zero runtime error,
+ -- provides context users can make sense of:
+ -- CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test comment and newline structure, will not give runtime error when
+ -- read_sql_construct trims trailing whitespace
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 7: , 42/);
+ ^
+ select fooey();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ PL/pgSQL function "fooey" line 6 at OPEN
+ -- test syntax error at :
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77);
+ end $$ language plpgsql;
+ ERROR: syntax error at or near ":"
+ LINE 6: open c1 ( p2 := 42, p1 : = 77);
+ ^
+ -- test another syntax error at ,
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77);
+ end $$ language plpgsql;
+ ERROR: syntax error at end of input
+ LINE 6: open c1 ( p2 := 42/, p1 : = 77);
+ ^
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
new file mode 100644
index b47c2de..f1fb7fd
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** select refcursor_test2(20000, 20000) as
*** 1946,1951 ****
--- 1946,2061 ----
refcursor_test2(20, 20) as "Should be true";
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, -- comment after , should be ignored
+ param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+
+ -- mixing named and positional, ok
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param1 := $1, $2);
+ end
+ $$ language plpgsql;
+
+ -- mixing named and positional, double param2, parse time error at second value
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $1, $2);
+ end
+ $$ language plpgsql;
+
+ -- mixing named and positional, double param1, parse time error at second value
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1($1, param1 := $2);
+ end
+ $$ language plpgsql;
+
+ -- not enough parameters, parse time error at );
+ create or replace function named(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := $2);
+ end
+ $$ language plpgsql;
+
+ -- double parameter, parse time error at second p2
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end $$ language plpgsql;
+
+ -- division by zero runtime error,
+ -- provides context users can make sense of:
+ -- CONTEXT: SQL statement "SELECT /* p1 := */ 42/0, /* p2 := */ 77;"
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test comment and newline structure, will not give runtime error when
+ -- read_sql_construct trims trailing whitespace
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (77 -- test
+ , 42/);
+ end $$ language plpgsql;
+ select fooey();
+
+ -- test syntax error at :
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42, p1 : = 77);
+ end $$ language plpgsql;
+
+ -- test another syntax error at ,
+ create or replace function fooey() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 ( p2 := 42/, p1 : = 77);
+ end $$ language plpgsql;
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-12 19:55 Kevin Grittner <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Kevin Grittner @ 2011-12-12 19:55 UTC (permalink / raw)
To: [email protected]; +Cc: pgsql-hackers
Yeb Havinga wrote:
> Forgot to copy regression output to expected - attached v7 fixes
> that.
This version addresses all of my concerns. It applies cleanly and
compiles without warning against current HEAD and performs as
advertised. I'm marking it Ready for Committer.
-Kevin
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-13 16:22 Heikki Linnakangas <[email protected]>
parent: Kevin Grittner <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Heikki Linnakangas @ 2011-12-13 16:22 UTC (permalink / raw)
To: Kevin Grittner <[email protected]>; +Cc: [email protected]; pgsql-hackers
On 12.12.2011 21:55, Kevin Grittner wrote:
> Yeb Havinga wrote:
>
>> Forgot to copy regression output to expected - attached v7 fixes
>> that.
>
> This version addresses all of my concerns. It applies cleanly and
> compiles without warning against current HEAD and performs as
> advertised. I'm marking it Ready for Committer.
This failed:
postgres=# do $$
declare
foocur CURSOR ("insane /* arg" int4) IS SELECT generate_series(1,
"insane /* arg");
begin
OPEN foocur("insane /* arg" := 10);
end;
$$;
ERROR: unterminated /* comment at or near "/* insane /* arg := */ 10;"
LINE 1: SELECT /* insane /* arg := */ 10;
^
QUERY: SELECT /* insane /* arg := */ 10;
CONTEXT: PL/pgSQL function "inline_code_block" line 5 at OPEN
I don't have much sympathy for anyone who uses argument names like that,
but it nevertheless ought to not fail. A simple way to fix that is to
constuct the query as: "value AS argname", instead of "/* argname := */
value". Then you can use the existing quote_identifier() function to do
the necessary quoting.
I replaced the plpgsql_isidentassign() function with a more generic
plpgsql_peek2() function, which allows you to peek ahead two tokens in
the input stream, without eating them. It's implemented using the
pushdown stack like plpgsql_isidentassign() was, but the division of
labor between pl_scanner.c and gram.y seems more clear this way. I'm
still not 100% happy with it. plpgsql_peek2() behaves differently from
plpgsql_yylex(), in that it doesn't perform variable or unreserved
keyword lookup. It could do that, but it would be quite pointless since
the only current caller doesn't want variable or unreserved keyword
lookup, so it would just have to work harder to undo them.
Attached is a patch with those changes. I also I removed a few of the
syntax error regression tests, that seemed excessive, plus some general
naming and comment fiddling. I'll apply this tomorrow, if it still looks
good to me after sleeping on it.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Attachments:
[text/x-diff] cursornamedparameter-v8-heikki.patch (24.6K, ../../[email protected]/2-cursornamedparameter-v8-heikki.patch)
download | inline diff:
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 2823,2833 **** OPEN curs1 FOR EXECUTE 'SELECT * FROM ' || quote_ident(tabname)
</para>
</sect3>
! <sect3>
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional>;
</synopsis>
<para>
--- 2823,2833 ----
</para>
</sect3>
! <sect3 id="plpgsql-open-bound-cursor">
<title>Opening a Bound Cursor</title>
<synopsis>
! OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional>;
</synopsis>
<para>
***************
*** 2847,2856 **** OPEN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argume
--- 2847,2867 ----
</para>
<para>
+ Argument values can be passed using either <firstterm>positional</firstterm>
+ or <firstterm>named</firstterm> notation. In positional
+ notation, all arguments are specified in order. In named notation,
+ each argument's name is specified using <literal>:=</literal> to
+ separate it from the argument expression. Similar to calling
+ functions, described in <xref linkend="sql-syntax-calling-funcs">, it
+ is also allowed to mix positional and named notation.
+ </para>
+
+ <para>
Examples (these use the cursor declaration examples above):
<programlisting>
OPEN curs2;
OPEN curs3(42);
+ OPEN curs3(key := 42);
</programlisting>
</para>
***************
*** 3169,3175 **** COMMIT;
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <replaceable>argument_values</replaceable> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
--- 3180,3186 ----
<synopsis>
<optional> <<<replaceable>label</replaceable>>> </optional>
! FOR <replaceable>recordvar</replaceable> IN <replaceable>bound_cursorvar</replaceable> <optional> ( <optional> <replaceable>argname</replaceable> := </optional> <replaceable>argument_value</replaceable> <optional>, ...</optional> ) </optional> LOOP
<replaceable>statements</replaceable>
END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
***************
*** 3179,3186 **** END LOOP <optional> <replaceable>label</replaceable> </optional>;
<command>FOR</> statement automatically opens the cursor, and it closes
the cursor again when the loop exits. A list of actual argument value
expressions must appear if and only if the cursor was declared to take
! arguments. These values will be substituted in the query, in just
! the same way as during an <command>OPEN</>.
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
--- 3190,3201 ----
<command>FOR</> statement automatically opens the cursor, and it closes
the cursor again when the loop exits. A list of actual argument value
expressions must appear if and only if the cursor was declared to take
! arguments. These values will be substituted in the query, in just
! the same way as during an <command>OPEN</> (see <xref
! linkend="plpgsql-open-bound-cursor">).
! </para>
!
! <para>
The variable <replaceable>recordvar</replaceable> is automatically
defined as type <type>record</> and exists only inside the loop (any
existing definition of the variable name is ignored within the loop).
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
***************
*** 67,72 **** static PLpgSQL_expr *read_sql_construct(int until,
--- 67,73 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken);
static PLpgSQL_expr *read_sql_expression(int until,
***************
*** 1313,1318 **** for_control : for_variable K_IN
--- 1314,1320 ----
"SELECT ",
true,
false,
+ true,
&expr1loc,
&tok);
***************
*** 1692,1698 **** stmt_raise : K_RAISE
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
--- 1694,1700 ----
expr = read_sql_construct(',', ';', K_USING,
", or ; or USING",
"SELECT ",
! true, true, true,
NULL, &tok);
new->params = lappend(new->params, expr);
}
***************
*** 1790,1796 **** stmt_dynexecute : K_EXECUTE
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
--- 1792,1798 ----
expr = read_sql_construct(K_INTO, K_USING, ';',
"INTO or USING or ;",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
***************
*** 1829,1835 **** stmt_dynexecute : K_EXECUTE
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
--- 1831,1837 ----
expr = read_sql_construct(',', ';', K_INTO,
", or ; or INTO",
"SELECT ",
! true, true, true,
NULL, &endtoken);
new->params = lappend(new->params, expr);
} while (endtoken == ',');
***************
*** 2322,2328 **** static PLpgSQL_expr *
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
--- 2324,2330 ----
read_sql_expression(int until, const char *expected)
{
return read_sql_construct(until, 0, 0, expected,
! "SELECT ", true, true, true, NULL, NULL);
}
/* Convenience routine to read an expression with two possible terminators */
***************
*** 2331,2337 **** read_sql_expression2(int until, int until2, const char *expected,
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
--- 2333,2339 ----
int *endtoken)
{
return read_sql_construct(until, until2, 0, expected,
! "SELECT ", true, true, true, NULL, endtoken);
}
/* Convenience routine to read a SQL statement that must end with ';' */
***************
*** 2339,2345 **** static PLpgSQL_expr *
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, NULL, NULL);
}
/*
--- 2341,2347 ----
read_sql_stmt(const char *sqlstart)
{
return read_sql_construct(';', 0, 0, ";",
! sqlstart, false, true, true, NULL, NULL);
}
/*
***************
*** 2352,2357 **** read_sql_stmt(const char *sqlstart)
--- 2354,2360 ----
* sqlstart: text to prefix to the accumulated SQL text
* isexpression: whether to say we're reading an "expression" or a "statement"
* valid_sql: whether to check the syntax of the expr (prefixed with sqlstart)
+ * bool: trim trailing whitespace
* startloc: if not NULL, location of first token is stored at *startloc
* endtoken: if not NULL, ending token is stored at *endtoken
* (this is only interesting if until2 or until3 isn't zero)
***************
*** 2364,2369 **** read_sql_construct(int until,
--- 2367,2373 ----
const char *sqlstart,
bool isexpression,
bool valid_sql,
+ bool trim,
int *startloc,
int *endtoken)
{
***************
*** 2443,2450 **** read_sql_construct(int until,
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
--- 2447,2455 ----
plpgsql_append_source_text(&ds, startlocation, yylloc);
/* trim any trailing whitespace, for neatness */
! if (trim)
! while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
! ds.data[--ds.len] = '\0';
expr = palloc0(sizeof(PLpgSQL_expr));
expr->dtype = PLPGSQL_DTYPE_EXPR;
***************
*** 3376,3389 **** check_labels(const char *start_label, const char *end_label, int end_location)
*
* If cursor has no args, just swallow the until token and return NULL.
* If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token. Consume all that and return a SELECT query that
! * evaluates the expression(s) (without the outer parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
int tok;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
--- 3381,3402 ----
*
* If cursor has no args, just swallow the until token and return NULL.
* If it does have args, we expect to see "( expr [, expr ...] )" followed
! * by the until token, where expr may be a plain expression, or a named
! * parameter assignment of the form IDENT := expr. Consume all that and
! * return a SELECT query that evaluates the expression(s) (without the outer
! * parens).
*/
static PLpgSQL_expr *
read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
{
PLpgSQL_expr *expr;
+ PLpgSQL_row *row;
int tok;
+ int argc = 0;
+ char **argv;
+ StringInfoData ds;
+ char *sqlstart = "SELECT ";
+ bool named = false;
tok = yylex();
if (cursor->cursor_explicit_argrow < 0)
***************
*** 3402,3407 **** read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
--- 3415,3423 ----
return NULL;
}
+ row = (PLpgSQL_row *) plpgsql_Datums[cursor->cursor_explicit_argrow];
+ argv = (char **) palloc0(row->nfields * sizeof(char *));
+
/* Else better provide arguments */
if (tok != '(')
ereport(ERROR,
***************
*** 3411,3419 **** read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
parser_errposition(yylloc)));
/*
! * Read expressions until the matching ')'.
*/
! expr = read_sql_expression(')', ")");
/* Next we'd better find the until token */
tok = yylex();
--- 3427,3545 ----
parser_errposition(yylloc)));
/*
! * Read the arguments, one by one.
*/
! for (argc = 0; argc < row->nfields; argc++)
! {
! PLpgSQL_expr *item;
! int endtoken;
! int argpos;
! int tok1,
! tok2;
! int arglocation;
!
! /* Check if it's a named parameter: "param := value" */
! plpgsql_peek2(&tok1, &tok2, &arglocation, NULL);
! if (tok1 == IDENT && tok2 == COLON_EQUALS)
! {
! char *argname;
!
! /* Read the argument name, and find its position */
! yylex();
! argname = yylval.str;
!
! for (argpos = 0; argpos < row->nfields; argpos++)
! {
! if (strcmp(row->fieldnames[argpos], argname) == 0)
! break;
! }
! if (argpos == row->nfields)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("cursor \"%s\" has no argument named \"%s\"",
! cursor->refname, argname),
! parser_errposition(yylloc)));
!
! /*
! * Eat the ":=". We already peeked, so the error should never
! * happen.
! */
! tok2 = yylex();
! if (tok2 != COLON_EQUALS)
! yyerror("syntax error");
!
! named = true;
! }
! else
! argpos = argc;
!
! /*
! * Read the value expression. To provide the user with meaningful
! * parse error positions, we check the syntax immediately, instead of
! * checking the final expression that may have the arguments
! * reordered. Trailing whitespace must not be trimmed, because
! * otherwise input of the form (param -- comment\n, param) would be
! * translated into a form where the second parameter is commented
! * out.
! */
! item = read_sql_construct(',', ')', 0,
! ",\" or \")",
! sqlstart,
! true, true,
! false, /* do not trim */
! NULL, &endtoken);
!
! if (endtoken == ')' && !(argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("not enough arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (endtoken == ',' && (argc == row->nfields - 1))
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("too many arguments for cursor \"%s\"",
! cursor->refname),
! parser_errposition(yylloc)));
!
! if (argv[argpos] != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("duplicate value for cursor \"%s\" parameter \"%s\"",
! cursor->refname, row->fieldnames[argpos]),
! parser_errposition(arglocation)));
!
! argv[argpos] = item->query + strlen(sqlstart);
! }
!
! /* Make positional argument list */
! initStringInfo(&ds);
! appendStringInfoString(&ds, sqlstart);
! for (argc = 0; argc < row->nfields; argc++)
! {
! Assert(argv[argc] != NULL);
!
! /*
! * Because named notation allows permutated argument lists, include
! * the parameter name for meaningful runtime errors.
! */
! appendStringInfoString(&ds, argv[argc]);
! if (named)
! appendStringInfo(&ds, " AS %s",
! quote_identifier(row->fieldnames[argc]));
! if (argc < row->nfields - 1)
! appendStringInfoString(&ds, ", ");
! }
! appendStringInfoChar(&ds, ';');
!
! expr = palloc0(sizeof(PLpgSQL_expr));
! expr->dtype = PLPGSQL_DTYPE_EXPR;
! expr->query = pstrdup(ds.data);
! expr->plan = NULL;
! expr->paramnos = NULL;
! expr->ns = plpgsql_ns_top();
! pfree(ds.data);
/* Next we'd better find the until token */
tok = yylex();
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
***************
*** 424,429 **** plpgsql_append_source_text(StringInfo buf,
--- 424,459 ----
}
/*
+ * Peek two tokens ahead in the input stream. The first token and its
+ * location the query are returned in *tok1_p and *tok1_loc, second token
+ * and its location in *tok2_p and *tok2_loc.
+ *
+ * NB: no variable or unreserved keyword lookup is performed here, they will
+ * be returned as IDENT. Reserved keywords are resolved as usual.
+ */
+ void
+ plpgsql_peek2(int *tok1_p, int *tok2_p, int *tok1_loc, int *tok2_loc)
+ {
+ int tok1,
+ tok2;
+ TokenAuxData aux1,
+ aux2;
+
+ tok1 = internal_yylex(&aux1);
+ tok2 = internal_yylex(&aux2);
+
+ *tok1_p = tok1;
+ if (tok1_loc)
+ *tok1_loc = aux1.lloc;
+ *tok2_p = tok2;
+ if (tok2_loc)
+ *tok2_loc = aux2.lloc;
+
+ push_back_token(tok2, &aux2);
+ push_back_token(tok1, &aux1);
+ }
+
+ /*
* plpgsql_scanner_errposition
* Report an error cursor position, if possible.
*
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 962,967 **** extern int plpgsql_yylex(void);
--- 962,969 ----
extern void plpgsql_push_back_token(int token);
extern void plpgsql_append_source_text(StringInfo buf,
int startlocation, int endlocation);
+ extern void plpgsql_peek2(int *tok1_p, int *tok2_p, int *tok1_loc,
+ int *tok2_loc);
extern int plpgsql_scanner_errposition(int location);
extern void plpgsql_yyerror(const char *message);
extern int plpgsql_location_to_lineno(int location);
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 2292,2297 **** select refcursor_test2(20000, 20000) as "Should be false",
--- 2292,2426 ----
(1 row)
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+ Should be false | Should be true
+ -----------------+----------------
+ f | t
+ (1 row)
+
+ -- mixing named and positional argument notations
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test2(20, 20);
+ namedparmcursor_test2
+ -----------------------
+ t
+ (1 row)
+
+ -- mixing named and positional: param2 is given twice, once in named notation
+ -- and second time in positional notation. Should throw an error at parse time
+ create function namedparmcursor_test3() returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := 20, 21);
+ end
+ $$ language plpgsql;
+ ERROR: duplicate value for cursor "c1" parameter "param2"
+ LINE 5: open c1(param2 := 20, 21);
+ ^
+ -- mixing named and positional: same as previous test, but param1 is duplicated
+ create function namedparmcursor_test4() returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(20, param1 := 21);
+ end
+ $$ language plpgsql;
+ ERROR: duplicate value for cursor "c1" parameter "param1"
+ LINE 5: open c1(20, param1 := 21);
+ ^
+ -- duplicate named parameter, should throw an error at parse time
+ create function namedparmcursor_test5() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end
+ $$ language plpgsql;
+ ERROR: duplicate value for cursor "c1" parameter "p2"
+ LINE 6: open c1 (p2 := 77, p2 := 42);
+ ^
+ -- not enough parameters, should throw an error at parse time
+ create function namedparmcursor_test6() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77);
+ end
+ $$ language plpgsql;
+ ERROR: not enough arguments for cursor "c1"
+ LINE 6: open c1 (p2 := 77);
+ ^
+ -- division by zero runtime error, the context given in the error message
+ -- should be sensible
+ create function namedparmcursor_test7() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select namedparmcursor_test7();
+ ERROR: division by zero
+ CONTEXT: SQL statement "SELECT 42/0 AS p1, 77 AS p2;"
+ PL/pgSQL function "namedparmcursor_test7" line 6 at OPEN
+ -- check that line comments work correctly within the argument list (there
+ -- is some special handling of this case in the code: the newline after the
+ -- comment must be preserved when the argument-evaluating query is
+ -- constructed, otherwise the comment effectively comments out the next
+ -- argument, too)
+ create function namedparmcursor_test8() returns int4 as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select count(*) from tenk1 where thousand = p1 and tenthous = p2;
+ n int4;
+ begin
+ open c1 (77 -- test
+ , 42);
+ fetch c1 into n;
+ return n;
+ end $$ language plpgsql;
+ select namedparmcursor_test8();
+ namedparmcursor_test8
+ -----------------------
+ 0
+ (1 row)
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 1946,1951 **** select refcursor_test2(20000, 20000) as "Should be false",
--- 1946,2059 ----
refcursor_test2(20, 20) as "Should be true";
--
+ -- tests for cursors with named parameter arguments
+ --
+ create function namedparmcursor_test1(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param12 int) for select * from rc_test where a > param1 and b > param12;
+ nonsense record;
+ begin
+ open c1(param12 := $2, param1 := $1);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+
+ select namedparmcursor_test1(20000, 20000) as "Should be false",
+ namedparmcursor_test1(20, 20) as "Should be true";
+
+ -- mixing named and positional argument notations
+ create function namedparmcursor_test2(int, int) returns boolean as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ nonsense record;
+ begin
+ open c1(param1 := $1, $2);
+ fetch c1 into nonsense;
+ close c1;
+ if found then
+ return true;
+ else
+ return false;
+ end if;
+ end
+ $$ language plpgsql;
+ select namedparmcursor_test2(20, 20);
+
+ -- mixing named and positional: param2 is given twice, once in named notation
+ -- and second time in positional notation. Should throw an error at parse time
+ create function namedparmcursor_test3() returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(param2 := 20, 21);
+ end
+ $$ language plpgsql;
+
+ -- mixing named and positional: same as previous test, but param1 is duplicated
+ create function namedparmcursor_test4() returns void as $$
+ declare
+ c1 cursor (param1 int, param2 int) for select * from rc_test where a > param1 and b > param2;
+ begin
+ open c1(20, param1 := 21);
+ end
+ $$ language plpgsql;
+
+ -- duplicate named parameter, should throw an error at parse time
+ create function namedparmcursor_test5() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p2 := 42);
+ end
+ $$ language plpgsql;
+
+ -- not enough parameters, should throw an error at parse time
+ create function namedparmcursor_test6() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77);
+ end
+ $$ language plpgsql;
+
+ -- division by zero runtime error, the context given in the error message
+ -- should be sensible
+ create function namedparmcursor_test7() returns void as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select * from tenk1 where thousand = p1 and tenthous = p2;
+ begin
+ open c1 (p2 := 77, p1 := 42/0);
+ end $$ language plpgsql;
+ select namedparmcursor_test7();
+
+ -- check that line comments work correctly within the argument list (there
+ -- is some special handling of this case in the code: the newline after the
+ -- comment must be preserved when the argument-evaluating query is
+ -- constructed, otherwise the comment effectively comments out the next
+ -- argument, too)
+ create function namedparmcursor_test8() returns int4 as $$
+ declare
+ c1 cursor (p1 int, p2 int) for
+ select count(*) from tenk1 where thousand = p1 and tenthous = p2;
+ n int4;
+ begin
+ open c1 (77 -- test
+ , 42);
+ fetch c1 into n;
+ return n;
+ end $$ language plpgsql;
+ select namedparmcursor_test8();
+
+ --
-- tests for "raise" processing
--
create function raise_test1(int) returns int as $$
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-13 17:34 Tom Lane <[email protected]>
parent: Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Tom Lane @ 2011-12-13 17:34 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Kevin Grittner <[email protected]>; [email protected]; pgsql-hackers
Heikki Linnakangas <[email protected]> writes:
> Attached is a patch with those changes. I also I removed a few of the
> syntax error regression tests, that seemed excessive, plus some general
> naming and comment fiddling. I'll apply this tomorrow, if it still looks
> good to me after sleeping on it.
The code looks reasonably clean now, although I noted one comment
thinko:
> + * bool: trim trailing whitespace
ITYM
> + * trim: trim trailing whitespace
However, I'm still concerned about whether this approach gives
reasonable error messages in cases where the error would be
detected during parse analysis of the rearranged statement.
The regression test examples don't cover such cases, and I'm
too busy right now to apply the patch and check for myself.
What happens for example if a named parameter's value contains
a misspelled variable reference, or a type conflict?
regards, tom lane
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-14 10:31 Yeb Havinga <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Yeb Havinga @ 2011-12-14 10:31 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; Kevin Grittner <[email protected]>; pgsql-hackers
On 2011-12-13 18:34, Tom Lane wrote:
> Heikki Linnakangas<[email protected]> writes:
>> Attached is a patch with those changes. I also I removed a few of the
>> syntax error regression tests, that seemed excessive, plus some general
>> naming and comment fiddling. I'll apply this tomorrow, if it still looks
>> good to me after sleeping on it.
> However, I'm still concerned about whether this approach gives
> reasonable error messages in cases where the error would be
> detected during parse analysis of the rearranged statement.
> The regression test examples don't cover such cases, and I'm
> too busy right now to apply the patch and check for myself.
> What happens for example if a named parameter's value contains
> a misspelled variable reference, or a type conflict?
I tested this and seems to be ok:
regression=# select namedparmcursor_test1(20000, 20000) as "Should be
false",
namedparmcursor_test1(20, 20) as "Should be true";
ERROR: column "yy" does not exist
LINE 1: SELECT x AS param1, yy AS param12;
regression=# select namedparmcursor_test1(20000, 20000) as "Should be
false",
namedparmcursor_test1(20, 20) as "Should be true";
ERROR: invalid input syntax for integer: "2011-11-29 19:26:10.029084"
CONTEXT: PL/pgSQL function "namedparmcursor_test1" line 8 at OPEN
regards,
Yeb Havinga
last error was created with
create or replace function namedparmcursor_test1(int, int) returns
boolean as $$
declare
c1 cursor (param1 int, param12 int) for select * from rc_test where
a > param1 and b > param12;
y int := 10;
x timestamp := now();
nonsense record;
begin
open c1(param12 := $1, param1 := x);
end
$$ language plpgsql;
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: [REVIEW] Patch for cursor calling with named parameters
@ 2011-12-14 13:58 Heikki Linnakangas <[email protected]>
parent: Yeb Havinga <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Heikki Linnakangas @ 2011-12-14 13:58 UTC (permalink / raw)
To: Yeb Havinga <[email protected]>; +Cc: Tom Lane <[email protected]>; Heikki Linnakangas <[email protected]>; Kevin Grittner <[email protected]>; pgsql-hackers
On 14.12.2011 12:31, Yeb Havinga wrote:
> On 2011-12-13 18:34, Tom Lane wrote:
>> Heikki Linnakangas<[email protected]> writes:
>>> Attached is a patch with those changes. I also I removed a few of the
>>> syntax error regression tests, that seemed excessive, plus some general
>>> naming and comment fiddling. I'll apply this tomorrow, if it still looks
>>> good to me after sleeping on it.
>> However, I'm still concerned about whether this approach gives
>> reasonable error messages in cases where the error would be
>> detected during parse analysis of the rearranged statement.
>> The regression test examples don't cover such cases, and I'm
>> too busy right now to apply the patch and check for myself.
>> What happens for example if a named parameter's value contains
>> a misspelled variable reference, or a type conflict?
>
> I tested this and seems to be ok:
>
> regression=# select namedparmcursor_test1(20000, 20000) as "Should be
> false",
> namedparmcursor_test1(20, 20) as "Should be true";
> ERROR: column "yy" does not exist
> LINE 1: SELECT x AS param1, yy AS param12;
For the record, the caret pointing to the position is there, too. As in:
regression=# do $$
declare
c1 cursor (param1 int, param2 int) for select 123;
begin
open c1(param2 := xxx, param1 := 123);
end;
$$;
ERROR: column "xxx" does not exist
LINE 1: SELECT 123 AS param1, xxx AS param2;
^
QUERY: SELECT 123 AS param1, xxx AS param2;
CONTEXT: PL/pgSQL function "inline_code_block" line 5 at OPEN
I think that's good enough. It would be even better if we could print
the original OPEN statement as the context, as in:
ERROR: column "xxx" does not exist
LINE 4: OPEN c1(param2 := xxx, param1 := 123);
^
but it didn't look quite like that before the patch either, and isn't
specific to this patch but more of a general usability issue in PL/pgSQL.
Committed.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 38+ messages in thread
* [PATCH] Distinguish properly when database-specific transaction list should be used.
@ 2026-05-06 07:38 Antonin Houska <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Antonin Houska @ 2026-05-06 07:38 UTC (permalink / raw)
Currently, decoding session that relies on database-specific xl_running_xacts
WAL records can also use some information of cluster-wide records and vice
versa. Although this approach might reduce the total number of
xl_running_xacts records, it's simply not correct.
SnapBuildProcessRunningXacts() now decides at the very beginning whether
particular record can be used or not.
---
src/backend/replication/logical/snapbuild.c | 55 +++++++++++++--------
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index c8309b96ed4..b9661b7d70a 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1157,6 +1157,39 @@ SnapBuildProcessRunningXacts(SnapBuild *builder, XLogRecPtr lsn, xl_running_xact
ReorderBufferTXN *txn;
TransactionId xmin;
+ /*
+ * Each decoding session should use either cluster-wide snapshots or
+ * database-specific ones, but not both. Since the latter can have more
+ * recent value of oldestRunningXid, builder->xmin could go backwards if
+ * it was followed by cluster-wide snapshot - see the ->xmin adjustment
+ * below.
+ */
+ if (!db_specific && OidIsValid(running->dbid))
+ return;
+
+ if (db_specific)
+ {
+ /*
+ * Make sure that we have the snapshots needed for startup, as well as
+ * those for regular cleanup: each time the cluster-wide snapshot is
+ * created (typically on slot creation or by checkpointer), the
+ * database-specific snapshot is requested here if the current session
+ * needs it.
+ */
+ if (!OidIsValid(running->dbid))
+ {
+ LogStandbySnapshot(MyDatabaseId);
+
+ return;
+ }
+ /*
+ * Snapshots issued for other databases do not contain the information
+ * about transactions in our database.
+ */
+ else if (running->dbid != MyDatabaseId)
+ return;
+ }
+
/*
* If we're not consistent yet, inspect the record to see whether it
* allows to get closer to being consistent. If we are consistent, dump
@@ -1171,17 +1204,7 @@ SnapBuildProcessRunningXacts(SnapBuild *builder, XLogRecPtr lsn, xl_running_xact
*/
if (db_specific)
{
- /*
- * If we must only keep track of transactions running in the
- * current database, we need transaction info from exactly that
- * database.
- */
- if (running->dbid != MyDatabaseId)
- {
- LogStandbySnapshot(MyDatabaseId);
-
- return;
- }
+ Assert(running->dbid == MyDatabaseId);
/*
* We'd better be able to check during scan if the plugin does not
@@ -1198,16 +1221,6 @@ SnapBuildProcessRunningXacts(SnapBuild *builder, XLogRecPtr lsn, xl_running_xact
else
SnapBuildSerialize(builder, lsn);
- /*
- * Database specific transaction info may exist to reach CONSISTENT state
- * faster, however the code below makes no use of it. Moreover, such
- * record might cause problems because the following normal (cluster-wide)
- * record can have lower value of oldestRunningXid. In that case, let's
- * wait with the cleanup for the next regular cluster-wide record.
- */
- if (OidIsValid(running->dbid))
- return;
-
/*
* Update range of interesting xids based on the running xacts
* information. We don't increase ->xmax using it, because once we are in
--
2.47.3
--=-=-=--
^ permalink raw reply [nested|flat] 38+ messages in thread
end of thread, other threads:[~2026-05-06 07:38 UTC | newest]
Thread overview: 38+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2011-09-15 08:18 Patch for cursor calling with named parameters Yeb Havinga <[email protected]>
2011-09-15 14:31 ` Cédric Villemain <[email protected]>
2011-09-15 15:30 ` Yeb Havinga <[email protected]>
2011-10-06 14:04 [REVIEW] Patch for cursor calling with named parameters Royce Ausburn <[email protected]>
2011-10-06 16:23 ` Tom Lane <[email protected]>
2011-10-06 16:38 ` Robert Haas <[email protected]>
2011-10-06 16:46 ` Tom Lane <[email protected]>
2011-10-06 17:29 ` David E. Wheeler <[email protected]>
2011-10-06 17:37 ` Tom Lane <[email protected]>
2011-10-06 17:39 ` David E. Wheeler <[email protected]>
2011-10-06 17:46 ` Tom Lane <[email protected]>
2011-10-06 17:51 ` David E. Wheeler <[email protected]>
2011-10-06 17:52 ` Robert Haas <[email protected]>
2011-10-06 20:15 ` Pavel Stehule <[email protected]>
2011-10-07 02:54 ` Royce Ausburn <[email protected]>
2011-10-07 10:21 ` Yeb Havinga <[email protected]>
2011-10-07 14:56 ` Yeb Havinga <[email protected]>
2011-10-11 11:55 ` Royce Ausburn <[email protected]>
2011-10-11 12:38 ` Yeb Havinga <[email protected]>
2011-10-11 12:40 ` Royce Ausburn <[email protected]>
2011-10-15 05:41 ` Tom Lane <[email protected]>
2011-10-17 08:44 ` Yeb Havinga <[email protected]>
2011-11-14 14:45 ` Yeb Havinga <[email protected]>
2011-11-15 10:17 ` Yeb Havinga <[email protected]>
2011-12-03 20:53 Re: [REVIEW] Patch for cursor calling with named parameters Kevin Grittner <[email protected]>
2011-12-05 10:06 ` Yeb Havinga <[email protected]>
2011-12-05 15:34 ` Kevin Grittner <[email protected]>
2011-12-05 17:09 ` Kevin Grittner <[email protected]>
2011-12-06 16:58 ` Kevin Grittner <[email protected]>
2011-12-07 09:00 ` Yeb Havinga <[email protected]>
2011-12-11 15:26 ` Yeb Havinga <[email protected]>
2011-12-12 12:32 ` Yeb Havinga <[email protected]>
2011-12-12 19:55 Re: [REVIEW] Patch for cursor calling with named parameters Kevin Grittner <[email protected]>
2011-12-13 16:22 ` Heikki Linnakangas <[email protected]>
2011-12-13 17:34 ` Tom Lane <[email protected]>
2011-12-14 10:31 ` Yeb Havinga <[email protected]>
2011-12-14 13:58 ` Heikki Linnakangas <[email protected]>
2026-05-06 07:38 [PATCH] Distinguish properly when database-specific transaction list should be used. Antonin Houska <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox