]> granicus.if.org Git - postgresql/commitdiff
Okay, I've had it with answering newbie questions about why plpgsql
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 4 Jul 2004 02:49:04 +0000 (02:49 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 4 Jul 2004 02:49:04 +0000 (02:49 +0000)
FOR loops are giving weird syntax errors.  Restructure parsing of FOR
loops so that the integer-loop-vs-query-loop decision is driven off
the presence of '..' between IN and LOOP, rather than the presence
of a matching record/row variable name.  Hopefully this will make the
behavior a bit more transparent.

doc/src/sgml/plpgsql.sgml
src/pl/plpgsql/src/gram.y

index b8eadd04552e328d93c879c08fec0c2036bd52b9..952351170cc2c3e161aedc866ccb507b84b0e37e 100644 (file)
@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.38 2004/05/16 23:22:06 neilc Exp $
+$PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.39 2004/07/04 02:48:52 tgl Exp $
 -->
 
 <chapter id="plpgsql"> 
@@ -1769,7 +1769,7 @@ END;
 
     <para>
      The <literal>FOR-IN-EXECUTE</> statement is another way to iterate over
-     records:
+     rows:
 <synopsis>
 <optional>&lt;&lt;<replaceable>label</replaceable>&gt;&gt;</optional>
 FOR <replaceable>record_or_row</replaceable> IN EXECUTE <replaceable>text_expression</replaceable> LOOP 
@@ -1788,13 +1788,12 @@ END LOOP;
     <para>
      The <application>PL/pgSQL</> parser presently distinguishes the
      two kinds of <literal>FOR</> loops (integer or query result) by checking
-     whether the target variable mentioned just after <literal>FOR</> has been
-     declared as a record or row variable.  If not, it's presumed to be
-     an integer <literal>FOR</> loop.  This can cause rather nonintuitive error
-     messages when the true problem is, say, that one has
-     misspelled the variable name after the <literal>FOR</>.  Typically
-     the complaint will be something like <literal>missing ".." at end of SQL
-     expression</>.
+     whether <literal>..</> appears outside any parentheses between
+     <literal>IN</> and <literal>LOOP</>.  If <literal>..</> is not seen then
+     the loop is presumed to be a loop over rows.  Mistyping the <literal>..</>
+     is thus likely to lead to a complaint along the lines of
+     <quote>loop variable of loop over rows must be a record or row</>,
+     rather than the simple syntax error one might expect to get.
     </para>
     </note>
   </sect2>
index 826e1539d1e25cb68f4ae24b2f9586992c7f73fd..9b70d944ab98aea9bba481ca119c7b428ad5955f 100644 (file)
@@ -4,7 +4,7 @@
  *                                               procedural language
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.56 2004/06/04 02:37:06 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.57 2004/07/04 02:49:04 tgl Exp $
  *
  *       This software is copyrighted by Jan Wieck - Hamburg.
  *
 
 
 static PLpgSQL_expr    *read_sql_construct(int until,
+                                                                                       int until2,
                                                                                        const char *expected,
                                                                                        bool isexpression,
-                                                                                       const char *sqlstart);
+                                                                                       const char *sqlstart,
+                                                                                       int *endtoken);
 static PLpgSQL_expr    *read_sql_stmt(const char *sqlstart);
 static PLpgSQL_type    *read_datatype(int tok);
 static PLpgSQL_stmt    *make_select_stmt(void);
