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

tglsfdc · tglsfdc · commit 58fdca2204de · 2025-03-22T14:17:00.000-04:00
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
@@ -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
@@ -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
@@ -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
  */

Original file line number	Diff line number	Diff line change
`@@ -1603,7 +1603,7 @@ exception_matches_conditions(ErrorData edata, PLpgSQL_condition cond)`
`1603`	`1603`	`* assert-failure. If you're foolish enough, you can match those`
`1604`	`1604`	`* explicitly.`
`1605`	`1605`	`*/`
`1606`		`- if (sqlerrstate == 0)`
	`1606`	`+ if (sqlerrstate == PLPGSQL_OTHERS)`
`1607`	`1607`	`{`
`1608`	`1608`	`if (edata->sqlerrcode != ERRCODE_QUERY_CANCELED &&`
`1609`	`1609`	`edata->sqlerrcode != ERRCODE_ASSERT_FAILURE)`