WHEN SQLSTATE '00000' THEN equals to WHEN OTHERS THEN

Started by David Fiedler10 months ago4 messages
#1David Fiedler
david.fido.fiedler@gmail.com

Hi,
I've stumbled across a code that used this condition, resulting in
unexpected behavior. I think it worths a note that catching 00000 is not
possible and that it results in a catch all handler.
What do you think? Should I post the expected text somewhere?

Thanks,
David Fiedler

--
*David Fiedler*
*737472531*
*david.fido.fiedler@gmail.com <david.fido.fiedler@gmail.com>*

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Fiedler (#1)
Re: WHEN SQLSTATE '00000' THEN equals to WHEN OTHERS THEN

David Fiedler <david.fido.fiedler@gmail.com> writes:

I've stumbled across a code that used this condition, resulting in
unexpected behavior. I think it worths a note that catching 00000 is not
possible and that it results in a catch all handler.

Hmph. The code thinks

* OTHERS is represented as code 0 (which would map to '00000', but we
* have no need to represent that as an exception condition).

but it evidently didn't consider the possibility of a user writing
'00000'. I'm more inclined to consider this a bug and change plpgsql
to use something else internally to represent OTHERS. We could use
-1, which AFAICS cannot be generated by MAKE_SQLSTATE.

regards, tom lane

#3Laurenz Albe
laurenz.albe@cybertec.at
In reply to: David Fiedler (#1)
Re: WHEN SQLSTATE '00000' THEN equals to WHEN OTHERS THEN

On Wed, 2025-03-19 at 15:03 +0100, David Fiedler wrote:

I've stumbled across a code that used this condition, resulting in unexpected behavior.
I think it worths a note that catching 00000 is not possible and that it results in a catch all handler.
What do you think? Should I post the expected text somewhere?

The code makes no sense, but what about this:

DO $$BEGIN RAISE EXCEPTION SQLSTATE '00000'; END;$$;
ERROR: 00000
CONTEXT: PL/pgSQL function inline_code_block line 1 at RAISE

Yours,
Laurenz Albe

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
1 attachment(s)
Re: WHEN SQLSTATE '00000' THEN equals to WHEN OTHERS THEN

[ redirecting to -hackers ]

I wrote:

David Fiedler <david.fido.fiedler@gmail.com> writes:

I've stumbled across a code that used this condition, resulting in
unexpected behavior. I think it worths a note that catching 00000 is not
possible and that it results in a catch all handler.

Hmph. The code thinks
* OTHERS is represented as code 0 (which would map to '00000', but we
* have no need to represent that as an exception condition).
but it evidently didn't consider the possibility of a user writing
'00000'. I'm more inclined to consider this a bug and change plpgsql
to use something else internally to represent OTHERS. We could use
-1, which AFAICS cannot be generated by MAKE_SQLSTATE.

Here's a patch for this. I'm unsure whether to change it in back
branches; is it conceivable that somebody is depending on WHEN
SQLSTATE '00000' mapping to WHEN OTHERS?

regards, tom lane

Attachments:

make-OTHERS-disjoint-from-sqlstates.patchtext/x-diff; charset=us-ascii; name=make-OTHERS-disjoint-from-sqlstates.patchDownload
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 d4377ceecbf..aed75bc20eb 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
  */