@@ -73,9 +75,11 @@ static       void check_assignable(PLpgSQL_datum *datum);
                }                                               dtlist;
                struct
                {
-                       int  reverse;
-                       PLpgSQL_expr *expr;
-               }                                               forilow;
+                       char *name;
+                       int  lineno;
+                       PLpgSQL_rec     *rec;
+                       PLpgSQL_row     *row;
+               }                                               forvariable;
                struct
                {
                        char *label;
@@ -110,11 +114,10 @@ static    void check_assignable(PLpgSQL_datum *datum);
 %type <expr>   opt_exitcond
 
 %type <ival>   assign_var cursor_variable
-%type <var>            fori_var cursor_varptr
+%type <var>            cursor_varptr
 %type <variable>       decl_cursor_arg
-%type <varname> fori_varname
-%type <forilow> fori_lower
-%type <rec>            fors_target
+%type <forvariable>    for_variable
+%type <stmt>   for_control
 
 %type <str>            opt_lblname opt_label
 %type <str>            opt_exitlabel
@@ -124,8 +127,8 @@ static      void check_assignable(PLpgSQL_datum *datum);
 %type <stmt>   proc_stmt pl_block
 %type <stmt>   stmt_assign stmt_if stmt_loop stmt_while stmt_exit
 %type <stmt>   stmt_return stmt_return_next stmt_raise stmt_execsql
-%type <stmt>   stmt_fori stmt_fors stmt_select stmt_perform
-%type <stmt>   stmt_dynexecute stmt_dynfors stmt_getdiag
+%type <stmt>   stmt_for stmt_select stmt_perform
+%type <stmt>   stmt_dynexecute stmt_getdiag
 %type <stmt>   stmt_open stmt_fetch stmt_close
 
 %type <intlist>        raise_params
@@ -616,9 +619,7 @@ proc_stmt           : pl_block ';'
                                                { $$ = $1; }
                                | stmt_while
                                                { $$ = $1; }
-                               | stmt_fori
-                                               { $$ = $1; }
-                               | stmt_fors
+                               | stmt_for
                                                { $$ = $1; }
                                | stmt_select
                                                { $$ = $1; }
@@ -634,8 +635,6 @@ proc_stmt           : pl_block ';'
                                                { $$ = $1; }
                                | stmt_dynexecute
                                                { $$ = $1; }
-                               | stmt_dynfors
-                                               { $$ = $1; }
                                | stmt_perform
                                                { $$ = $1; }
                                | stmt_getdiag
@@ -882,152 +881,218 @@ stmt_while              : opt_label K_WHILE lno expr_until_loop loop_body
                                        }
                                ;
 
-stmt_fori              : opt_label K_FOR lno fori_var K_IN fori_lower expr_until_loop loop_body
+stmt_for               : opt_label K_FOR for_control loop_body
                                        {
-                                               PLpgSQL_stmt_fori               *new;
+                                               /* This runs after we've scanned the loop body */
+                                               if ($3->cmd_type == PLPGSQL_STMT_FORI)
+                                               {
+                                                       PLpgSQL_stmt_fori               *new;
 
-                                               new = malloc(sizeof(PLpgSQL_stmt_fori));
-                                               memset(new, 0, sizeof(PLpgSQL_stmt_fori));
+                                                       new = (PLpgSQL_stmt_fori *) $3;
+                                                       new->label        = $1;
+                                                       new->body         = $4;
+                                                       $$ = (PLpgSQL_stmt *) new;
+                                               }
+                                               else if ($3->cmd_type == PLPGSQL_STMT_FORS)
+                                               {
+                                                       PLpgSQL_stmt_fors               *new;
 
-                                               new->cmd_type = PLPGSQL_STMT_FORI;
-                                               new->lineno   = $3;
-                                               new->label        = $1;
-                                               new->var          = $4;
-                                               new->reverse  = $6.reverse;
-                                               new->lower        = $6.expr;
-                                               new->upper        = $7;
-                                               new->body         = $8;
+                                                       new = (PLpgSQL_stmt_fors *) $3;
+                                                       new->label        = $1;
+                                                       new->body         = $4;
+                                                       $$ = (PLpgSQL_stmt *) new;
+                                               }
+                                               else
+                                               {
+                                                       PLpgSQL_stmt_dynfors    *new;
 
-                                               plpgsql_ns_pop();
+                                                       Assert($3->cmd_type == PLPGSQL_STMT_DYNFORS);
+                                                       new = (PLpgSQL_stmt_dynfors *) $3;
+                                                       new->label        = $1;
+                                                       new->body         = $4;
+                                                       $$ = (PLpgSQL_stmt *) new;
+                                               }
 
-                                               $$ = (PLpgSQL_stmt *)new;
+                                               /* close namespace started in opt_label */
+                                               plpgsql_ns_pop();
                                        }
                                ;
 
