Create infrastructure for "soft" error reporting.

tglsfdc · pull[bot] · commit 55f40fd72df3 · 2024-05-14T00:43:58.000Z
Postgres' standard mechanism for reporting errors (ereport() or elog()) is used for all sorts of error conditions. This means that throwing an exception via ereport(ERROR) requires an expensive transaction or subtransaction abort and cleanup, since the exception catcher dare not make many assumptions about what has gone wrong. There are situations where we would rather have a lighter-weight mechanism for dealing with errors that are known to be safe to recover from without a full transaction cleanup. This commit creates infrastructure to let us adapt existing error-reporting code for that purpose. See the included documentation changes for details. Follow-on commits will provide test code and usage examples. The near-term plan is to convert most if not all datatype input functions to report invalid input "softly". This will enable implementing some SQL/JSON features cleanly and without the cost of subtransactions, and it will also allow creating COPY options to deal with bad input without cancelling the whole COPY. This patch is mostly by me, but it owes very substantial debt to earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul. Thanks also to Andres Freund for review. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml
@@ -900,6 +900,17 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
    function is written in C.
   </para>
 
+  <para>
+   In <productname>PostgreSQL</productname> version 16 and later,
+   it is desirable for base types' input functions to
+   return <quote>soft</quote> errors using the
+   new <function>errsave()</function>/<function>ereturn()</function>
+   mechanism, rather than throwing <function>ereport()</function>
+   exceptions as in previous versions.
+   See <filename>src/backend/utils/fmgr/README</filename> for more
+   information.
+  </para>
+
  </refsect1>
 
  <refsect1>
diff --git a/src/backend/nodes/Makefile b/src/backend/nodes/Makefile
@@ -56,6 +56,7 @@ node_headers = \
 	nodes/bitmapset.h \
 	nodes/extensible.h \
 	nodes/lockoptions.h \
+	nodes/miscnodes.h \
 	nodes/replnodes.h \
 	nodes/supportnodes.h \
 	nodes/value.h \
diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
@@ -68,6 +68,7 @@ sub elem
   nodes/bitmapset.h
   nodes/extensible.h
   nodes/lockoptions.h
+  nodes/miscnodes.h
   nodes/replnodes.h
   nodes/supportnodes.h
   nodes/value.h
@@ -89,6 +90,7 @@ sub elem
   executor/tuptable.h
   foreign/fdwapi.h
   nodes/lockoptions.h
+  nodes/miscnodes.h
   nodes/replnodes.h
   nodes/supportnodes.h
 );
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
@@ -71,6 +71,7 @@
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
 #include "mb/pg_wchar.h"
+#include "nodes/miscnodes.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "postmaster/bgworker.h"
@@ -611,6 +612,128 @@ errfinish(const char *filename, int lineno, const char *funcname)
 	CHECK_FOR_INTERRUPTS();
 }
 
