From 58fdca2204de5f683f025df37553e5e69cb6adb1 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 22 Mar 2025 14:17:00 -0400 Subject: [PATCH] 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 Author: Tom Lane Discussion: https://postgr.es/m/CAHjN70-=H5EpTOuZVbC8mPvRS5EfZ4MY2=OUdVDWoyGvKhb+Rw@mail.gmail.com --- src/pl/plpgsql/src/pl_comp.c | 6 +----- src/pl/plpgsql/src/pl_exec.c | 2 +- src/pl/plpgsql/src/plpgsql.h | 5 ++++- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index f36a244140e..6fdba95962d 100644 --- 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 9c41ca08253..bb99781c56e 100644 --- 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 aea0d0f98b2..b67847b5111 100644 --- 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 */