Check number of parameters in RAISE statement at compile time.

The number of % parameter markers in RAISE statement should match the number
of parameters given. We used to check that at execution time, but we have
all the information needed at compile time, so let's check it at compile
time instead. It's generally better to find mistakes earlier.

Marko Tiikkaja, reviewed by Fabien Coelho

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 91fadb0f14d8802dbff7dabbb5b455b1ad22fc0c..f008e937eefcbe9487c1b298ef11c467e7e591f6 100644 (file)
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3403,6 +3403,9 @@ RAISE ;
     Inside the format string, <literal>%</literal> is replaced by the
     string representation of the next optional argument's value. Write
     <literal>%%</literal> to emit a literal <literal>%</literal>.
+    The number of arguments must match the number of <literal>%</>
+    placeholders in the format string, or an error is raised during
+    the compilation of the function.
    </para>
 
    <para>

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 69d1965253d878b1ba3946334973679dd6913e63..11cb47b5225d987b815d652adc77247b63e2022c 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2939,10 +2939,9 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
                                        continue;
                                }
 
+                               /* should have been checked at compile time */
                                if (current_param == NULL)
-                                       ereport(ERROR,
-                                                       (errcode(ERRCODE_SYNTAX_ERROR),
-                                                 errmsg("too few parameters specified for RAISE")));
+                                       elog(ERROR, "unexpected RAISE parameter list length");
 
                                paramvalue = exec_eval_expr(estate,
                                                                          (PLpgSQL_expr *) lfirst(current_param),
@@ -2963,14 +2962,9 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
                                appendStringInfoChar(&ds, cp[0]);
                }
 
-               /*
-                * If more parameters were specified than were required to process the
-                * format string, throw an error
-                */
+               /* should have been checked at compile time */
                if (current_param != NULL)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_SYNTAX_ERROR),
-                                        errmsg("too many parameters specified for RAISE")));
+                       elog(ERROR, "unexpected RAISE parameter list length");
 
                err_message = ds.data;
                /* No pfree(ds.data), the pfree(err_message) does it */

diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index e3a992cf0ff57fc1e2cfc455fd4c2390f666ed1c..893f3a486f52366af9302cf31c4cf132977cb695 100644 (file)
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -106,6 +106,7 @@ static      void                     check_labels(const char *start_label,
 static PLpgSQL_expr    *read_cursor_args(PLpgSQL_var *cursor,
                                                                                  int until, const char *expected);
 static List                    *read_raise_options(void);
+static void                    check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 
 %}
 
@@ -1849,6 +1850,8 @@ stmt_raise                : K_RAISE
                                                                new->options = read_raise_options();
                                                }
 
+                                               check_raise_parameters(new);
+
                                                $$ = (PLpgSQL_stmt *)new;
                                        }
                                ;
@@ -3767,6 +3770,41 @@ read_raise_options(void)
        return result;
 }
 
+/*
+ * Check that the number of parameter placeholders in the message matches the
+ * number of parameters passed to it, if a message was given.
+ */
+static void
+check_raise_parameters(PLpgSQL_stmt_raise *stmt)
+{
+       char       *cp;
+       int                     expected_nparams = 0;
+
+       if (stmt->message == NULL)
+               return;
+
+       for (cp = stmt->message; *cp; cp++)
+       {
+               if (cp[0] == '%')
+               {
+                       /* ignore literal % characters */
+                       if (cp[1] == '%')
+                               cp++;
+                       else
+                               expected_nparams++;
+               }
+       }
+
+       if (expected_nparams < list_length(stmt->params))
+               ereport(ERROR,
+                               (errcode(ERRCODE_SYNTAX_ERROR),
+                               errmsg("too many parameters specified for RAISE")));
+       if (expected_nparams > list_length(stmt->params))
+               ereport(ERROR,
+                               (errcode(ERRCODE_SYNTAX_ERROR),
+                               errmsg("too few parameters specified for RAISE")));
+}
+
 /*
  * Fix up CASE statement
  */

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 8892bb417d39a020c14b4f4d550f1bfa0c9597a3..983f1b8b5a4e65713c4d01d9bebfa9b7bcfe1633 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2446,18 +2446,29 @@ begin
     return $1;
 end;
 $$ language plpgsql;
-select raise_test1(5);
 ERROR:  too many parameters specified for RAISE
-CONTEXT:  PL/pgSQL function raise_test1(integer) line 3 at RAISE
+CONTEXT:  compilation of PL/pgSQL function "raise_test1" near line 3
 create function raise_test2(int) returns int as $$
 begin
     raise notice 'This message has too few parameters: %, %, %', $1, $1;
     return $1;
 end;
 $$ language plpgsql;
-select raise_test2(10);
 ERROR:  too few parameters specified for RAISE
-CONTEXT:  PL/pgSQL function raise_test2(integer) line 3 at RAISE
+CONTEXT:  compilation of PL/pgSQL function "raise_test2" near line 3
+create function raise_test3(int) returns int as $$
+begin
+    raise notice 'This message has no parameters (despite having %% signs in it)!';
+    return $1;
+end;
+$$ language plpgsql;
+select raise_test3(1);
+NOTICE:  This message has no parameters (despite having % signs in it)!
+ raise_test3 
+-------------
+           1
+(1 row)
+
 -- Test re-RAISE inside a nested exception block.  This case is allowed
 -- by Oracle's PL/SQL but was handled differently by PG before 9.1.
 CREATE FUNCTION reraise_test() RETURNS void AS $$

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index c5616ee8fc638b68fa6bc60a4d9fcc6ac7cec11e..2abcbc8d3cd9b2dce3aba2251584e25d10f8c8f4 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2078,8 +2078,6 @@ begin
 end;
 $$ language plpgsql;
 
-select raise_test1(5);
-
 create function raise_test2(int) returns int as $$
 begin
     raise notice 'This message has too few parameters: %, %, %', $1, $1;
@@ -2087,7 +2085,14 @@ begin
 end;
 $$ language plpgsql;
 
-select raise_test2(10);
+create function raise_test3(int) returns int as $$
+begin
+    raise notice 'This message has no parameters (despite having %% signs in it)!';
+    return $1;
+end;
+$$ language plpgsql;
+
+select raise_test3(1);
 
 -- Test re-RAISE inside a nested exception block.  This case is allowed
 -- by Oracle's PL/SQL but was handled differently by PG before 9.1.