From 4adead1d224278ff3064636063a818eba17cb211 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 14 Dec 2011 15:55:37 +0200 Subject: [PATCH] Add support for passing cursor parameters in named notation in PL/pgSQL. Yeb Havinga, reviewed by Kevin Grittner, with small changes by me. --- doc/src/sgml/plpgsql.sgml | 23 +++- src/pl/plpgsql/src/gram.y | 154 +++++++++++++++++++++++--- src/pl/plpgsql/src/pl_scanner.c | 30 +++++ src/pl/plpgsql/src/plpgsql.h | 2 + src/test/regress/expected/plpgsql.out | 129 +++++++++++++++++++++ src/test/regress/sql/plpgsql.sql | 108 ++++++++++++++++++ 6 files changed, 429 insertions(+), 17 deletions(-) diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index f33cef55ed..5a1e33fb4b 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -2823,11 +2823,11 @@ OPEN curs1 FOR EXECUTE 'SELECT * FROM ' || quote_ident(tabname) - + Opening a Bound Cursor -OPEN bound_cursorvar ( argument_values ) ; +OPEN bound_cursorvar ( argument_name := argument_value , ... ) ; @@ -2846,11 +2846,22 @@ OPEN bound_cursorvar ( argume behavior was already determined. + + Argument values can be passed using either positional + or named notation. In positional + notation, all arguments are specified in order. In named notation, + each argument's name is specified using := to + separate it from the argument expression. Similar to calling + functions, described in , it + is also allowed to mix positional and named notation. + + Examples (these use the cursor declaration examples above): OPEN curs2; OPEN curs3(42); +OPEN curs3(key := 42); @@ -3169,7 +3180,7 @@ COMMIT; <<label>> -FOR recordvar IN bound_cursorvar ( argument_values ) LOOP +FOR recordvar IN bound_cursorvar ( argument_name := argument_value , ... ) LOOP statements END LOOP label ; @@ -3180,7 +3191,11 @@ END LOOP label ; 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 OPEN. + the same way as during an OPEN (see ). + + + The variable recordvar is automatically defined as 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 index 8c4c2f71ca..c04afb50fe 100644 --- a/src/pl/plpgsql/src/gram.y +++ b/src/pl/plpgsql/src/gram.y @@ -67,6 +67,7 @@ static PLpgSQL_expr *read_sql_construct(int until, const char *sqlstart, bool isexpression, bool valid_sql, + bool trim, int *startloc, int *endtoken); static PLpgSQL_expr *read_sql_expression(int until, @@ -1313,6 +1314,7 @@ for_control : for_variable K_IN "SELECT ", true, false, + true, &expr1loc, &tok); @@ -1692,7 +1694,7 @@ stmt_raise : K_RAISE expr = read_sql_construct(',', ';', K_USING, ", or ; or USING", "SELECT ", - true, true, + true, true, true, NULL, &tok); new->params = lappend(new->params, expr); } @@ -1790,7 +1792,7 @@ stmt_dynexecute : K_EXECUTE expr = read_sql_construct(K_INTO, K_USING, ';', "INTO or USING or ;", "SELECT ", - true, true, + true, true, true, NULL, &endtoken); new = palloc(sizeof(PLpgSQL_stmt_dynexecute)); @@ -1829,7 +1831,7 @@ stmt_dynexecute : K_EXECUTE expr = read_sql_construct(',', ';', K_INTO, ", or ; or INTO", "SELECT ", - true, true, + true, true, true, NULL, &endtoken); new->params = lappend(new->params, expr); } while (endtoken == ','); @@ -2322,7 +2324,7 @@ static PLpgSQL_expr * read_sql_expression(int until, const char *expected) { return read_sql_construct(until, 0, 0, expected, - "SELECT ", true, true, NULL, NULL); + "SELECT ", true, true, true, NULL, NULL); } /* Convenience routine to read an expression with two possible terminators */ @@ -2331,7 +2333,7 @@ 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); + "SELECT ", true, true, true, NULL, endtoken); } /* Convenience routine to read a SQL statement that must end with ';' */ @@ -2339,7 +2341,7 @@ static PLpgSQL_expr * read_sql_stmt(const char *sqlstart) { return read_sql_construct(';', 0, 0, ";", - sqlstart, false, true, NULL, NULL); + sqlstart, false, true, true, NULL, NULL); } /* @@ -2352,6 +2354,7 @@ read_sql_stmt(const char *sqlstart) * 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) + * trim: 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,6 +2367,7 @@ read_sql_construct(int until, const char *sqlstart, bool isexpression, bool valid_sql, + bool trim, int *startloc, int *endtoken) { @@ -2443,8 +2447,11 @@ 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'; + 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; @@ -3375,15 +3382,23 @@ check_labels(const char *start_label, const char *end_label, int end_location) * 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). + * If it does have args, we expect to see "( arg [, arg ...] )" followed + * by the until token, where arg may be a plain expression, or a named + * parameter assignment of the form argname := 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,6 +3417,9 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected) 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,9 +3429,119 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected) parser_errposition(yylloc))); /* - * Read expressions until the matching ')'. + * Read the arguments, one by one. */ - expr = read_sql_expression(')', ")"); + 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(); diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c index 76e8436e50..e6dcaae7a5 100644 --- a/src/pl/plpgsql/src/pl_scanner.c +++ b/src/pl/plpgsql/src/pl_scanner.c @@ -423,6 +423,36 @@ plpgsql_append_source_text(StringInfo buf, endlocation - startlocation); } +/* + * 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. diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index c638f4323f..28460215e5 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -962,6 +962,8 @@ extern int plpgsql_yylex(void); 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); diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 238bf5f0ae..bdef259c7e 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2291,6 +2291,135 @@ select refcursor_test2(20000, 20000) as "Should be false", f | t (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 -- diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index b47c2de312..f577dc3cdc 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -1945,6 +1945,114 @@ $$ language plpgsql; select refcursor_test2(20000, 20000) as "Should be false", 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 --