Hello Marko,
Here's a patch for making PL/PgSQL throw an error during compilation (instead
of runtime) if the number of parameters passed to RAISE don't match the
number of placeholders in error message. I'm sure people can see the pros of
doing it this way.
Patch scanned, applied & tested without problem on head.
The compile-time raise parameter checking is a good move.
3 minor points:
- I would suggest to avoid "continue" within a loop so that the code is
simpler to understand, at least for me.
- I would suggest to update the documentation accordingly.
- The execution code now contains error detections which should never be
raised, but I suggest to keep it in place anyway. However I would suggest
to add comments about the fact that it should not be triggered.
See the attached patch which implements these suggestions on top of your
patch.
--
Fabien.
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 91fadb0..a3c763a 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3403,6 +3403,8 @@ 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 optional arguments must match the number of <literal>%</>
+ placeholders in the format string, otherwise a compilation error is reported.
</para>
<para>
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 69d1965..a3cd6fc 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2939,6 +2939,10 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
continue;
}
+ /*
+ * Too few arguments to "raise":
+ * This should never happen as a compile-time check is performed.
+ */
if (current_param == NULL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -2965,7 +2969,8 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
/*
* If more parameters were specified than were required to process the
- * format string, throw an error
+ * format string, throw an error.
+ * This should never happen as a compile-time check is performed.
*/
if (current_param != NULL)
ereport(ERROR,
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index ba928e3..03976e4 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -3785,15 +3785,12 @@ check_raise_parameters(PLpgSQL_stmt_raise *stmt)
for (cp = stmt->message; *cp; cp++)
{
- if (cp[0] != '%')
- continue;
- /* literal %, ignore */
- if (cp[1] == '%')
- {
- cp++;
- continue;
+ if (cp[0] == '%') {
+ if (cp[1] == '%') /* literal %, ignore */
+ cp++;
+ else
+ expected_nparams++;
}
- expected_nparams++;
}
if (expected_nparams < list_length(stmt->params))
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
