On 2022-07-15 Fr 17:07, Andres Freund wrote:
> Hi,
>
> On 2022-07-08 17:05:49 -0400, Andrew Dunstan wrote:
>> On 2022-07-05 Tu 15:04, Andrew Dunstan wrote:
>>> On 2022-07-05 Tu 14:36, Andres Freund wrote:
>>>>>> I think Andrew's beta 2 comment was more about my other architectural
>>>>>> complains around the json expression eval stuff.
>>>>> Right. That's being worked on but it's not going to be a mechanical fix.
>>>> Any updates here?
>>> Not yet. A colleague and I are working on it. I'll post a status this
>>> week if we can't post a fix.
>> We're still working on it. We've made substantial progress but there are
>> some tests failing that we need to fix.
> I think we need to resolve this soon - or consider the alternatives. A lot of
> the new json stuff doesn't seem fully baked, so I'm starting to wonder if we
> have to consider pushing it a release further down.
>
> Perhaps you could post your current state? I might be able to help resolving
> some of the problems.
Ok. Here is the state of things. This has proved to be rather more
intractable than I expected. Almost all the legwork here has been done
by Amit Langote, for which he deserves both my thanks and considerable
credit, but I take responsibility for it.
I just discovered today that this scheme is failing under
"force_parallel_mode = regress". I have as yet no idea if that can be
fixed simply or not. Apart from that I think the main outstanding issue
is to fill in the gaps in llvm_compile_expr().
If you have help you can offer that would be very welcome.
I'd still very much like to get this done, but if the decision is we've
run out of time I'll be sad but understand.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
From 7c0d831f3baf6fb6ec27d1e033209535945e4858 Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <and...@dunslane.net>
Date: Mon, 18 Jul 2022 11:03:13 -0400
Subject: [PATCH 1/3] in JsonExprState just store a pointer to the input
FmgrInfo
---
src/backend/executor/execExpr.c | 5 ++++-
src/backend/executor/execExprInterp.c | 2 +-
src/include/executor/execExpr.h | 2 +-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index c8d7145fe3..a55e5000e2 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2606,11 +2606,14 @@ ExecInitExprRec(Expr *node, ExprState *state,
(jexpr->result_coercion && jexpr->result_coercion->via_io))
{
Oid typinput;
+ FmgrInfo *finfo;
/* lookup the result type's input function */
getTypeInputInfo(jexpr->returning->typid, &typinput,
&jsestate->input.typioparam);
- fmgr_info(typinput, &jsestate->input.func);
+ finfo = palloc0(sizeof(FmgrInfo));
+ fmgr_info(typinput, finfo);
+ jsestate->input.finfo = finfo;
}
jsestate->args = NIL;
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 723770fda0..0512a81c7c 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4664,7 +4664,7 @@ ExecEvalJsonExprCoercion(ExprEvalStep *op, ExprContext *econtext,
/* strip quotes and call typinput function */
char *str = *isNull ? NULL : JsonbUnquote(jb);
- return InputFunctionCall(&jsestate->input.func, str,
+ return InputFunctionCall(jsestate->input.finfo, str,
jsestate->input.typioparam,
jexpr->returning->typmod);
}
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 1e3f1bbee8..e55a572854 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -754,7 +754,7 @@ typedef struct JsonExprState
struct
{
- FmgrInfo func; /* typinput function for output type */
+ FmgrInfo *finfo; /* typinput function for output type */
Oid typioparam;
} input; /* I/O info for output type */
--
2.34.1
From ee39dadaa18f45c59224d4da908b36db7a2a8b0f Mon Sep 17 00:00:00 2001
From: amitlan <amitlangot...@gmail.com>
Date: Mon, 18 Jul 2022 11:04:17 -0400
Subject: [PATCH 2/3] Evaluate various JsonExpr sub-expressions using parent
ExprState
These include PASSING args, ON ERROR, ON EMPTY default expressions,
and result_coercion expression.
To do so, this moves a bunch of code from ExecEvalJson(),
ExecEvalJsonExpr(), and ExecEvalJsonExprCoercion(), all of which
would previously run under the single step EEOP_JSONEXPR steps into
a number of new (sub-) ExprEvalSteps that are now added for a given
JsonExpr.
TODO: update llvm_compile_expr() to handle newly added
EEOP_JSONEXPR_* steps.
---
src/backend/executor/execExpr.c | 288 +++++++++++++++-----
src/backend/executor/execExprInterp.c | 366 ++++++++++++++++++--------
src/backend/jit/llvm/llvmjit_expr.c | 35 ++-
src/include/executor/execExpr.h | 136 ++++++++--
4 files changed, 639 insertions(+), 186 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index a55e5000e2..4dbb24aaee 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2562,62 +2562,47 @@ ExecInitExprRec(Expr *node, ExprState *state,
{
JsonExpr *jexpr = castNode(JsonExpr, node);
JsonExprState *jsestate = palloc0(sizeof(JsonExprState));
+ JsonExprPreEvalState *pre_eval = &jsestate->pre_eval;
+ JsonExprPostEvalState *post_eval = &jsestate->post_eval;
ListCell *argexprlc;
ListCell *argnamelc;
-
- scratch.opcode = EEOP_JSONEXPR;
- scratch.d.jsonexpr.jsestate = jsestate;
+ int skip_step_off;
+ int onempty_step_off;
+ int onerror_step_off;
+ int coercion_step_off;
+ int coercion_error_step_off;
+ int result_step_off;
+ ExprEvalStep *as;
+ bool throwErrors =
+ (jexpr->on_error->btype == JSON_BEHAVIOR_ERROR);
jsestate->jsexpr = jexpr;
- jsestate->formatted_expr =
- palloc(sizeof(*jsestate->formatted_expr));
-
+ /*
+ * Add steps to compute formatted_expr, pathspec, and
+ * PASSING arg expressions as things that must be
+ * evaluated before the "main" JSON path evaluation.
+ */
ExecInitExprRec((Expr *) jexpr->formatted_expr, state,
- &jsestate->formatted_expr->value,
- &jsestate->formatted_expr->isnull);
-
- jsestate->pathspec =
- palloc(sizeof(*jsestate->pathspec));
-
+ &pre_eval->formatted_expr.value,
+ &pre_eval->formatted_expr.isnull);
ExecInitExprRec((Expr *) jexpr->path_spec, state,
- &jsestate->pathspec->value,
- &jsestate->pathspec->isnull);
-
- jsestate->res_expr =
- palloc(sizeof(*jsestate->res_expr));
-
- jsestate->result_expr = jexpr->result_coercion
- ? ExecInitExprWithCaseValue((Expr *) jexpr->result_coercion->expr,
- state->parent,
- &jsestate->res_expr->value,
- &jsestate->res_expr->isnull)
- : NULL;
-
- jsestate->default_on_empty = !jexpr->on_empty ? NULL :
- ExecInitExpr((Expr *) jexpr->on_empty->default_expr,
- state->parent);
-
- jsestate->default_on_error =
- ExecInitExpr((Expr *) jexpr->on_error->default_expr,
- state->parent);
-
- if (jexpr->omit_quotes ||
- (jexpr->result_coercion && jexpr->result_coercion->via_io))
- {
- Oid typinput;
- FmgrInfo *finfo;
-
- /* lookup the result type's input function */
- getTypeInputInfo(jexpr->returning->typid, &typinput,
- &jsestate->input.typioparam);
- finfo = palloc0(sizeof(FmgrInfo));
- fmgr_info(typinput, finfo);
- jsestate->input.finfo = finfo;
- }
-
- jsestate->args = NIL;
-
+ &pre_eval->pathspec.value,
+ &pre_eval->pathspec.isnull);
+
+ /*
+ * However, before pushing steps for PASSING args, push a step
+ * that will decide whether to skip evaluating the args and
+ * JSON path evaluation depending on whether either of
+ * formatted_expr and pathspec is NULL.
+ */
+ scratch.opcode = EEOP_JSONEXPR_SKIP;
+ scratch.d.jsonexpr_skip.pre_eval = pre_eval;
+ scratch.d.jsonexpr_skip.post_eval = post_eval;
+ skip_step_off = state->steps_len;
+ ExprEvalPushStep(state, &scratch);
+
+ jsestate->pre_eval.args = NIL;
forboth(argexprlc, jexpr->passing_values,
argnamelc, jexpr->passing_names)
{
@@ -2628,19 +2613,202 @@ ExecInitExprRec(Expr *node, ExprState *state,
var->name = pstrdup(argname->sval);
var->typid = exprType((Node *) argexpr);
var->typmod = exprTypmod((Node *) argexpr);
- var->estate = ExecInitExpr(argexpr, state->parent);
+
+ /*
+ * Not necessary when being evaluated for a JsonExpr, like
+ * in this case.
+ */
+ var->estate = NULL;
var->econtext = NULL;
var->mcxt = NULL;
- var->evaluated = false;
- var->value = (Datum) 0;
- var->isnull = true;
- jsestate->args =
- lappend(jsestate->args, var);
+ /*
+ * Mark these as always evaluated because they must have
+ * been evaluated before JSON path evaluation begins,
+ * because we haven't pushed the step for the latter yet.
+ */
+ var->evaluated = true;
+
+ ExecInitExprRec((Expr *) argexpr, state,
+ &var->value,
+ &var->isnull);
+
+ pre_eval->args = lappend(pre_eval->args, var);
+ }
+
+ /* Now push the step for the actual JSON path evaluation. */
+ scratch.opcode = EEOP_JSONEXPR_PATH;
+ scratch.d.jsonexpr.jsestate = jsestate;
+ ExprEvalPushStep(state, &scratch);
+
+ /*
+ * Now push steps to control the expressions evaluated based
+ * on the result of JSON path evaluation.
+ */
+
+ /*
+ * Step to handle ON EMPTY behavior correctly. Might be
+ * skipped over (along with default expression steps added
+ * below) if 1) the JSON path evaluation result is not empty,
+ * 2) ON ERROR behavior must be applied instead. Jump target
+ * addressesnecessary to do so will be set later.
+ */
+ onempty_step_off = -1;
+ if (jexpr->on_empty)
+ {
+ scratch.opcode = EEOP_JSONEXPR_ONEMPTY;
+ scratch.d.jsonexpr_onempty.post_eval = post_eval;
+ scratch.d.jsonexpr_onempty.jexpr = jexpr;
+ onempty_step_off = state->steps_len;
+ ExprEvalPushStep(state, &scratch);
+
+ /* Push step(s) for the default ON EMPTY expression. */
+ if (jexpr->on_empty->default_expr)
+ ExecInitExprRec((Expr *) jexpr->on_empty->default_expr,
+ state, resv, resnull);
+ }
+
+ /*
+ * Step to handle ON ERROR behavior correctly. Might be
+ * skipped over (along with default expression steps added
+ * below) if no error occurs. Jump target address necessary
+ * to do so will be set later.
+ */
+ Assert(jexpr->on_error != NULL);
+ scratch.opcode = EEOP_JSONEXPR_ONERROR;
+ scratch.d.jsonexpr_onerror.post_eval = post_eval;
+ scratch.d.jsonexpr_onerror.jexpr = jexpr;
+ onerror_step_off = state->steps_len;
+ ExprEvalPushStep(state, &scratch);
+
+ /* Push step(s) for the default ON ERROR expression. */
+ if (jexpr->on_error->default_expr)
+ ExecInitExprRec((Expr *) jexpr->on_error->default_expr,
+ state, resv, resnull);
+
+ /*
+ * Step to handle applying coercion correctly. Might be
+ * skipped if no coercion needs to be applied separately
+ * from JSON path evaluation itself. Jump target address
+ * necessary to do so will be set later.
+ */
+ scratch.opcode = EEOP_JSONEXPR_COERCION;
+ scratch.d.jsonexpr_coercion.jsestate = jsestate;
+ scratch.d.jsonexpr_coercion.jump_skip_coercion = state->steps_len + 1;
+ coercion_step_off = state->steps_len;
+ ExprEvalPushStep(state, &scratch);
+
+ /*
+ * Step to apply ON ERROR behavior if an error occurs when
+ * applying coercion. Will be skipped over (along with default
+ * expression steps added) if no error occurs. Jump target
+ * address necessary to do so will be set later.
+ */
+ scratch.opcode = EEOP_JSONEXPR_COERCION_ERROR;
+ scratch.d.jsonexpr_coercion_error.post_eval = post_eval;
+ scratch.d.jsonexpr_coercion_error.jexpr = jexpr;
+ coercion_error_step_off = state->steps_len;
+ ExprEvalPushStep(state, &scratch);
+
+ /* Push steps to evaluate the default ON ERROR expression. */
+ if (jexpr->on_error->default_expr)
+ ExecInitExprRec((Expr *) jexpr->on_error->default_expr,
+ state, resv, resnull);
+
+ /*
+ * Step to handle applying result_coercion if needed. Might
+ * be skipped if computing result_coercion is rendered
+ * unnecessary by earlier steps. Jump target address to do
+ * so will be set later.
+ */
+ result_step_off = state->steps_len;
+ if (jexpr->result_coercion && jexpr->result_coercion->expr)
+ {
+ Datum *save_innermost_caseval;
+ bool *save_innermost_isnull;
+
+ scratch.opcode = EEOP_JSONEXPR_RESULT;
+ scratch.d.jsonexpr_result.post_eval = post_eval;
+ scratch.d.jsonexpr_result.jexpr = jexpr;
+ ExprEvalPushStep(state, &scratch);
+
+ /* Steps to compute result_coercion expression. */
+ save_innermost_caseval = state->innermost_caseval;
+ save_innermost_isnull = state->innermost_casenull;
+
+ state->innermost_caseval = resv;
+ state->innermost_casenull = resnull;
+
+ ExecInitExprRec((Expr *) jexpr->result_coercion->expr,
+ state, resv, resnull);
+
+ state->innermost_caseval = save_innermost_caseval;
+ state->innermost_casenull = save_innermost_isnull;
+ }
+
+ /*
+ * Adjust jump target addresses in various post-eval steps now
+ * that we have all the steps in place.
+ */
+
+ /* EEOP_JSONEXPR_SKIP */
+ as = &state->steps[skip_step_off];
+ as->d.jsonexpr_skip.jump_coercion = coercion_step_off;
+
+ /* EEOP_JSONEXPR_ONEMPTY */
+ if (onempty_step_off >= 0)
+ {
+ as = &state->steps[onempty_step_off];
+ as->d.jsonexpr_onempty.jump_error = onerror_step_off;
+ as->d.jsonexpr_onempty.jump_coercion = coercion_step_off;
}
- jsestate->cache = NULL;
+ /* EEOP_JSONEXPR_ONERROR */
+ as = &state->steps[onerror_step_off];
+ as->d.jsonexpr_onerror.jump_coercion = coercion_step_off;
+
+ /* EEOP_JSONEXPR_COERCION */
+ as = &state->steps[coercion_step_off];
+ as->d.jsonexpr_coercion.jump_coercion_error = coercion_error_step_off;
+
+ /* EEOP_JSONEXPR_COERCION_ERROR */
+ as = &state->steps[coercion_error_step_off];
+ as->d.jsonexpr_coercion_error.jump_coercion = coercion_step_off;
+ as->d.jsonexpr_coercion_error.jump_skip_coercion_error = result_step_off;
+
+ /* EEOP_JSONEXPR_RESULT */
+ as = &state->steps[result_step_off];
+ as->d.jsonexpr_result.jump_skip_result = state->steps_len;
+
+ /*
+ * Set if we must use a sub-transaction around path evaluation
+ * that can be aborted on error instead of aborting the parent
+ * query.
+ */
+ jsestate->needSubtrans =
+ ExecEvalJsonNeedsSubTransaction(jexpr, &jsestate->coercions);
+ jsestate->coercionNeedSubtrans =
+ (!jsestate->needSubtrans && !throwErrors);
+
+ /*
+ * Set RETURNING type's input function used by
+ * ExecEvalJsonExprCoercion().
+ */
+ if (jexpr->omit_quotes ||
+ (jexpr->result_coercion && jexpr->result_coercion->via_io))
+ {
+ Oid typinput;
+ FmgrInfo *finfo;
+
+ /* lookup the result type's input function */
+ getTypeInputInfo(jexpr->returning->typid, &typinput,
+ &jsestate->input.typioparam);
+ finfo = palloc0(sizeof(FmgrInfo));
+ fmgr_info(typinput, finfo);
+ jsestate->input.finfo = finfo;
+ }
+ /* Set up coercion related data structures. */
if (jexpr->coercions)
{
JsonCoercion **coercion;
@@ -2648,11 +2816,8 @@ ExecInitExprRec(Expr *node, ExprState *state,
Datum *caseval;
bool *casenull;
- jsestate->coercion_expr =
- palloc(sizeof(*jsestate->coercion_expr));
-
- caseval = &jsestate->coercion_expr->value;
- casenull = &jsestate->coercion_expr->isnull;
+ caseval = resv;
+ casenull = resnull;
for (cstate = &jsestate->coercions.null,
coercion = &jexpr->coercions->null;
@@ -2667,7 +2832,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
}
}
- ExprEvalPushStep(state, &scratch);
break;
}
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 0512a81c7c..7a533c16df 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -158,6 +158,15 @@ static TupleDesc get_cached_rowtype(Oid type_id, int32 typmod,
bool *changed);
static void ExecEvalRowNullInt(ExprState *state, ExprEvalStep *op,
ExprContext *econtext, bool checkisnull);
+static Datum ExecEvalJsonBehavior(JsonBehavior *behavior, bool *is_null);
+typedef Datum (*JsonFunc) (ExprEvalStep *op, ExprContext *econtext,
+ Datum item, bool *resnull, void *p, bool *error);
+static Datum ExecEvalJsonExprSubtrans(JsonFunc func, ExprEvalStep *op,
+ ExprContext *econtext,
+ Datum res, bool *resnull,
+ void *p, bool *error, bool subtrans);
+static Datum ExecEvalJsonExprCoercion(ExprEvalStep *op, ExprContext *econtext,
+ Datum res, bool *isNull, void *p, bool *error);
/* fast-path evaluation functions */
static Datum ExecJustInnerVar(ExprState *state, ExprContext *econtext, bool *isnull);
@@ -490,7 +499,13 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
&&CASE_EEOP_SUBPLAN,
&&CASE_EEOP_JSON_CONSTRUCTOR,
&&CASE_EEOP_IS_JSON,
- &&CASE_EEOP_JSONEXPR,
+ &&CASE_EEOP_JSONEXPR_SKIP,
+ &&CASE_EEOP_JSONEXPR_PATH,
+ &&CASE_EEOP_JSONEXPR_ONERROR,
+ &&CASE_EEOP_JSONEXPR_ONEMPTY,
+ &&CASE_EEOP_JSONEXPR_COERCION,
+ &&CASE_EEOP_JSONEXPR_COERCION_ERROR,
+ &&CASE_EEOP_JSONEXPR_RESULT,
&&CASE_EEOP_AGG_STRICT_DESERIALIZE,
&&CASE_EEOP_AGG_DESERIALIZE,
&&CASE_EEOP_AGG_STRICT_INPUT_CHECK_ARGS,
@@ -1817,13 +1832,178 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_NEXT();
}
- EEO_CASE(EEOP_JSONEXPR)
+ EEO_CASE(EEOP_JSONEXPR_SKIP)
+ {
+ JsonExprPreEvalState *pre_eval = op->d.jsonexpr_skip.pre_eval;
+ JsonExprPostEvalState *post_eval = op->d.jsonexpr_skip.post_eval;
+
+ /*
+ * Skip JSON evaluation if either of the input expressions has
+ * turned out to be NULL, though do execute domain checks for
+ * NULLs, which are handled by the coercion step.
+ */
+ if (pre_eval->formatted_expr.isnull || pre_eval->pathspec.isnull)
+ {
+ post_eval->coercion = NULL;
+ *op->resvalue = 0;
+ *op->resnull = true;
+ EEO_JUMP(op->d.jsonexpr_skip.jump_coercion);
+ }
+
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_JSONEXPR_PATH)
{
/* too complex for an inline implementation */
ExecEvalJson(state, op, econtext);
EEO_NEXT();
}
+ EEO_CASE(EEOP_JSONEXPR_ONEMPTY)
+ {
+ JsonExprPostEvalState *post_eval = op->d.jsonexpr_onempty.post_eval;
+ JsonExpr *jexpr = op->d.jsonexpr_onempty.jexpr;
+
+ /*
+ * Skip ON EMPTY handling if not empty or if it must be handled
+ * with a ON ERROR behavior instead.
+ */
+ if (post_eval->error)
+ EEO_JUMP(op->d.jsonexpr_onempty.jump_error);
+ else if (!post_eval->empty)
+ EEO_JUMP(op->d.jsonexpr_onempty.jump_coercion);
+
+ /*
+ * If a non-default behavior is specified, get the appropriate
+ * value and skip over to the coercion step.
+ */
+ if (jexpr->on_empty->btype != JSON_BEHAVIOR_DEFAULT)
+ {
+ *op->resvalue = ExecEvalJsonBehavior(jexpr->on_empty,
+ op->resnull);
+ EEO_JUMP(op->d.jsonexpr_onempty.jump_coercion);
+ }
+
+ /*
+ * Evaluate the default ON EMPTY expression.
+ *
+ * Nothing for _COERCION and _RESULT steps to do as the
+ * expression we are about to compute has been coerced
+ * appropriately by the parser.
+ */
+ post_eval->coercion_done = true;
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_JSONEXPR_ONERROR)
+ {
+ JsonExprPostEvalState *post_eval = op->d.jsonexpr_onerror.post_eval;
+ JsonExpr *jexpr = op->d.jsonexpr_onerror.jexpr;
+
+ /* Nothing to do if no error has been found to begin with. */
+ if (!post_eval->error)
+ EEO_JUMP(op->d.jsonexpr_onerror.jump_coercion);
+
+ /*
+ * If a non-default behavior is specified, get the appropriate
+ * value and skip over to the coercion step.
+ */
+ if (jexpr->on_error->btype != JSON_BEHAVIOR_DEFAULT)
+ {
+ /* Execute non-default ON ERROR behavior */
+ *op->resvalue = ExecEvalJsonBehavior(jexpr->on_error,
+ op->resnull);
+ EEO_JUMP(op->d.jsonexpr_onerror.jump_coercion);
+ }
+
+ /*
+ * Evaluate the default ON ERROR expression. See the note
+ * above regarding coercion.
+ */
+ post_eval->coercion_done = true;
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_JSONEXPR_COERCION_ERROR)
+ {
+ JsonExprPostEvalState *post_eval = op->d.jsonexpr_coercion_error.post_eval;
+ JsonExpr *jexpr = op->d.jsonexpr_coercion_error.jexpr;
+
+ /* Nothing to if no error was found when applying coercion. */
+ if (!post_eval->coercion_error)
+ EEO_JUMP(op->d.jsonexpr_coercion_error.jump_skip_coercion_error);
+
+ /*
+ * If a non-default behavior is specified, get the appropriate
+ * value and skip over to the coercion step.
+ */
+ if (jexpr->on_error->btype != JSON_BEHAVIOR_DEFAULT)
+ {
+ /* Execute non-default ON ERROR behavior */
+ *op->resvalue = ExecEvalJsonBehavior(jexpr->on_error,
+ op->resnull);
+
+ /* Coercion the ON ERROR expression. */
+ post_eval->coercion_done = false;
+ post_eval->coercion = NULL;
+ EEO_JUMP(op->d.jsonexpr_coercion_error.jump_coercion);
+ }
+
+ /*
+ * Evaluate the default ON ERROR expression. See the note
+ * above regarding coercion.
+ */
+ post_eval->coercion_done = true;
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_JSONEXPR_COERCION)
+ {
+ JsonExprState *jsestate = op->d.jsonexpr_coercion.jsestate;
+ JsonExprPostEvalState *post_eval = &jsestate->post_eval;
+ bool coersionUseSubtrans;
+
+ /*
+ * Don't use a sub-transaction if coercing the ON ERROR expression
+ * after the previous coercion attempt caused an error so that any
+ * that any errors that are encountered downstream this time are
+ * thrown there, that is, not rethrown to be handled here.
+ * XXX - doesn't that violate ON ERROR behavior? Actually, that is
+ * how it behaved even before this expression eval logic rewrite!
+ */
+ coersionUseSubtrans =
+ (jsestate->coercionNeedSubtrans &&
+ !post_eval->coercion_error);
+
+ /* ExecEvalJson() may have found coercion to be unnecessary. */
+ if (post_eval->coercion_done)
+ EEO_JUMP(op->d.jsonexpr_coercion.jump_skip_coercion);
+
+ post_eval->coercion_error = false;
+ *op->resvalue =
+ ExecEvalJsonExprSubtrans(ExecEvalJsonExprCoercion, op, econtext,
+ *op->resvalue,
+ op->resnull, NULL,
+ &post_eval->coercion_error,
+ coersionUseSubtrans);
+ if (post_eval->coercion_error)
+ EEO_JUMP(op->d.jsonexpr_coercion.jump_coercion_error);
+
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_JSONEXPR_RESULT)
+ {
+ JsonExprPostEvalState *post_eval = op->d.jsonexpr_result.post_eval;
+
+ if (post_eval->coercion_done)
+ EEO_JUMP(op->d.jsonexpr_result.jump_skip_result);
+
+ /* Evaluate result_coercion expression. */
+ EEO_NEXT();
+ }
+
EEO_CASE(EEOP_LAST)
{
/* unreachable */
@@ -4602,8 +4782,7 @@ ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op,
* Evaluate a JSON error/empty behavior result.
*/
static Datum
-ExecEvalJsonBehavior(ExprContext *econtext, JsonBehavior *behavior,
- ExprState *default_estate, bool *is_null)
+ExecEvalJsonBehavior(JsonBehavior *behavior, bool *is_null)
{
*is_null = false;
@@ -4628,7 +4807,9 @@ ExecEvalJsonBehavior(ExprContext *econtext, JsonBehavior *behavior,
return (Datum) 0;
case JSON_BEHAVIOR_DEFAULT:
- return ExecEvalExpr(default_estate, econtext, is_null);
+ /* Always handled in the caller. */
+ Assert(false);
+ return (Datum) 0;
default:
elog(ERROR, "unrecognized SQL/JSON behavior %d", behavior->btype);
@@ -4643,18 +4824,25 @@ static Datum
ExecEvalJsonExprCoercion(ExprEvalStep *op, ExprContext *econtext,
Datum res, bool *isNull, void *p, bool *error)
{
- ExprState *estate = p;
- JsonExprState *jsestate;
+ JsonExprState *jsestate = op->d.jsonexpr_coercion.jsestate;
+ JsonExprPostEvalState *post_eval = &jsestate->post_eval;
+ JsonExpr *jexpr = jsestate->jsexpr;
+ ExprState *estate = post_eval->coercion ?
+ post_eval->coercion->estate : NULL;
+
+ /*
+ * Initially assume we will indeed perform the coercion either
+ * using post_eval->coercion, with the input function for the
+ * output type, or with json_populate_type().
+ */
+ post_eval->coercion_done = true;
if (estate) /* coerce using specified expression */
return ExecEvalExpr(estate, econtext, isNull);
- jsestate = op->d.jsonexpr.jsestate;
-
- if (jsestate->jsexpr->op != JSON_EXISTS_OP)
+ if (jexpr->op != JSON_EXISTS_OP)
{
- JsonCoercion *coercion = jsestate->jsexpr->result_coercion;
- JsonExpr *jexpr = jsestate->jsexpr;
+ JsonCoercion *coercion = jexpr->result_coercion;
Jsonb *jb = *isNull ? NULL : DatumGetJsonbP(res);
if ((coercion && coercion->via_io) ||
@@ -4672,18 +4860,16 @@ ExecEvalJsonExprCoercion(ExprEvalStep *op, ExprContext *econtext,
return json_populate_type(res, JSONBOID,
jexpr->returning->typid,
jexpr->returning->typmod,
- &jsestate->cache,
+ &post_eval->cache,
econtext->ecxt_per_query_memory,
isNull);
}
- if (jsestate->result_expr)
- {
- jsestate->res_expr->value = res;
- jsestate->res_expr->isnull = *isNull;
-
- res = ExecEvalExpr(jsestate->result_expr, econtext, isNull);
- }
+ /*
+ * Let the caller know that no coercion was done here, so it can
+ * coerce with jexpr->result_coercion if there's one.
+ */
+ post_eval->coercion_done = false;
return res;
}
@@ -4717,17 +4903,30 @@ EvalJsonPathVar(void *cxt, char *varName, int varNameLen,
if (!var)
return -1;
- if (!var->evaluated)
+ /*
+ * When belonging to a JsonExpr, path variables are computed with the
+ * JsonExpr's ExprState (var->estate is NULL), so don't need to be computed
+ * here. In some other cases, such as when the path variables belonging
+ * to a JsonTable instead, those variables must be evaluated on their own,
+ * without the enclosing JsonExpr itself needing to be evaluated, so must
+ * be handled here.
+ */
+ if (var->estate && !var->evaluated)
{
MemoryContext oldcxt = var->mcxt ?
MemoryContextSwitchTo(var->mcxt) : NULL;
+ Assert(var->econtext != NULL);
var->value = ExecEvalExpr(var->estate, var->econtext, &var->isnull);
var->evaluated = true;
if (oldcxt)
MemoryContextSwitchTo(oldcxt);
}
+ else
+ {
+ Assert(var->evaluated);
+ }
if (var->isnull)
{
@@ -4832,9 +5031,6 @@ ExecPrepareJsonItemCoercion(JsonbValue *item,
return res;
}
-typedef Datum (*JsonFunc) (ExprEvalStep *op, ExprContext *econtext,
- Datum item, bool *resnull, void *p, bool *error);
-
static Datum
ExecEvalJsonExprSubtrans(JsonFunc func, ExprEvalStep *op,
ExprContext *econtext,
@@ -4905,7 +5101,6 @@ typedef struct
{
JsonPath *path;
bool *error;
- bool coercionInSubtrans;
} ExecEvalJsonExprContext;
static Datum
@@ -4916,16 +5111,17 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
ExecEvalJsonExprContext *cxt = pcxt;
JsonPath *path = cxt->path;
JsonExprState *jsestate = op->d.jsonexpr.jsestate;
+ JsonExprPreEvalState *pre_eval = &jsestate->pre_eval;
+ JsonExprPostEvalState *post_eval = &jsestate->post_eval;
JsonExpr *jexpr = jsestate->jsexpr;
- ExprState *estate = NULL;
- bool empty = false;
+ bool *empty = &post_eval->empty;
Datum res = (Datum) 0;
switch (jexpr->op)
{
case JSON_QUERY_OP:
- res = JsonPathQuery(item, path, jexpr->wrapper, &empty, error,
- jsestate->args);
+ res = JsonPathQuery(item, path, jexpr->wrapper, empty, error,
+ pre_eval->args);
if (error && *error)
{
*resnull = true;
@@ -4937,16 +5133,19 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
case JSON_VALUE_OP:
{
struct JsonCoercionState *jcstate;
- JsonbValue *jbv = JsonPathValue(item, path, &empty, error,
- jsestate->args);
+ JsonbValue *jbv = JsonPathValue(item, path, empty, error,
+ pre_eval->args);
if (error && *error)
+ {
+ *resnull = true;
return (Datum) 0;
+ }
if (!jbv) /* NULL or empty */
break;
- Assert(!empty);
+ Assert(!*empty);
*resnull = false;
@@ -4964,7 +5163,6 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
jsestate->jsexpr->returning,
&jsestate->coercions,
&jcstate);
-
if (jcstate->coercion &&
(jcstate->coercion->via_io ||
jcstate->coercion->via_populate))
@@ -4972,6 +5170,7 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
if (error)
{
*error = true;
+ *resnull = true;
return (Datum) 0;
}
@@ -4983,32 +5182,30 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
(errcode(ERRCODE_SQL_JSON_ITEM_CANNOT_BE_CAST_TO_TARGET_TYPE),
errmsg("SQL/JSON item cannot be cast to target type")));
}
- else if (!jcstate->estate)
- return res; /* no coercion */
-
- /* coerce using specific expression */
- estate = jcstate->estate;
- jsestate->coercion_expr->value = res;
- jsestate->coercion_expr->isnull = *resnull;
+ else if (jcstate->estate == NULL)
+ {
+ /* No coercion needed */
+ post_eval->coercion_done = true;
+ }
+ else
+ {
+ /* Coerce using specific expression */
+ post_eval->coercion = jcstate;
+ }
break;
}
case JSON_EXISTS_OP:
{
bool exists = JsonPathExists(item, path,
- jsestate->args,
+ pre_eval->args,
error);
*resnull = error && *error;
res = BoolGetDatum(exists);
- if (!jsestate->result_expr)
+ if (jexpr->result_coercion == NULL)
return res;
-
- /* coerce using result expression */
- estate = jsestate->result_expr;
- jsestate->res_expr->value = res;
- jsestate->res_expr->isnull = *resnull;
break;
}
@@ -5021,7 +5218,11 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
return (Datum) 0;
}
- if (empty)
+ /*
+ * If the ON EMPTY behavior is to cause an error, do so here. Other
+ * behaviors will be handled by the caller.
+ */
+ if (*empty)
{
Assert(jexpr->on_empty); /* it is not JSON_EXISTS */
@@ -5037,24 +5238,9 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
(errcode(ERRCODE_NO_SQL_JSON_ITEM),
errmsg("no SQL/JSON item")));
}
-
- if (jexpr->on_empty->btype == JSON_BEHAVIOR_DEFAULT)
-
- /*
- * Execute DEFAULT expression as a coercion expression, because
- * its result is already coerced to the target type.
- */
- estate = jsestate->default_on_empty;
- else
- /* Execute ON EMPTY behavior */
- res = ExecEvalJsonBehavior(econtext, jexpr->on_empty,
- jsestate->default_on_empty,
- resnull);
}
- return ExecEvalJsonExprSubtrans(ExecEvalJsonExprCoercion, op, econtext,
- res, resnull, estate, error,
- cxt->coercionInSubtrans);
+ return res;
}
bool
@@ -5082,64 +5268,30 @@ ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
{
ExecEvalJsonExprContext cxt;
JsonExprState *jsestate = op->d.jsonexpr.jsestate;
+ JsonExprPreEvalState *pre_eval = &jsestate->pre_eval;
+ JsonExprPostEvalState *post_eval = &jsestate->post_eval;
JsonExpr *jexpr = jsestate->jsexpr;
Datum item;
Datum res = (Datum) 0;
JsonPath *path;
- ListCell *lc;
- bool error = false;
- bool needSubtrans;
bool throwErrors = jexpr->on_error->btype == JSON_BEHAVIOR_ERROR;
*op->resnull = true; /* until we get a result */
*op->resvalue = (Datum) 0;
- if (jsestate->formatted_expr->isnull || jsestate->pathspec->isnull)
- {
- /* execute domain checks for NULLs */
- (void) ExecEvalJsonExprCoercion(op, econtext, res, op->resnull,
- NULL, NULL);
+ item = pre_eval->formatted_expr.value;
+ path = DatumGetJsonPathP(pre_eval->pathspec.value);
- Assert(*op->resnull);
- return;
- }
-
- item = jsestate->formatted_expr->value;
- path = DatumGetJsonPathP(jsestate->pathspec->value);
-
- /* reset JSON path variable contexts */
- foreach(lc, jsestate->args)
- {
- JsonPathVariableEvalContext *var = lfirst(lc);
-
- var->econtext = econtext;
- var->evaluated = false;
- }
-
- needSubtrans = ExecEvalJsonNeedsSubTransaction(jexpr, &jsestate->coercions);
+ /* Reset JsonExprPostEvalState for this evaluation. */
+ memset(post_eval, 0, sizeof(*post_eval));
cxt.path = path;
- cxt.error = throwErrors ? NULL : &error;
- cxt.coercionInSubtrans = !needSubtrans && !throwErrors;
- Assert(!needSubtrans || cxt.error);
+ cxt.error = throwErrors ? NULL : &post_eval->error;
+ Assert(!jsestate->needSubtrans || cxt.error);
res = ExecEvalJsonExprSubtrans(ExecEvalJsonExpr, op, econtext, item,
op->resnull, &cxt, cxt.error,
- needSubtrans);
-
- if (error)
- {
- /* Execute ON ERROR behavior */
- res = ExecEvalJsonBehavior(econtext, jexpr->on_error,
- jsestate->default_on_error,
- op->resnull);
-
- /* result is already coerced in DEFAULT behavior case */
- if (jexpr->on_error->btype != JSON_BEHAVIOR_DEFAULT)
- res = ExecEvalJsonExprCoercion(op, econtext, res,
- op->resnull,
- NULL, NULL);
- }
+ jsestate->needSubtrans);
*op->resvalue = res;
}
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index b6b6512ef1..c133600005 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -2359,11 +2359,44 @@ llvm_compile_expr(ExprState *state)
LLVMBuildBr(b, opblocks[opno + 1]);
break;
- case EEOP_JSONEXPR:
+ case EEOP_JSONEXPR_SKIP:
+ /* XXX - handle! */
+ break;
+ case EEOP_JSONEXPR_PATH:
build_EvalXFunc(b, mod, "ExecEvalJson",
v_state, op, v_econtext);
LLVMBuildBr(b, opblocks[opno + 1]);
break;
+ case EEOP_JSONEXPR_ONERROR:
+ /* XXX - handle! */
+ break;
+ case EEOP_JSONEXPR_ONEMPTY:
+ /* XXX - handle! */
+ break;
+ case EEOP_JSONEXPR_COERCION:
+ /* XXX - handle! */
+ break;
+ case EEOP_JSONEXPR_COERCION_ERROR:
+ /* XXX - handle! */
+ break;
+ case EEOP_JSONEXPR_RESULT:
+ {
+ /*
+ * Perform the step at jsonexpr_result.jumpnext if the
+ * coercion already done, which effectively skips the
+ * step(s) to perform the coercion using the result
+ * expression.
+ * XXX - Hoping this DTRT.
+ */
+ LLVMBuildCondBr(b,
+ LLVMBuildICmp(b, LLVMIntEQ,
+ l_sbool_const(op->d.jsonexpr_result.post_eval->coercion_done),
+ l_sbool_const(1),
+ ""),
+ opblocks[op->d.jsonexpr_result.jump_skip_result],
+ opblocks[opno + 1]);
+ }
+ break;
case EEOP_LAST:
Assert(false);
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index e55a572854..358b831f7a 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -244,7 +244,13 @@ typedef enum ExprEvalOp
EEOP_SUBPLAN,
EEOP_JSON_CONSTRUCTOR,
EEOP_IS_JSON,
- EEOP_JSONEXPR,
+ EEOP_JSONEXPR_SKIP,
+ EEOP_JSONEXPR_PATH,
+ EEOP_JSONEXPR_ONERROR,
+ EEOP_JSONEXPR_ONEMPTY,
+ EEOP_JSONEXPR_COERCION,
+ EEOP_JSONEXPR_COERCION_ERROR,
+ EEOP_JSONEXPR_RESULT,
/* aggregation related nodes */
EEOP_AGG_STRICT_DESERIALIZE,
@@ -682,12 +688,71 @@ typedef struct ExprEvalStep
JsonIsPredicate *pred; /* original expression node */
} is_json;
- /* for EEOP_JSONEXPR */
+ /* for EEOP_JSONEXPR_PATH */
struct
{
struct JsonExprState *jsestate;
} jsonexpr;
+ /* for EEOP_JSONEXPR_SKIP */
+ struct
+ {
+ /* Same as jsonexpr.jsestate */
+ struct JsonExprPreEvalState *pre_eval;
+ struct JsonExprPostEvalState *post_eval;
+ int jump_coercion;
+ } jsonexpr_skip;
+
+ /* for EEOP_JSONEXPR_ONEMPTY */
+ struct
+ {
+ /* Points to jsonexpr.jsestate->post_eval */
+ struct JsonExprPostEvalState *post_eval;
+ /* Same as jsonexpr.jsestate->jsexpr */
+ JsonExpr *jexpr;
+ int jump_error;
+ int jump_coercion;
+ } jsonexpr_onempty;
+
+ /* for EEOP_JSONEXPR_ONERROR */
+ struct
+ {
+ /* Points to jsonexpr.jsestate->post_eval */
+ struct JsonExprPostEvalState *post_eval;
+ /* Same as jsonexpr.jsestate->jsexpr */
+ JsonExpr *jexpr;
+ int jump_coercion;
+ } jsonexpr_onerror;
+
+ /* for EEOP_JSONEXPR_COERCION */
+ struct
+ {
+ /* Same as jsonexpr.jsestate */
+ struct JsonExprState *jsestate;
+ int jump_skip_coercion;
+ int jump_coercion_error;
+ } jsonexpr_coercion;
+
+ /* for EEOP_JSONEXPR_COERCION_ERROR */
+ struct
+ {
+ /* Points to jsonexpr.jsestate->post_eval */
+ struct JsonExprPostEvalState *post_eval;
+ /* Same as jsonexpr.jsestate->jsexpr */
+ JsonExpr *jexpr;
+ int jump_skip_coercion_error;
+ int jump_coercion;
+ } jsonexpr_coercion_error;
+
+ /* for EEOP_JSONEXPR_RESULT */
+ struct
+ {
+ /* Points to jsonexpr.jsestate->post_eval */
+ struct JsonExprPostEvalState *post_eval;
+ /* Same as jsonexpr.jsestate->jsexpr */
+ JsonExpr *jexpr;
+ int jump_skip_result;
+ } jsonexpr_result;
} d;
} ExprEvalStep;
@@ -747,30 +812,69 @@ typedef struct JsonConstructorExprState
int nargs;
} JsonConstructorExprState;
+/*
+ * State for (EEOP_JSONEXPR_) ONERROR, ONEMPTY, COERCION, COERCION_ERROR,
+ * and RESULT steps that will run after EEOP_JSONEXPR_PATH.
+ */
+typedef struct JsonExprPostEvalState
+{
+ /* Is JSON item empty? */
+ bool empty;
+
+ /* Did JSON item evaluation cause an error? */
+ bool error;
+
+ /* Did coercion evaluation cause an error? */
+ bool coercion_error;
+
+ /* Has the result been coerced properly? */
+ bool coercion_done;
+
+ /* Cache for json_populate_type() called for coercion in some cases */
+ void *cache;
+
+ /*
+ * Coercion to be applied to JSON item returned by JsonPathValue(),
+ * set by ExecPrepareJsonItemCoercion(). */
+ struct JsonCoercionState *coercion;
+} JsonExprPostEvalState;
+
+/*
+ * Information computed before evaluating a JsonExpr expression.
+ */
+typedef struct JsonExprPreEvalState
+{
+ /* value/isnull for JsonExpr.formatted_expr */
+ NullableDatum formatted_expr;
+
+ /* value/isnull for JsonExpr.pathspec */
+ NullableDatum pathspec;
+
+ /* JsonPathVariableEvalContext entries for JsonExpr.passing_values */
+ List *args;
+} JsonExprPreEvalState;
+
/* EEOP_JSONEXPR state, too big to inline */
typedef struct JsonExprState
{
JsonExpr *jsexpr; /* original expression node */
+ JsonExprPreEvalState pre_eval;
+ JsonExprPostEvalState post_eval;
+
+ /*
+ * Should use a sub-transaction for path evaluation and subsequent
+ * coercion evaluation, if any?
+ */
+ bool needSubtrans;
+ bool coercionNeedSubtrans;
+
struct
{
FmgrInfo *finfo; /* typinput function for output type */
Oid typioparam;
} input; /* I/O info for output type */
- NullableDatum
- *formatted_expr, /* formatted context item value */
- *res_expr, /* result item */
- *coercion_expr, /* input for JSON item coercion */
- *pathspec; /* path specification value */
-
- ExprState *result_expr; /* coerced to output type */
- ExprState *default_on_empty; /* ON EMPTY DEFAULT expression */
- ExprState *default_on_error; /* ON ERROR DEFAULT expression */
- List *args; /* passing arguments */
-
- void *cache; /* cache for json_populate_type() */
-
struct JsonCoercionsState
{
struct JsonCoercionState
@@ -779,7 +883,7 @@ typedef struct JsonExprState
ExprState *estate; /* coercion expression state */
} null,
string,
- numeric ,
+ numeric,
boolean,
date,
time,
--
2.34.1
From 5d904e7f8357345079531251849c9e4fe9d161b1 Mon Sep 17 00:00:00 2001
From: amitlan <amitlangot...@gmail.com>
Date: Mon, 18 Jul 2022 11:06:04 -0400
Subject: [PATCH 3/3] Use one ExprState to implement JsonItemCoercions
Current implementation assigns one ExprState for every JsonCoercion
member of JsonItemCoercions, which seems wasteful.
This commit instead assigns the ExprState to JsonItemCoercions itself
and arranges to compute the member JsonCoercion's expressions using
ExprEvalSteps instead.
---
src/backend/executor/execExpr.c | 113 ++++++++++++++++++----
src/backend/executor/execExprInterp.c | 133 ++++++++++++++++++++------
src/backend/jit/llvm/llvmjit_expr.c | 3 +
src/backend/optimizer/util/clauses.c | 6 +-
src/include/executor/execExpr.h | 75 +++++++++------
src/include/utils/jsonb.h | 2 +
6 files changed, 256 insertions(+), 76 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 4dbb24aaee..be98544e05 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2786,7 +2786,8 @@ ExecInitExprRec(Expr *node, ExprState *state,
* query.
*/
jsestate->needSubtrans =
- ExecEvalJsonNeedsSubTransaction(jexpr, &jsestate->coercions);
+ ExecEvalJsonNeedsSubTransaction(jexpr,
+ true /* coerce */);
jsestate->coercionNeedSubtrans =
(!jsestate->needSubtrans && !throwErrors);
@@ -2810,26 +2811,102 @@ ExecInitExprRec(Expr *node, ExprState *state,
/* Set up coercion related data structures. */
if (jexpr->coercions)
+ jsestate->coercions =
+ ExecInitExprWithCaseValue((Expr *) jexpr->coercions,
+ state->parent,
+ resv, resnull);
+
+ break;
+ }
+
+ case T_JsonItemCoercions:
+ {
+ JsonItemCoercions *coercions = castNode(JsonItemCoercions,
+ node);
+ JsonItemCoercionsState *jcstate =
+ palloc0(sizeof(JsonItemCoercionsState));
+ JsonCoercion **coercion;
+ JsonItemCoercionState *cstate;
+ ExprEvalStep *as;
+ int json_item_coercion_step_id;
+ List *adjust_jumps = NIL;
+ ListCell *lc;
+
+ /*
+ * First push a step to read the value provided by the parent
+ * JsonExpr via a CaseTestExpr.
+ */
+ scratch.opcode = EEOP_CASE_TESTVAL;
+ scratch.d.casetest.value = state->innermost_caseval;
+ scratch.d.casetest.isnull = state->innermost_casenull;
+ ExprEvalPushStep(state, &scratch);
+
+ /* Push the control step. */
+ scratch.opcode = EEOP_JSON_ITEM_COERCION;
+ scratch.d.json_item_coercion.jcstate = jcstate;
+ json_item_coercion_step_id = state->steps_len;
+ ExprEvalPushStep(state, &scratch);
+ /* Will set jump_skip_coercion target address below. */
+
+ /*
+ * Now push the steps of individual coercion's expression, if
+ * needed.
+ */
+ for (cstate = &jcstate->null,
+ coercion = &coercions->null;
+ coercion <= &coercions->composite;
+ coercion++, cstate++)
{
- JsonCoercion **coercion;
- struct JsonCoercionState *cstate;
- Datum *caseval;
- bool *casenull;
-
- caseval = resv;
- casenull = resnull;
-
- for (cstate = &jsestate->coercions.null,
- coercion = &jexpr->coercions->null;
- coercion <= &jexpr->coercions->composite;
- coercion++, cstate++)
+ cstate->coercion = *coercion;
+ if (cstate->coercion && cstate->coercion->expr)
{
- cstate->coercion = *coercion;
- cstate->estate = *coercion ?
- ExecInitExprWithCaseValue((Expr *) (*coercion)->expr,
- state->parent,
- caseval, casenull) : NULL;
+ Datum *save_innermost_caseval;
+ bool *save_innermost_isnull;
+
+ cstate->jump_eval_expr = state->steps_len;
+
+ /* Push step(s) to compute (*coercion)->expr. */
+ save_innermost_caseval = state->innermost_caseval;
+ save_innermost_isnull = state->innermost_casenull;
+
+ state->innermost_caseval = resv;
+ state->innermost_casenull = resnull;
+
+ ExecInitExprRec((Expr *) cstate->coercion->expr,
+ state, resv, resnull);
+
+ state->innermost_caseval = save_innermost_caseval;
+ state->innermost_casenull = save_innermost_isnull;
}
+ else
+ cstate->jump_eval_expr = -1;
+
+ /* Emit JUMP step to jump to end of coercions code */
+ scratch.opcode = EEOP_JUMP;
+
+ /*
+ * Remember JUMP step address to set the actual jump
+ * target address below.
+ */
+ adjust_jumps = lappend_int(adjust_jumps,
+ state->steps_len);
+ ExprEvalPushStep(state, &scratch);
+ }
+
+ /*
+ * Adjust the jump target address of the control step and all
+ * the jumps we added in the above loop to make them point to
+ * the step after the last step that would have been added
+ * above.
+ */
+ as = &state->steps[json_item_coercion_step_id];
+ as->d.json_item_coercion.jump_skip_item_coercion = state->steps_len;
+ foreach(lc, adjust_jumps)
+ {
+ int jump_step_id = lfirst_int(lc);
+
+ as = &state->steps[jump_step_id];
+ as->d.jump.jumpdone = state->steps_len;
}
break;
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 7a533c16df..f09d643921 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -506,6 +506,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
&&CASE_EEOP_JSONEXPR_COERCION,
&&CASE_EEOP_JSONEXPR_COERCION_ERROR,
&&CASE_EEOP_JSONEXPR_RESULT,
+ &&CASE_EEOP_JSON_ITEM_COERCION,
&&CASE_EEOP_AGG_STRICT_DESERIALIZE,
&&CASE_EEOP_AGG_DESERIALIZE,
&&CASE_EEOP_AGG_STRICT_INPUT_CHECK_ARGS,
@@ -1844,7 +1845,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
*/
if (pre_eval->formatted_expr.isnull || pre_eval->pathspec.isnull)
{
- post_eval->coercion = NULL;
+ post_eval->coercions = NULL;
*op->resvalue = 0;
*op->resnull = true;
EEO_JUMP(op->d.jsonexpr_skip.jump_coercion);
@@ -1946,7 +1947,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
/* Coercion the ON ERROR expression. */
post_eval->coercion_done = false;
- post_eval->coercion = NULL;
+ post_eval->coercions = NULL;
EEO_JUMP(op->d.jsonexpr_coercion_error.jump_coercion);
}
@@ -2004,6 +2005,23 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_NEXT();
}
+ EEO_CASE(EEOP_JSON_ITEM_COERCION)
+ {
+ JsonItemCoercionsState *jcstate = op->d.json_item_coercion.jcstate;
+ JsonItemCoercionState *cstate = NULL;
+
+ *op->resvalue =
+ ExecPrepareJsonItemCoercion(*op->resvalue, jcstate, &cstate);
+ if (cstate->coercion && cstate->coercion->expr)
+ {
+ Assert(cstate->jump_eval_expr >= 0);
+ EEO_JUMP(cstate->jump_eval_expr);
+ }
+
+ /* Skip over all of the steps added for this JsonItemCoercions. */
+ EEO_JUMP(op->d.json_item_coercion.jump_skip_item_coercion);
+ }
+
EEO_CASE(EEOP_LAST)
{
/* unreachable */
@@ -4827,8 +4845,7 @@ ExecEvalJsonExprCoercion(ExprEvalStep *op, ExprContext *econtext,
JsonExprState *jsestate = op->d.jsonexpr_coercion.jsestate;
JsonExprPostEvalState *post_eval = &jsestate->post_eval;
JsonExpr *jexpr = jsestate->jsexpr;
- ExprState *estate = post_eval->coercion ?
- post_eval->coercion->estate : NULL;
+ ExprState *estate = post_eval->coercions;
/*
* Initially assume we will indeed perform the coercion either
@@ -4940,19 +4957,84 @@ EvalJsonPathVar(void *cxt, char *varName, int varNameLen,
return id;
}
+/*
+ * Return a coercion among those given in 'coercions' for given
+ * JSON item.
+ */
+JsonCoercion *
+ExecGetJsonItemCoercion(JsonbValue *item, JsonItemCoercions *coercions)
+{
+ if (item->type == jbvBinary &&
+ JsonContainerIsScalar(item->val.binary.data))
+ {
+ JsonbValue buf;
+ bool res PG_USED_FOR_ASSERTS_ONLY;
+
+ res = JsonbExtractScalar(item->val.binary.data, &buf);
+ item = &buf;
+ Assert(res);
+ }
+
+ /* get coercion state reference and datum of the corresponding SQL type */
+ switch (item->type)
+ {
+ case jbvNull:
+ return coercions->null;
+
+ case jbvString:
+ return coercions->string;
+
+ case jbvNumeric:
+ return coercions->numeric;
+
+ case jbvBool:
+ return coercions->boolean;
+
+ case jbvDatetime:
+ switch (item->val.datetime.typid)
+ {
+ case DATEOID:
+ return coercions->date;
+ case TIMEOID:
+ return coercions->time;
+ case TIMETZOID:
+ return coercions->timetz;
+ case TIMESTAMPOID:
+ return coercions->timestamp;
+ case TIMESTAMPTZOID:
+ return coercions->timestamptz;
+ default:
+ elog(ERROR, "unexpected jsonb datetime type oid %u",
+ item->val.datetime.typid);
+ }
+ break;
+
+ case jbvArray:
+ case jbvObject:
+ case jbvBinary:
+ return coercions->composite;
+
+ default:
+ elog(ERROR, "unexpected jsonb value type %d", item->type);
+ }
+
+ Assert(false);
+ return NULL;
+}
+
/*
* Prepare SQL/JSON item coercion to the output type. Returned a datum of the
* corresponding SQL type and a pointer to the coercion state.
*/
Datum
-ExecPrepareJsonItemCoercion(JsonbValue *item,
- JsonReturning *returning,
- struct JsonCoercionsState *coercions,
- struct JsonCoercionState **pcoercion)
+ExecPrepareJsonItemCoercion(Datum itemval,
+ JsonItemCoercionsState *coercions,
+ JsonItemCoercionState **pcoercion)
{
- struct JsonCoercionState *coercion;
+ JsonbValue *item = DatumGetJsonbValueP(itemval);
+ JsonItemCoercionState *coercion;
Datum res;
- JsonbValue buf;
+ JsonbValue buf;
if (item->type == jbvBinary &&
JsonContainerIsScalar(item->val.binary.data))
@@ -5132,7 +5214,7 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
case JSON_VALUE_OP:
{
- struct JsonCoercionState *jcstate;
+ JsonCoercion *coercion;
JsonbValue *jbv = JsonPathValue(item, path, empty, error,
pre_eval->args);
@@ -5158,14 +5240,12 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
break;
}
- /* Use coercion from SQL/JSON item type to the output type */
- res = ExecPrepareJsonItemCoercion(jbv,
- jsestate->jsexpr->returning,
- &jsestate->coercions,
- &jcstate);
- if (jcstate->coercion &&
- (jcstate->coercion->via_io ||
- jcstate->coercion->via_populate))
+ /*
+ * Error out no cast exists to coerce SQL/JSON item to the
+ * the output type
+ */
+ coercion = ExecGetJsonItemCoercion(jbv, jsestate->jsexpr->coercions);
+ if (coercion && (coercion->via_io || coercion->via_populate))
{
if (error)
{
@@ -5182,15 +5262,11 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
(errcode(ERRCODE_SQL_JSON_ITEM_CANNOT_BE_CAST_TO_TARGET_TYPE),
errmsg("SQL/JSON item cannot be cast to target type")));
}
- else if (jcstate->estate == NULL)
- {
- /* No coercion needed */
- post_eval->coercion_done = true;
- }
else
{
- /* Coerce using specific expression */
- post_eval->coercion = jcstate;
+ /* Coerce using specific expression. */
+ res = JsonbValuePGetDatum(jbv);
+ post_eval->coercions = jsestate->coercions;
}
break;
}
@@ -5244,8 +5320,7 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext,
}
bool
-ExecEvalJsonNeedsSubTransaction(JsonExpr *jsexpr,
- struct JsonCoercionsState *coercions)
+ExecEvalJsonNeedsSubTransaction(JsonExpr *jsexpr, bool coerce)
{
if (jsexpr->on_error->btype == JSON_BEHAVIOR_ERROR)
return false;
@@ -5253,7 +5328,7 @@ ExecEvalJsonNeedsSubTransaction(JsonExpr *jsexpr,
if (jsexpr->op == JSON_EXISTS_OP && !jsexpr->result_coercion)
return false;
- if (!coercions)
+ if (!coerce)
return true;
return false;
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index c133600005..5f06a87373 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -2397,6 +2397,9 @@ llvm_compile_expr(ExprState *state)
opblocks[opno + 1]);
}
break;
+ case EEOP_JSON_ITEM_COERCION:
+ /* XXX - handle! */
+ break;
case EEOP_LAST:
Assert(false);
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 533df86ff7..9c02218155 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -901,7 +901,11 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context)
{
JsonExpr *jsexpr = (JsonExpr *) node;
- if (ExecEvalJsonNeedsSubTransaction(jsexpr, NULL))
+ /*
+ * XXX - don't really know why it makes sense to ignore the coercion
+ * part here.
+ */
+ if (ExecEvalJsonNeedsSubTransaction(jsexpr, false /* coerce */))
{
context->max_hazard = PROPARALLEL_UNSAFE;
return true;
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 358b831f7a..2639d7c73d 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -251,6 +251,7 @@ typedef enum ExprEvalOp
EEOP_JSONEXPR_COERCION,
EEOP_JSONEXPR_COERCION_ERROR,
EEOP_JSONEXPR_RESULT,
+ EEOP_JSON_ITEM_COERCION,
/* aggregation related nodes */
EEOP_AGG_STRICT_DESERIALIZE,
@@ -753,6 +754,13 @@ typedef struct ExprEvalStep
JsonExpr *jexpr;
int jump_skip_result;
} jsonexpr_result;
+
+ /* for EEOP_JSON_ITEM_COERCION */
+ struct
+ {
+ struct JsonItemCoercionsState *jcstate;
+ int jump_skip_item_coercion;
+ } json_item_coercion;
} d;
} ExprEvalStep;
@@ -833,10 +841,8 @@ typedef struct JsonExprPostEvalState
/* Cache for json_populate_type() called for coercion in some cases */
void *cache;
- /*
- * Coercion to be applied to JSON item returned by JsonPathValue(),
- * set by ExecPrepareJsonItemCoercion(). */
- struct JsonCoercionState *coercion;
+ /* Coercion state; same as parent JsonExprState.coercions when not NULL */
+ ExprState *coercions;
} JsonExprPostEvalState;
/*
@@ -875,26 +881,40 @@ typedef struct JsonExprState
Oid typioparam;
} input; /* I/O info for output type */
- struct JsonCoercionsState
- {
- struct JsonCoercionState
- {
- JsonCoercion *coercion; /* coercion expression */
- ExprState *estate; /* coercion expression state */
- } null,
- string,
- numeric,
- boolean,
- date,
- time,
- timetz,
- timestamp,
- timestamptz,
- composite;
- } coercions; /* states for coercion from SQL/JSON item
- * types directly to the output type */
+ /* To apply coercion to the final result if needed. */
+ ExprState *coercions;
} JsonExprState;
+/*
+ * State for a given member of JsonItemCoercions.
+ */
+typedef struct JsonItemCoercionState
+{
+ /* Expression used to evaluate the coercion */
+ JsonCoercion *coercion;
+
+ /* ExprEvalStep to compute this coercion's expression */
+ int jump_eval_expr;
+} JsonItemCoercionState;
+
+/*
+ * State for evaluating the coercion for a given JSON item using one of
+ * the following coercions.
+ */
+typedef struct JsonItemCoercionsState
+{
+ JsonItemCoercionState null;
+ JsonItemCoercionState string;
+ JsonItemCoercionState numeric;
+ JsonItemCoercionState boolean;
+ JsonItemCoercionState date;
+ JsonItemCoercionState time;
+ JsonItemCoercionState timetz;
+ JsonItemCoercionState timestamp;
+ JsonItemCoercionState timestamptz;
+ JsonItemCoercionState composite;
+} JsonItemCoercionsState;
+
/* functions in execExpr.c */
extern void ExprEvalPushStep(ExprState *es, const ExprEvalStep *s);
@@ -956,12 +976,11 @@ extern void ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
extern void ExecEvalJson(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
-extern Datum ExecPrepareJsonItemCoercion(struct JsonbValue *item,
- JsonReturning *returning,
- struct JsonCoercionsState *coercions,
- struct JsonCoercionState **pjcstate);
-extern bool ExecEvalJsonNeedsSubTransaction(JsonExpr *jsexpr,
- struct JsonCoercionsState *);
+JsonCoercion *ExecGetJsonItemCoercion(struct JsonbValue *item, JsonItemCoercions *coercions);
+extern Datum ExecPrepareJsonItemCoercion(Datum itemval,
+ JsonItemCoercionsState *coercions,
+ JsonItemCoercionState **pjcstate);
+extern bool ExecEvalJsonNeedsSubTransaction(JsonExpr *jsexpr, bool coerce);
extern Datum ExecEvalExprPassingCaseValue(ExprState *estate,
ExprContext *econtext, bool *isnull,
Datum caseval_datum,
diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h
index bae466b523..6bdd9f5121 100644
--- a/src/include/utils/jsonb.h
+++ b/src/include/utils/jsonb.h
@@ -69,8 +69,10 @@ typedef enum
/* Convenience macros */
#define DatumGetJsonbP(d) ((Jsonb *) PG_DETOAST_DATUM(d))
+#define DatumGetJsonbValueP(d) ((JsonbValue *) DatumGetPointer(d))
#define DatumGetJsonbPCopy(d) ((Jsonb *) PG_DETOAST_DATUM_COPY(d))
#define JsonbPGetDatum(p) PointerGetDatum(p)
+#define JsonbValuePGetDatum(p) PointerGetDatum(p)
#define PG_GETARG_JSONB_P(x) DatumGetJsonbP(PG_GETARG_DATUM(x))
#define PG_GETARG_JSONB_P_COPY(x) DatumGetJsonbPCopy(PG_GETARG_DATUM(x))
#define PG_RETURN_JSONB_P(x) PG_RETURN_POINTER(x)
--
2.34.1