+
+/*
+ * errsave_start --- begin a "soft" error-reporting cycle
+ *
+ * If "context" isn't an ErrorSaveContext node, this behaves as
+ * errstart(ERROR, domain), and the errsave() macro ends up acting
+ * exactly like ereport(ERROR, ...).
+ *
+ * If "context" is an ErrorSaveContext node, but the node creator only wants
+ * notification of the fact of a soft error without any details, we just set
+ * the error_occurred flag in the ErrorSaveContext node and return false,
+ * which will cause us to skip the remaining error processing steps.
+ *
+ * Otherwise, create and initialize error stack entry and return true.
+ * Subsequently, errmsg() and perhaps other routines will be called to further
+ * populate the stack entry.  Finally, errsave_finish() will be called to
+ * tidy up.
+ */
+bool
+errsave_start(struct Node *context, const char *domain)
+{
+	ErrorSaveContext *escontext;
+	ErrorData  *edata;
+
+	/*
+	 * Do we have a context for soft error reporting?  If not, just punt to
+	 * errstart().
+	 */
+	if (context == NULL || !IsA(context, ErrorSaveContext))
+		return errstart(ERROR, domain);
+
+	/* Report that a soft error was detected */
+	escontext = (ErrorSaveContext *) context;
+	escontext->error_occurred = true;
+
+	/* Nothing else to do if caller wants no further details */
+	if (!escontext->details_wanted)
+		return false;
+
+	/*
+	 * Okay, crank up a stack entry to store the info in.
+	 */
+
+	recursion_depth++;
+
+	/* Initialize data for this error frame */
+	edata = get_error_stack_entry();
+	edata->elevel = LOG;		/* signal all is well to errsave_finish */
+	set_stack_entry_domain(edata, domain);
+	/* Select default errcode based on the assumed elevel of ERROR */
+	edata->sqlerrcode = ERRCODE_INTERNAL_ERROR;
+
+	/*
+	 * Any allocations for this error state level should go into the caller's
+	 * context.  We don't need to pollute ErrorContext, or even require it to
+	 * exist, in this code path.
+	 */
+	edata->assoc_context = CurrentMemoryContext;
+
+	recursion_depth--;
+	return true;
+}
+
+/*
+ * errsave_finish --- end a "soft" error-reporting cycle
+ *
+ * If errsave_start() decided this was a regular error, behave as
+ * errfinish().  Otherwise, package up the error details and save
+ * them in the ErrorSaveContext node.
+ */
+void
+errsave_finish(struct Node *context, const char *filename, int lineno,
+			   const char *funcname)
+{
+	ErrorSaveContext *escontext = (ErrorSaveContext *) context;
+	ErrorData  *edata = &errordata[errordata_stack_depth];
+
+	/* verify stack depth before accessing *edata */
+	CHECK_STACK_DEPTH();
+
+	/*
+	 * If errsave_start punted to errstart, then elevel will be ERROR or
+	 * perhaps even PANIC.  Punt likewise to errfinish.
+	 */
+	if (edata->elevel >= ERROR)
+	{
+		errfinish(filename, lineno, funcname);
+		pg_unreachable();
+	}
+
+	/*
+	 * Else, we should package up the stack entry contents and deliver them to
+	 * the caller.
+	 */
+	recursion_depth++;
+
+	/* Save the last few bits of error state into the stack entry */
+	set_stack_entry_location(edata, filename, lineno, funcname);
+
+	/* Replace the LOG value that errsave_start inserted */
+	edata->elevel = ERROR;
+
+	/*
+	 * We skip calling backtrace and context functions, which are more likely
+	 * to cause trouble than provide useful context; they might act on the
+	 * assumption that a transaction abort is about to occur.
+	 */
+
+	/*
+	 * Make a copy of the error info for the caller.  All the subsidiary
+	 * strings are already in the caller's context, so it's sufficient to
+	 * flat-copy the stack entry.
+	 */
+	escontext->error_data = palloc_object(ErrorData);
+	memcpy(escontext->error_data, edata, sizeof(ErrorData));
+
+	/* Exit error-handling context */
+	errordata_stack_depth--;
+	recursion_depth--;
+}
+
+
 /*
  * get_error_stack_entry --- allocate and initialize a new stack entry
  *
diff --git a/src/backend/utils/fmgr/README b/src/backend/utils/fmgr/README
@@ -267,6 +267,78 @@ See windowapi.h for more information.
 information about the context of the CALL statement, particularly
 whether it is within an "atomic" execution context.
 
+* Some callers of datatype input functions (and in future perhaps
+other classes of functions) pass an instance of ErrorSaveContext.
+This indicates that the caller wishes to handle "soft" errors without
+a transaction-terminating exception being thrown: instead, the callee
+should store information about the error cause in the ErrorSaveContext
+struct and return a dummy result value.  Further details appear in
+"Handling Soft Errors" below.
+
+
+Handling Soft Errors
+--------------------
+
+Postgres' standard mechanism for reporting errors (ereport() or elog())
+is used for all sorts of error conditions.  This means that throwing
+an exception via ereport(ERROR) requires an expensive transaction or
+subtransaction abort and cleanup, since the exception catcher dare not
+make many assumptions about what has gone wrong.  There are situations
+where we would rather have a lighter-weight mechanism for dealing
+with errors that are known to be safe to recover from without a full
+transaction cleanup.  SQL-callable functions can support this need
+using the ErrorSaveContext context mechanism.
+
+To report a "soft" error, a SQL-callable function should call
+	errsave(fcinfo->context, ...)
+where it would previously have done
+	ereport(ERROR, ...)
+If the passed "context" is NULL or is not an ErrorSaveContext node,
+then errsave behaves precisely as ereport(ERROR): the exception is
+thrown via longjmp, so that control does not return.  If "context"
+is an ErrorSaveContext node, then the error information included in
+errsave's subsidiary reporting calls is stored into the context node
+and control returns from errsave normally.  The function should then
+return a dummy value to its caller.  (SQL NULL is recommendable as
+the dummy value; but anything will do, since the caller is expected
+to ignore the function's return value once it sees that an error has
+been reported in the ErrorSaveContext node.)
+
+If there is nothing to do except return after calling errsave(),
+you can save a line or two by writing
+	ereturn(fcinfo->context, dummy_value, ...)
+to perform errsave() and then "return dummy_value".
+
+An error reported "softly" must be safe, in the sense that there is
+no question about our ability to continue normal processing of the
+transaction.  Error conditions that should NOT be handled this way
+include out-of-memory, unexpected internal errors, or anything that
+cannot easily be cleaned up after.  Such cases should still be thrown
+with ereport, as they have been in the past.
+
+Considering datatype input functions as examples, typical "soft" error
+conditions include input syntax errors and out-of-range values.  An
+input function typically detects such cases with simple if-tests and
+can easily change the ensuing ereport call to an errsave or ereturn.
+Because of this restriction, it's typically not necessary to pass
+the ErrorSaveContext pointer down very far, as errors reported by
+low-level functions are typically reasonable to consider internal.
+(Another way to frame the distinction is that input functions should
+report all invalid-input conditions softly, but internal problems are
+hard errors.)
+
+Because no transaction cleanup will occur, a function that is exiting
+after errsave() returns will bear responsibility for resource cleanup.
+It is not necessary to be concerned about small leakages of palloc'd
+memory, since the caller should be running the function in a short-lived
+memory context.  However, resources such as locks, open files, or buffer
+pins must be closed out cleanly, as they would be in the non-error code
+path.
+
+Conventions for callers that use the ErrorSaveContext mechanism
+to trap errors are discussed with the declaration of that struct,
+in nodes/miscnodes.h.
+
 
 Functions Accepting or Returning Sets
 -------------------------------------
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
@@ -23,6 +23,7 @@
 #include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
+#include "nodes/miscnodes.h"
 #include "nodes/nodeFuncs.h"
 #include "pgstat.h"
 #include "utils/acl.h"
@@ -1549,6 +1550,70 @@ InputFunctionCall(FmgrInfo *flinfo, char *str, Oid typioparam, int32 typmod)
 	return result;
 }
 
+/*
+ * Call a previously-looked-up datatype input function, with non-exception
+ * handling of "soft" errors.
+ *
+ * This is basically like InputFunctionCall, but the converted Datum is
+ * returned into *result while the function result is true for success or
+ * false for failure.  Also, the caller may pass an ErrorSaveContext node.
+ * (We declare that as "fmNodePtr" to avoid including nodes.h in fmgr.h.)
+ *
+ * If escontext points to an ErrorSaveContext, any "soft" errors detected by
+ * the input function will be reported by filling the escontext struct and
+ * returning false.  (The caller can choose to test SOFT_ERROR_OCCURRED(),
+ * but checking the function result instead is usually cheaper.)
+ *
+ * If escontext does not point to an ErrorSaveContext, errors are reported
+ * via ereport(ERROR), so that there is no functional difference from
+ * InputFunctionCall; the result will always be true if control returns.
+ */
+bool
+InputFunctionCallSafe(FmgrInfo *flinfo, char *str,
+					  Oid typioparam, int32 typmod,
+					  fmNodePtr escontext,
+					  Datum *result)
+{
+	LOCAL_FCINFO(fcinfo, 3);
+
+	if (str == NULL && flinfo->fn_strict)
+	{
+		*result = (Datum) 0;	/* just return null result */
+		return true;
+	}
+
+	InitFunctionCallInfoData(*fcinfo, flinfo, 3, InvalidOid, escontext, NULL);
+
+	fcinfo->args[0].value = CStringGetDatum(str);
+	fcinfo->args[0].isnull = false;
+	fcinfo->args[1].value = ObjectIdGetDatum(typioparam);
+	fcinfo->args[1].isnull = false;
+	fcinfo->args[2].value = Int32GetDatum(typmod);
+	fcinfo->args[2].isnull = false;
+
+	*result = FunctionCallInvoke(fcinfo);
+
+	/* Result value is garbage, and could be null, if an error was reported */
+	if (SOFT_ERROR_OCCURRED(escontext))
+		return false;
+
+	/* Otherwise, should get null result if and only if str is NULL */
+	if (str == NULL)
+	{
+		if (!fcinfo->isnull)
+			elog(ERROR, "input function %u returned non-NULL",
+				 flinfo->fn_oid);
+	}
+	else
+	{
+		if (fcinfo->isnull)
+			elog(ERROR, "input function %u returned NULL",
+				 flinfo->fn_oid);
+	}
+
+	return true;
+}
+
 /*
  * Call a previously-looked-up datatype output function.
  *
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
@@ -700,6 +700,10 @@ extern Datum OidFunctionCall9Coll(Oid functionId, Oid collation,
 /* Special cases for convenient invocation of datatype I/O functions. */
 extern Datum InputFunctionCall(FmgrInfo *flinfo, char *str,
 							   Oid typioparam, int32 typmod);
+extern bool InputFunctionCallSafe(FmgrInfo *flinfo, char *str,
+								  Oid typioparam, int32 typmod,
+								  fmNodePtr escontext,
+								  Datum *result);
 extern Datum OidInputFunctionCall(Oid functionId, char *str,
 								  Oid typioparam, int32 typmod);
 extern char *OutputFunctionCall(FmgrInfo *flinfo, Datum val);
diff --git a/src/include/nodes/meson.build b/src/include/nodes/meson.build
@@ -16,6 +16,7 @@ node_support_input_i = [
   'nodes/bitmapset.h',
   'nodes/extensible.h',
   'nodes/lockoptions.h',
+  'nodes/miscnodes.h',
   'nodes/replnodes.h',
   'nodes/supportnodes.h',
   'nodes/value.h',
diff --git a/src/include/nodes/miscnodes.h b/src/include/nodes/miscnodes.h
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
