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
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 $$
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
