From 15c72174f35d75d31efa35968e39f1bb09f5a9aa Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 12 Feb 2006 06:37:05 +0000 Subject: [PATCH] Apply code-reviewed version of for-scalar-list patch: mostly, fixing it to report reasonable errors in error cases. --- doc/src/sgml/plpgsql.sgml | 19 +-- src/pl/plpgsql/src/gram.y | 168 ++++++++++++++------------ src/test/regress/expected/plpgsql.out | 45 +++++-- src/test/regress/sql/plpgsql.sql | 23 ++-- 4 files changed, 157 insertions(+), 98 deletions(-) diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 0c3839cd4d..f0f5aec431 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -1,5 +1,5 @@ @@ -1968,10 +1968,12 @@ END LOOP label ; - This form of FOR creates a loop that iterates over a range of integer - values. The variable + This form of FOR creates a loop that iterates over a range + of integer values. The variable name is automatically defined as type - integer and exists only inside the loop. The two expressions giving + integer and exists only inside the loop (any existing + definition of the variable name is ignored within the loop). + The two expressions giving the lower and upper bound of the range are evaluated once when entering the loop. The iteration step is normally 1, but is -1 when REVERSE is specified. @@ -2012,9 +2014,9 @@ FOR target IN query LOOP statements END LOOP label ; - Target is a record variable, row variable, - or a comma-separated list of simple variables and record/row fields - which is successively assigned each row + The target is a record variable, row variable, + or comma-separated list of scalar variables. + The target is successively assigned each row resulting from the query (which must be a SELECT command) and the loop body is executed for each row. Here is an example: @@ -2069,7 +2071,8 @@ END LOOP label ; IN and LOOP. If .. is not seen then the loop is presumed to be a loop over rows. Mistyping the .. is thus likely to lead to a complaint along the lines of - loop variable of loop over rows must be a record or row or scalar variable, + loop variable of loop over rows must be a record or row variable + or list of scalar variables, rather than the simple syntax error one might expect to get. diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y index a23e57a158..0dcafb46ba 100644 --- a/src/pl/plpgsql/src/gram.y +++ b/src/pl/plpgsql/src/gram.y @@ -4,7 +4,7 @@ * procedural language * * IDENTIFICATION - * $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.84 2006/02/12 06:03:38 momjian Exp $ + * $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.85 2006/02/12 06:37:05 tgl Exp $ * * This software is copyrighted by Jan Wieck - Hamburg. * @@ -54,13 +54,14 @@ static PLpgSQL_stmt *make_fetch_stmt(void); static void check_assignable(PLpgSQL_datum *datum); static PLpgSQL_row *read_into_scalar_list(const char *initial_name, PLpgSQL_datum *initial_datum); +static PLpgSQL_row *make_scalar_list1(const char *initial_name, + PLpgSQL_datum *initial_datum, + int lineno); static void check_sql_expr(const char *stmt); static void plpgsql_sql_error_callback(void *arg); static void check_labels(const char *start_label, const char *end_label); -static PLpgSQL_row *make_scalar_list1(const char *name, - PLpgSQL_datum *variable); - + %} %union { @@ -76,9 +77,9 @@ static PLpgSQL_row *make_scalar_list1(const char *name, { char *name; int lineno; + PLpgSQL_datum *scalar; PLpgSQL_rec *rec; PLpgSQL_row *row; - PLpgSQL_datum *scalar; } forvariable; struct { @@ -895,13 +896,14 @@ for_control : } else if ($2.scalar) { - new->row = make_scalar_list1($2.name, $2.scalar); - check_assignable((PLpgSQL_datum *) new->row); + /* convert single scalar to list */ + new->row = make_scalar_list1($2.name, $2.scalar, $2.lineno); + /* no need for check_assignable */ } else { - plpgsql_error_lineno = $1; - yyerror("loop variable of loop over rows must be a record, row, or scalar variable"); + plpgsql_error_lineno = $2.lineno; + yyerror("loop variable of loop over rows must be a record or row variable or list of scalar variables"); } new->query = expr; @@ -950,24 +952,24 @@ for_control : PLpgSQL_expr *expr2; PLpgSQL_var *fvar; PLpgSQL_stmt_fori *new; + char *varname; /* First expression is well-formed */ check_sql_expr(expr1->query); expr2 = plpgsql_read_expression(K_LOOP, "LOOP"); - /* T_SCALAR identifier waits for converting */ - if ($2.scalar) - { - char *name; - plpgsql_convert_ident($2.name, &name, 1); - pfree($2.name); - $2.name = name; - } + /* should have had a single variable name */ + plpgsql_error_lineno = $2.lineno; + if ($2.scalar && $2.row) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("integer FOR loop must have just one target variable"))); /* create loop's private variable */ + plpgsql_convert_ident($2.name, &varname, 1); fvar = (PLpgSQL_var *) - plpgsql_build_variable($2.name, + plpgsql_build_variable(varname, $2.lineno, plpgsql_build_datatype(INT4OID, -1), @@ -1021,13 +1023,14 @@ for_control : } else if ($2.scalar) { - new->row = make_scalar_list1($2.name, $2.scalar); - check_assignable((PLpgSQL_datum *) new->row); + /* convert single scalar to list */ + new->row = make_scalar_list1($2.name, $2.scalar, $2.lineno); + /* no need for check_assignable */ } else { - plpgsql_error_lineno = $1; - yyerror("loop variable of loop over rows must be record, row, or scalar variable"); + plpgsql_error_lineno = $2.lineno; + yyerror("loop variable of loop over rows must be a record or row variable or list of scalar variables"); } new->query = expr1; @@ -1047,55 +1050,63 @@ for_control : * if any, because that's what we need for the loop-over-query case. Note * that we must NOT apply check_assignable() or any other semantic check * until we know what's what. + * + * However, if we see a comma-separated list of names, we know that it + * can't be an integer FOR loop and so it's OK to check the variables + * immediately. In particular, for T_WORD followed by comma, we should + * complain that the name is not known rather than say it's a syntax error. + * Note that the non-error result of this case sets *both* $$.scalar and + * $$.row; see the for_control production. */ for_variable : T_SCALAR - { - int tok; - char *name; - - name = pstrdup(yytext); - $$.scalar = yylval.scalar; - $$.lineno = plpgsql_scanner_lineno(); + { + int tok; - if((tok = yylex()) == ',') - { - plpgsql_push_back_token(tok); - $$.name = NULL; - $$.row = read_into_scalar_list(name, $$.scalar); - $$.rec = NULL; - $$.scalar = NULL; - - pfree(name); - } - else - { - plpgsql_push_back_token(tok); - $$.name = name; - $$.row = NULL; - $$.rec = NULL; - } + $$.name = pstrdup(yytext); + $$.lineno = plpgsql_scanner_lineno(); + $$.scalar = yylval.scalar; + $$.rec = NULL; + $$.row = NULL; + /* check for comma-separated list */ + tok = yylex(); + plpgsql_push_back_token(tok); + if (tok == ',') + $$.row = read_into_scalar_list($$.name, $$.scalar); } | T_WORD { - char *name; + int tok; - plpgsql_convert_ident(yytext, &name, 1); - $$.name = name; + $$.name = pstrdup(yytext); $$.lineno = plpgsql_scanner_lineno(); + $$.scalar = NULL; $$.rec = NULL; $$.row = NULL; + /* check for comma-separated list */ + tok = yylex(); + plpgsql_push_back_token(tok); + if (tok == ',') + { + plpgsql_error_lineno = $$.lineno; + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("\"%s\" is not a scalar variable", + $$.name))); + } } | T_RECORD { - $$.name = NULL; + $$.name = pstrdup(yytext); $$.lineno = plpgsql_scanner_lineno(); + $$.scalar = NULL; $$.rec = yylval.rec; $$.row = NULL; } | T_ROW { - $$.name = NULL; + $$.name = pstrdup(yytext); $$.lineno = plpgsql_scanner_lineno(); + $$.scalar = NULL; $$.row = yylval.row; $$.rec = NULL; } @@ -2121,30 +2132,6 @@ make_fetch_stmt(void) } -static PLpgSQL_row * -make_scalar_list1(const char *name, - PLpgSQL_datum *variable) -{ - PLpgSQL_row *row; - check_assignable(variable); - - row = palloc(sizeof(PLpgSQL_row)); - row->dtype = PLPGSQL_DTYPE_ROW; - row->refname = pstrdup("*internal*"); - row->lineno = plpgsql_scanner_lineno(); - row->rowtupdesc = NULL; - row->nfields = 1; - row->fieldnames = palloc(sizeof(char *) * 1); - row->varnos = palloc(sizeof(int) * 1); - row->fieldnames[0] = pstrdup(name); - row->varnos[0] = variable->dno; - - plpgsql_adddatum((PLpgSQL_datum *)row); - - return row; -} - - static void check_assignable(PLpgSQL_datum *datum) { @@ -2256,6 +2243,37 @@ read_into_scalar_list(const char *initial_name, return row; } +/* + * Convert a single scalar into a "row" list. This is exactly + * like read_into_scalar_list except we never consume any input. + * In fact, since this can be invoked long after the source + * input was actually read, the lineno has to be passed in. + */ +static PLpgSQL_row * +make_scalar_list1(const char *initial_name, + PLpgSQL_datum *initial_datum, + int lineno) +{ + PLpgSQL_row *row; + + check_assignable(initial_datum); + + row = palloc(sizeof(PLpgSQL_row)); + row->dtype = PLPGSQL_DTYPE_ROW; + row->refname = pstrdup("*internal*"); + row->lineno = lineno; + row->rowtupdesc = NULL; + row->nfields = 1; + row->fieldnames = palloc(sizeof(char *)); + row->varnos = palloc(sizeof(int)); + row->fieldnames[0] = pstrdup(initial_name); + row->varnos[0] = initial_datum->dno; + + plpgsql_adddatum((PLpgSQL_datum *)row); + + return row; +} + /* * When the PL/PgSQL parser expects to see a SQL statement, it is very * liberal in what it accepts; for example, we often assume an diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 3bcafcfb4d..1a4307dce2 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2722,22 +2722,53 @@ $$ language plpgsql; ERROR: end label "outer_label" specified for unlabelled block CONTEXT: compile of PL/pgSQL function "end_label4" near line 5 -- using list of scalars in fori and fore stmts -create function for_vect() returns void as $$ +create function for_vect() returns void as $proc$ <>declare a integer; b varchar; c varchar; r record; begin - -- old fori - for i in 1 .. 10 loop + -- fori + for i in 1 .. 3 loop raise notice '%', i; end loop; - for a in select 1 from generate_series(1,4) loop + -- fore with record var + for r in select gs as aa, 'BB' as bb, 'CC' as cc from generate_series(1,4) gs loop + raise notice '% % %', r.aa, r.bb, r.cc; + end loop; + -- fore with single scalar + for a in select gs from generate_series(1,4) gs loop raise notice '%', a; end loop; - for a,b,c in select generate_series, 'BB','CC' from generate_series(1,4) loop + -- fore with multiple scalars + for a,b,c in select gs, 'BB','CC' from generate_series(1,4) gs loop raise notice '% % %', a, b, c; end loop; -- using qualified names in fors, fore is enabled, disabled only for fori - for lbl.a, lbl.b, lbl.c in execute E'select generate_series, \'bb\',\'cc\' from generate_series(1,4)' loop + for lbl.a, lbl.b, lbl.c in execute $$select gs, 'bb','cc' from generate_series(1,4) gs$$ loop raise notice '% % %', a, b, c; end loop; end; -$$ language plpgsql; +$proc$ language plpgsql; +select for_vect(); +NOTICE: 1 +NOTICE: 2 +NOTICE: 3 +NOTICE: 1 BB CC +NOTICE: 2 BB CC +NOTICE: 3 BB CC +NOTICE: 4 BB CC +NOTICE: 1 +NOTICE: 2 +NOTICE: 3 +NOTICE: 4 +NOTICE: 1 BB CC +NOTICE: 2 BB CC +NOTICE: 3 BB CC +NOTICE: 4 BB CC +NOTICE: 1 bb cc +NOTICE: 2 bb cc +NOTICE: 3 bb cc +NOTICE: 4 bb cc + for_vect +---------- + +(1 row) + diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 63b8bef14e..5e8db47572 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -2281,24 +2281,31 @@ begin end; $$ language plpgsql; - -- using list of scalars in fori and fore stmts -create function for_vect() returns void as $$ +create function for_vect() returns void as $proc$ <>declare a integer; b varchar; c varchar; r record; begin - -- old fori - for i in 1 .. 10 loop + -- fori + for i in 1 .. 3 loop raise notice '%', i; end loop; - for a in select 1 from generate_series(1,4) loop + -- fore with record var + for r in select gs as aa, 'BB' as bb, 'CC' as cc from generate_series(1,4) gs loop + raise notice '% % %', r.aa, r.bb, r.cc; + end loop; + -- fore with single scalar + for a in select gs from generate_series(1,4) gs loop raise notice '%', a; end loop; - for a,b,c in select generate_series, 'BB','CC' from generate_series(1,4) loop + -- fore with multiple scalars + for a,b,c in select gs, 'BB','CC' from generate_series(1,4) gs loop raise notice '% % %', a, b, c; end loop; -- using qualified names in fors, fore is enabled, disabled only for fori - for lbl.a, lbl.b, lbl.c in execute E'select generate_series, \'bb\',\'cc\' from generate_series(1,4)' loop + for lbl.a, lbl.b, lbl.c in execute $$select gs, 'bb','cc' from generate_series(1,4) gs$$ loop raise notice '% % %', a, b, c; end loop; end; -$$ language plpgsql; +$proc$ language plpgsql; + +select for_vect(); -- 2.49.0