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

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 9b52bab52fcb0fc03078f07f7cd589be42957365..a5395536a1399548c2e7b8d9bdf7b78b55d8024c 100644 (file)
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -4233,9 +4233,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;
 
@@ -4313,6 +4315,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
@@ -4384,9 +4389,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;
 
@@ -4441,9 +4455,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 c32ac7c5098f16f3830d47448e7ca1743ae21c94..aa68c115ba9a6478f0260ba8c8d6283aa15f1145 100644 (file)
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4414,8 +4414,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)
@@ -4424,8 +4424,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)
@@ -4445,14 +4446,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;