Don't capture hard errors in with-clause parser

When parsing WITH clause arguments for ALTER TABLE to extract
compression parameters, syntax errors in the text format are caught
using PG_TRY and PG_CATCH and then returning an indication if the parse
succeeded or not.

If an out-of-memory error occurs inside execution of the input function
and this error is caught, it can continue executing and potentially
cause cascading out-of-memory errors and in the end exhausting the error
stack.

This commit solves this by using checking the category of the thrown
error and only allow errors in the data exception and the syntax errors
and access rules violation category, which are "soft" errors in this
case. Hard errors, like out-of-memory errors, are re-thrown allowing
the backend to deal with it properly.

It also adds tests and test utilities that allow you to raise an error
of an arbitrary category when converting a string to the type.

Author: mkindahl

.unreleased/pr_7893

@@ -0,0 +1 @@
+Fixes: #7893 Don't capture hard errors in with-clause parser

src/with_clause_parser.c

@@ -161,12 +161,38 @@ parse_arg(WithClauseDefinition arg, DefElem *def)
 
 	Assert(OidIsValid(in_fn));
 
+	/*
+	 * We could use InputFunctionCallSafe() here but this is just supported
+	 * for PG16 and later, so we opt for checking if the failure is what we
+	 * expected and re-throwing the error otherwise.
+	 */
 	PG_TRY();
 	{
 		val = OidInputFunctionCall(in_fn, value, typIOParam, -1);
 	}
 	PG_CATCH();
 	{
+		const int sqlerrcode = geterrcode();
+		/*
+		 * We can deal with the Data Exception category and in the Syntax
+		 * Error or Access Rule Violation category, but if the error is an
+		 * insufficient resources category, for example, an out of memory
+		 * error, we should just re-throw it.
+		 *
+		 * Errors in other categories are unlikely, but we cannot do anything
+		 * with them anyway, so just re-throw them as well.
+		 */
+		if (ERRCODE_TO_CATEGORY(sqlerrcode) != ERRCODE_DATA_EXCEPTION &&
+			ERRCODE_TO_CATEGORY(sqlerrcode) != ERRCODE_SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION)
+		{
+			PG_RE_THROW();
+		}
+		FlushErrorState();
+
+		/* We are currently using the ErrorContext, but since we are going to
+		 * raise an error later, there is no reason to switch memory context
+		 * nor restore the resource owner here. */
+
 		Form_pg_type typetup;
 		HeapTuple tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(arg.type_id));
 		if (!HeapTupleIsValid(tup))

test/src/test_with_clause_parser.c

@@ -10,16 +10,48 @@
 #include <commands/defrem.h>
 #include <fmgr.h>
 #include <funcapi.h>
+#include <postgres_ext.h>
 #include <utils/array.h>
 #include <utils/builtins.h>
+#include <utils/elog.h>
 #include <utils/lsyscache.h>
 #include <utils/memutils.h>
+#include <utils/regproc.h>
 
 #include "annotations.h"
 #include "export.h"
 #include "test_utils.h"
 #include "with_clause_parser.h"
 
+TS_FUNCTION_INFO_V1(ts_sqlstate_raise_in);
+TS_FUNCTION_INFO_V1(ts_sqlstate_raise_out);
+
+/*
+ * Input function that will raise the error code give. This means that you can
+ * trigger an error when reading and converting a string to this type.
+ */
+Datum
+ts_sqlstate_raise_in(PG_FUNCTION_ARGS)
+{
+	char *code = PG_GETARG_CSTRING(0);
+	if (strlen(code) != 5)
+		ereport(ERROR,
+				errcode(ERRCODE_SYNTAX_ERROR),
+				errmsg("error code \"%s\" was not of length 5", code));
+	int sqlstate = MAKE_SQLSTATE(code[0], code[1], code[2], code[3], code[4]);
+	ereport(ERROR, errcode(sqlstate), errmsg("Raised requested error code \"%s\"", code));
+	return 0;
+}
+
+/*
+ * Dummy function, we do not store values of this type anywhere.
+ */
+Datum
+ts_sqlstate_raise_out(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_CSTRING("uninteresting");
+}
+
 static DefElem *
 def_elem_from_texts(Datum *texts, int nelems)
 {
@@ -177,6 +209,7 @@ typedef enum TestArgs
 	TestArgDefault,
 	TestArgName,
 	TestArgRegclass,
+	TestArgRaise,
 } TestArgs;
 
 static WithClauseDefinition test_args[] = {
@@ -188,8 +221,14 @@ static WithClauseDefinition test_args[] = {
 						 .type_id = INT4OID,
 						 .default_val = (Datum)-100 },
 	[TestArgName] = { .arg_names = {"name", NULL}, .type_id = NAMEOID, },
-	[TestArgRegclass] = { .arg_names = {"regclass", NULL},
-						  .type_id = REGCLASSOID },
+	[TestArgRegclass] = {
+		.arg_names = {"regclass", NULL},
+		.type_id = REGCLASSOID,
+	},
+	[TestArgRaise] = {
+		.arg_names = {"sqlstate_raise", NULL},
+		.type_id = InvalidOid,
+	},
 };
 
 typedef struct WithClauseValue
@@ -207,6 +246,32 @@ TS_TEST_FN(ts_test_with_clause_parse)
 	HeapTuple tuple;
 	WithClauseValue *result;
 
+	/*
+	 * Look up any missing type ids before using it below to allow
+	 * user-defined types.
+	 *
+	 * Note that this will not look up types we have found in previous calls
+	 * of this function.
+	 *
+	 * We use the slightly more complicated way of calling to_regtype since
+	 * that exists on all versions of PostgreSQL. We cannot use regtypein
+	 * since that can generate errors and we do not want to deal with that.
+	 */
+	for (unsigned int i = 0; i < TS_ARRAY_LEN(test_args); ++i)
+	{
+		LOCAL_FCINFO(fcinfo_in, 1);
+		Datum result;
+		if (!OidIsValid(test_args[i].type_id))
+		{
+			InitFunctionCallInfoData(*fcinfo_in, NULL, 1, InvalidOid, NULL, NULL);
+			fcinfo_in->args[0].value = CStringGetTextDatum(test_args[i].arg_names[0]);
+			fcinfo_in->args[0].isnull = false;
+			result = to_regtype(fcinfo_in);
+			if (!fcinfo_in->isnull)
+				test_args[i].type_id = DatumGetObjectId(result);
+		}
+	}
+
 	if (SRF_IS_FIRSTCALL())
 	{
 		ArrayType *with_clause_array;