-fori_var               : fori_varname
+for_control            : lno for_variable K_IN
                                        {
-                                               PLpgSQL_var             *new;
+                                               int                     tok;
+                                               bool            reverse = false;
+                                               bool            execute = false;
+                                               PLpgSQL_expr *expr1;
 
-                                               new = (PLpgSQL_var *)
-                                                       plpgsql_build_variable($1.name, $1.lineno,
-                                                                                                  plpgsql_build_datatype(INT4OID,
-                                                                                                                                                 -1),
-                                                                                                  true);
+                                               /* check for REVERSE and EXECUTE */
+                                               tok = yylex();
+                                               if (tok == K_REVERSE)
+                                               {
+                                                       reverse = true;
+                                                       tok = yylex();
+                                               }
 
-                                               plpgsql_add_initdatums(NULL);
+                                               if (tok == K_EXECUTE)
+                                                       execute = true;
+                                               else
+                                                       plpgsql_push_back_token(tok);
 
-                                               $$ = new;
-                                       }
-                               ;
+                                               /* Collect one or two expressions */
+                                               expr1 = read_sql_construct(K_DOTDOT,
+                                                                                                  K_LOOP,
+                                                                                                  "LOOP",
+                                                                                                  true,
+                                                                                                  "SELECT ",
+                                                                                                  &tok);
 
