v4-0001-SQL-JSON-Avoid-initializing-unnecessary-ON-ERROR-.patch

From 95b412ef6cf6c7d35714e2dc1a1d234397bbf3aa Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 6 Sep 2024 12:05:40 +0900
Subject: [PATCH v4] SQL/JSON: Avoid initializing unnecessary ON ERROR / ON
 EMPTY steps

When the ON ERROR / ON EMPTY behavior is to return NULL, returning
NULL directly from ExecEvalJsonExprPath() suffices. Therefore, there's
no need to create separate steps to check the error/empty flag or
those to evaluate the the constant NULL expression.  This speeds up
common cases because the default ON ERROR / ON EMPTY behavior for
JSON_QUERY() and JSON_VALUE() is to return NULL.  However, these steps
are necessary if the RETURNING type is a domain, as constraints on the
domain may need to be checked.

Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
---
 src/backend/executor/execExpr.c       | 32 ++++++++++++++++++++++-----
 src/backend/executor/execExprInterp.c | 14 +++++++-----
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 63289ee35e..c8077aa57b 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -4411,9 +4411,11 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
 	List	   *jumps_return_null = NIL;
 	List	   *jumps_to_end = NIL;
 	ListCell   *lc;
-	ErrorSaveContext *escontext =
-		jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR ?
-		&jsestate->escontext : NULL;
+	ErrorSaveContext *escontext;
+	bool		returning_domain =
+		get_typtype(jsexpr->returning->typid) == TYPTYPE_DOMAIN;
+
+	Assert(jsexpr->on_error != NULL);
 
 	jsestate->jsexpr = jsexpr;
 
@@ -4491,6 +4493,9 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
 	scratch->d.constval.isnull = true;
 	ExprEvalPushStep(state, scratch);
 
+	escontext = jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR ?
+		&jsestate->escontext : NULL;
+
 	/*
 	 * To handle coercion errors softly, use the following ErrorSaveContext to
 	 * pass to ExecInitExprRec() when initializing the coercion expressions
@@ -4562,9 +4567,18 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
 	 * Step to check jsestate->error and return the ON ERROR expression if
 	 * there is one.  This handles both the errors that occur during jsonpath
 	 * evaluation in EEOP_JSONEXPR_PATH and subsequent coercion evaluation.
+	 *
+	 * Speed up common cases by avoiding extra steps for a NULL-valued ON
+	 * ERROR expression unless RETURNING a domain type, where constraints must
+	 * be checked. ExecEvalJsonExprPath() already returns NULL on error,
+	 * making additional steps unnecessary in typical scenarios. Note that the
+	 * default ON ERROR behavior for JSON_VALUE() and JSON_QUERY() is to
+	 * return NULL.
 	 */
-	if (jsexpr->on_error &&
-		jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR)
+	if (jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR &&
+		(!(IsA(jsexpr->on_error->expr, Const) &&
+		   ((Const *) jsexpr->on_error->expr)->constisnull) ||
+		 returning_domain))
 	{
 		ErrorSaveContext *saved_escontext;
 
@@ -4619,9 +4633,15 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
 	/*
 	 * Step to check jsestate->empty and return the ON EMPTY expression if
 	 * there is one.
+	 *
+	 * See the comment above for details on the optimization for NULL-valued
+	 * expressions.
 	 */
 	if (jsexpr->on_empty != NULL &&
-		jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR)
+		jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR &&
+		(!(IsA(jsexpr->on_empty->expr, Const) &&
+		   ((Const *) jsexpr->on_empty->expr)->constisnull) ||
+		 returning_domain))
 	{
 		ErrorSaveContext *saved_escontext;
 
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index a6c47f61e0..9fd988cc99 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4550,8 +4550,8 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
 				/* Set up to catch coercion errors of the ON EMPTY value. */
 				jsestate->escontext.error_occurred = false;
 				jsestate->escontext.details_wanted = true;
-				Assert(jsestate->jump_empty >= 0);
-				return jsestate->jump_empty;
+				/* Jump to end if the ON EMPTY behavior is to return NULL */
+				return jsestate->jump_empty >= 0 ? jsestate->jump_empty : jsestate->jump_end;
 			}
 		}
 		else if (jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR)
@@ -4560,8 +4560,9 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
 			/* Set up to catch coercion errors of the ON ERROR value. */
 			jsestate->escontext.error_occurred = false;
 			jsestate->escontext.details_wanted = true;
-			Assert(!throw_error && jsestate->jump_error >= 0);
-			return jsestate->jump_error;
+			Assert(!throw_error);
+			/* Jump to end if the ON ERROR behavior is to return NULL */
+			return jsestate->jump_error >= 0 ? jsestate->jump_error : jsestate->jump_end;
 		}
 
 		if (jsexpr->column_name)
@@ -4581,14 +4582,15 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
 	 */
 	if (error)
 	{
-		Assert(!throw_error && jsestate->jump_error >= 0);
+		Assert(!throw_error);
 		*op->resvalue = (Datum) 0;
 		*op->resnull = true;
 		jsestate->error.value = BoolGetDatum(true);
 		/* Set up to catch coercion errors of the ON ERROR value. */
 		jsestate->escontext.error_occurred = false;
 		jsestate->escontext.details_wanted = true;
-		return jsestate->jump_error;
+		/* Jump to end if the ON ERROR behavior is to return NULL */
+		return jsestate->jump_error >= 0 ? jsestate->jump_error : jsestate->jump_end;
 	}
 
 	return jump_eval_coercion >= 0 ? jump_eval_coercion : jsestate->jump_end;
-- 
2.43.0