plpgsql: make WHEN OTHERS distinct from WHEN SQLSTATE '00000'.

The catchall exception condition OTHERS was represented as
sqlerrstate == 0, which was a poor choice because that comes
out the same as SQLSTATE '00000'.  While we don't issue that
as an error code ourselves, there isn't anything particularly
stopping users from doing so.  Use -1 instead, which can't
match any allowed SQLSTATE string.

While at it, invent a macro PLPGSQL_OTHERS to use instead of
a hard-coded magic number.

While this seems like a bug fix, I'm inclined not to back-patch.
It seems barely possible that someone has written code like this
and would be annoyed by changing the behavior in a minor release.

Reported-by: David Fiedler <david.fido.fiedler@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAHjN70-=H5EpTOuZVbC8mPvRS5EfZ4MY2=OUdVDWoyGvKhb+Rw@mail.gmail.com

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index f36a244140e6752a6cf46c58c4f093cb5c34ff2d..6fdba95962d5e93f1e874e69b9c08dc3db26f6d1 100644 (file)
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -2273,14 +2273,10 @@ plpgsql_parse_err_condition(char *condname)
     * here.
     */
 
-   /*
-    * OTHERS is represented as code 0 (which would map to '00000', but we
-    * have no need to represent that as an exception condition).
-    */
    if (strcmp(condname, "others") == 0)
    {
        new = palloc(sizeof(PLpgSQL_condition));
-       new->sqlerrstate = 0;
+       new->sqlerrstate = PLPGSQL_OTHERS;
        new->condname = condname;
        new->next = NULL;
        return new;

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 9c41ca082532d578f68c7e3aec04a5e114bbac0c..bb99781c56e3923c8101103da7893a992e64e318 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -1603,7 +1603,7 @@ exception_matches_conditions(ErrorData *edata, PLpgSQL_condition *cond)
         * assert-failure.  If you're foolish enough, you can match those
         * explicitly.
         */
-       if (sqlerrstate == 0)
+       if (sqlerrstate == PLPGSQL_OTHERS)
        {
            if (edata->sqlerrcode != ERRCODE_QUERY_CANCELED &&
                edata->sqlerrcode != ERRCODE_ASSERT_FAILURE)

diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index aea0d0f98b2977ddcfcc37f0aa1469938a6f0289..b67847b5111c5a403b69b0e98b3b08ae041e7074 100644 (file)
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -490,11 +490,14 @@ typedef struct PLpgSQL_stmt
  */
 typedef struct PLpgSQL_condition
 {
-   int         sqlerrstate;    /* SQLSTATE code */
+   int         sqlerrstate;    /* SQLSTATE code, or PLPGSQL_OTHERS */
    char       *condname;       /* condition name (for debugging) */
    struct PLpgSQL_condition *next;
 } PLpgSQL_condition;
 
+/* This value mustn't match any possible output of MAKE_SQLSTATE() */
+#define PLPGSQL_OTHERS (-1)
+
 /*
  * EXCEPTION block
  */