From 95b412ef6cf6c7d35714e2dc1a1d234397bbf3aa Mon Sep 17 00:00:00 2001 From: Amit Langote 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 Author: Jian He Author: Amit Langote 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