-fori_varname   : T_SCALAR
-                                       {
-                                               char    *name;
+                                               if (tok == K_DOTDOT)
+                                               {
+                                                       /* Found .., so it must be an integer loop */
+                                                       PLpgSQL_stmt_fori       *new;
+                                                       PLpgSQL_expr            *expr2;
+                                                       PLpgSQL_var                     *fvar;
 
-                                               plpgsql_convert_ident(yytext, &name, 1);
-                                               /* name should be malloc'd for use as varname */
-                                               $$.name = strdup(name);
-                                               $$.lineno  = plpgsql_scanner_lineno();
-                                               pfree(name);
-                                       }
-                               | T_WORD
-                                       {
-                                               char    *name;
+                                                       expr2 = plpgsql_read_expression(K_LOOP, "LOOP");
 
-                                               plpgsql_convert_ident(yytext, &name, 1);
-                                               /* name should be malloc'd for use as varname */
-                                               $$.name = strdup(name);
-                                               $$.lineno  = plpgsql_scanner_lineno();
-                                               pfree(name);
-                                       }
-                               ;
+                                                       if (execute)
+                                                       {
+                                                               plpgsql_error_lineno = $1;
+                                                               yyerror("cannot specify EXECUTE in integer for-loop");
+                                                       }
 
-fori_lower             :
-                                       {
-                                               int                     tok;
+                                                       /* name should be malloc'd for use as varname */
+                                                       fvar = (PLpgSQL_var *)
+                                                               plpgsql_build_variable(strdup($2.name),
+                                                                                                          $2.lineno,
+                                                                                                          plpgsql_build_datatype(INT4OID,
+                                                                                                                                                         -1),
+                                                                                                          true);
 
-                                               tok = yylex();
-                                               if (tok == K_REVERSE)
-                                               {
-                                                       $$.reverse = 1;
+                                                       /* put the for-variable into the local block */
+                                                       plpgsql_add_initdatums(NULL);
+
+                                                       new = malloc(sizeof(PLpgSQL_stmt_fori));
+                                                       memset(new, 0, sizeof(PLpgSQL_stmt_fori));
+
+                                                       new->cmd_type = PLPGSQL_STMT_FORI;
+                                                       new->lineno   = $1;
+                                                       new->var          = fvar;
+                                                       new->reverse  = reverse;
+                                                       new->lower        = expr1;
+                                                       new->upper        = expr2;
+
+                                                       $$ = (PLpgSQL_stmt *) new;
                                                }
-                                               else
+                                               else if (execute)
                                                {
-                                                       $$.reverse = 0;
-                                                       plpgsql_push_back_token(tok);
-                                               }
+                                                       /* No .., so it must be a loop over rows */
+                                                       PLpgSQL_stmt_dynfors    *new;
 
-                                               $$.expr = plpgsql_read_expression(K_DOTDOT, "..");
-                                       }
-                               ;
+                                                       if (reverse)
+                                                       {
+                                                               plpgsql_error_lineno = $1;
+                                                               yyerror("cannot specify REVERSE in loop over rows");
+                                                       }
 
-stmt_fors              : opt_label K_FOR lno fors_target K_IN K_SELECT expr_until_loop loop_body
-                                       {
-                                               PLpgSQL_stmt_fors               *new;
+                                                       new = malloc(sizeof(PLpgSQL_stmt_dynfors));
+                                                       memset(new, 0, sizeof(PLpgSQL_stmt_dynfors));
 
-                                               new = malloc(sizeof(PLpgSQL_stmt_fors));
-                                               memset(new, 0, sizeof(PLpgSQL_stmt_fors));
+                                                       new->cmd_type = PLPGSQL_STMT_DYNFORS;
+                                                       new->lineno   = $1;
+                                                       if ($2.rec)
+                                                               new->rec = $2.rec;
+                                                       else if ($2.row)
+                                                               new->row = $2.row;
+                                                       else
+                                                       {
+                                                               plpgsql_error_lineno = $1;
+                                                               yyerror("loop variable of loop over rows must be a record or row variable");
+                                                       }
+                                                       new->query = expr1;
 
-                                               new->cmd_type = PLPGSQL_STMT_FORS;
-                                               new->lineno   = $3;
-                                               new->label        = $1;
-                                               switch ($4->dtype)
-                                               {
-                                                       case PLPGSQL_DTYPE_REC:
-                                                               new->rec = $4;
-                                                               break;
-                                                       case PLPGSQL_DTYPE_ROW:
-                                                               new->row = (PLpgSQL_row *)$4;
-                                                               break;
-                                                       default:
-                                                               elog(ERROR, "unrecognized dtype: %d",
-                                                                        $4->dtype);
+                                                       $$ = (PLpgSQL_stmt *) new;
                                                }
-                                               new->query = $7;
-                                               new->body  = $8;
+                                               else
+                                               {
+                                                       /* No .., so it must be a loop over rows */
+                                                       PLpgSQL_stmt_fors               *new;
+                                                       char                                    *newquery;
 
-                                               plpgsql_ns_pop();
+                                                       if (reverse)
+                                                       {
+                                                               plpgsql_error_lineno = $1;
+                                                               yyerror("cannot specify REVERSE in loop over rows");
+                                                       }
 
-                                               $$ = (PLpgSQL_stmt *)new;
-                                       }
-                               ;
+                                                       new = malloc(sizeof(PLpgSQL_stmt_fors));
+                                                       memset(new, 0, sizeof(PLpgSQL_stmt_fors));
 
-stmt_dynfors : opt_label K_FOR lno fors_target K_IN K_EXECUTE expr_until_loop loop_body
-                                       {
-                                               PLpgSQL_stmt_dynfors    *new;
+                                                       new->cmd_type = PLPGSQL_STMT_FORS;
+                                                       new->lineno   = $1;
+                                                       if ($2.rec)
+                                                               new->rec = $2.rec;
+                                                       else if ($2.row)
+                                                               new->row = $2.row;
+                                                       else
+                                                       {
+                                                               plpgsql_error_lineno = $1;
+                                                               yyerror("loop variable of loop over rows must be a record or row variable");
+                                                       }
+                                                       /*
+                                                        * Must get rid of the "SELECT " we prepended
+                                                        * to expr1's text
+                                                        */
+                                                       newquery = strdup(expr1->query + 7);
+                                                       free(expr1->query);
+                                                       expr1->query = newquery;
 
-                                               new = malloc(sizeof(PLpgSQL_stmt_dynfors));
-                                               memset(new, 0, sizeof(PLpgSQL_stmt_dynfors));
+                                                       new->query = expr1;
 
-                                               new->cmd_type = PLPGSQL_STMT_DYNFORS;
-                                               new->lineno   = $3;
-                                               new->label        = $1;
-                                               switch ($4->dtype)
-                                               {
-                                                       case PLPGSQL_DTYPE_REC:
-                                                               new->rec = $4;
-                                                               break;
-                                                       case PLPGSQL_DTYPE_ROW:
-                                                               new->row = (PLpgSQL_row *)$4;
-                                                               break;
-                                                       default:
-                                                               elog(ERROR, "unrecognized dtype: %d",
-                                                                        $4->dtype);
+                                                       $$ = (PLpgSQL_stmt *) new;
                                                }
-                                               new->query = $7;
-                                               new->body  = $8;
+                                       }
+                               ;
 
-                                               plpgsql_ns_pop();
+for_variable   : T_SCALAR
+                                       {
+                                               char            *name;
 
-                                               $$ = (PLpgSQL_stmt *)new;
+                                               plpgsql_convert_ident(yytext, &name, 1);
+                                               $$.name = name;
+                                               $$.lineno  = plpgsql_scanner_lineno();
+                                               $$.rec = NULL;
+                                               $$.row = NULL;
                                        }
-                               ;
+                               | T_WORD
+                                       {
+                                               char            *name;
+
+                                               plpgsql_convert_ident(yytext, &name, 1);
+                                               $$.name = name;
+                                               $$.lineno  = plpgsql_scanner_lineno();
+                                               $$.rec = NULL;
+                                               $$.row = NULL;
+                                       }
+                               | T_RECORD
+                                       {
+                                               char            *name;
 
-fors_target            : T_RECORD
-                                       { $$ = yylval.rec; }
+                                               plpgsql_convert_ident(yytext, &name, 1);
+                                               $$.name = name;
+                                               $$.lineno  = plpgsql_scanner_lineno();
+                                               $$.rec = yylval.rec;
+                                               $$.row = NULL;
+                                       }
                                | T_ROW
                                        {
-                                               $$ = (PLpgSQL_rec *)(yylval.row);
+                                               char            *name;
+
+                                               plpgsql_convert_ident(yytext, &name, 1);
+                                               $$.name = name;
+                                               $$.lineno  = plpgsql_scanner_lineno();
+                                               $$.row = yylval.row;
+                                               $$.rec = NULL;
                                        }
                                ;
 
@@ -1521,20 +1586,33 @@ lno                             :
 PLpgSQL_expr *
 plpgsql_read_expression(int until, const char *expected)
 {
-       return read_sql_construct(until, expected, true, "SELECT ");
+       return read_sql_construct(until, 0, expected, true, "SELECT ", NULL);
 }
 
 static PLpgSQL_expr *
 read_sql_stmt(const char *sqlstart)
 {
-       return read_sql_construct(';', ";", false, sqlstart);
+       return read_sql_construct(';', 0, ";", false, sqlstart, NULL);
 }
 
+/*
+ * Read a SQL construct and build a PLpgSQL_expr for it.
+ *
+ * until:              token code for expected terminator
+ * until2:             token code for alternate terminator (pass 0 if none)
+ * expected:   text to use in complaining that terminator was not found
+ * isexpression: whether to say we're reading an "expression" or a "statement"
+ * sqlstart:   text to prefix to the accumulated SQL text
+ * endtoken:   if not NULL, ending token is stored at *endtoken
+ *                             (this is only interesting if until2 isn't zero)
+ */
 static PLpgSQL_expr *
 read_sql_construct(int until,
+                                  int until2,
                                   const char *expected,
                                   bool isexpression,
-                                  const char *sqlstart)
+                                  const char *sqlstart,
+                                  int *endtoken)
 {
        int                                     tok;
        int                                     lno;
@@ -1554,6 +1632,8 @@ read_sql_construct(int until,
                tok = yylex();
                if (tok == until && parenlevel == 0)
                        break;
+               if (tok == until2 && parenlevel == 0)
+                       break;
                if (tok == '(' || tok == '[')
                        parenlevel++;
                else if (tok == ')' || tok == ']')
@@ -1586,7 +1666,6 @@ read_sql_construct(int until,
                                                (errcode(ERRCODE_SYNTAX_ERROR),
                                                 errmsg("missing \"%s\" at end of SQL statement",
                                                                expected)));
-                       break;
                }
                if (plpgsql_SpaceScanned)
                        plpgsql_dstring_append(&ds, " ");
@@ -1616,6 +1695,9 @@ read_sql_construct(int until,
                }
        }
 
+       if (endtoken)
+               *endtoken = tok;
+
        expr = malloc(sizeof(PLpgSQL_expr) + sizeof(int) * nparams - sizeof(int));
        expr->dtype                     = PLPGSQL_DTYPE_EXPR;
        expr->query                     = strdup(plpgsql_dstring_get(&ds));