Hi, On 2022-06-17 10:30:55 -0700, Andres Freund wrote: > On 2022-06-17 10:33:08 -0400, Tom Lane wrote: > > Andres Freund <and...@anarazel.de> writes: > > > The remaining difference looks like it's largely caused by the > > > enable_timeout_after(IDLE_STATS_UPDATE_TIMEOUT, ...) introduced as part > > > of the > > > pgstats patch. It's only really visible when I pin a single connection > > > pgbench > > > to the same CPU core as the server (which gives a ~16% boost here). > > > > > It's not the timeout itself - that we amortize nicely (via 09cf1d522). > > > It's > > > that enable_timeout_after() does a GetCurrentTimestamp(). > > > > > Not sure yet what the best way to fix that is. > > > > Maybe not queue a new timeout if the old one is still active? > > Right now we disable the timer after ReadCommand(). We can of course change > that. At first I thought we might need more bookkeeping to do so, to avoid > ProcessInterrupts() triggering pgstat_report_stat() when the timer fires > later, but we probably can jury-rig something with DoingCommandRead && > IsTransactionOrTransactionBlock() or such.
Here's a patch for that. One thing I noticed is that disable_timeout() calls do schedule_alarm(GetCurrentTimestamp()) if there's any other active timeout, even if the to-be-disabled timer is already disabled. Of course callers of disable_timeout() can guard against that using get_timeout_active(), but that spreads repetitive code around... I opted to add a fastpath for that, instead of using get_timeout_active(). Afaics that's safe to do without disarming the signal handler, but I'd welcome a look from somebody that knows this code. > I guess one advantage of something like this could be that we could possibly > move the arming of the timeout to pgstat.c. But that looks like it might be > more complicated than really worth it. I didn't do that yet, but am curious whether others think this would be preferrable. > > BTW, it looks like that patch also falsified this comment > > (postgres.c:4478): > > > > * At most one of these timeouts will be active, so there's no > > need to > > * worry about combining the timeout.c calls into one. > > Hm, yea. I guess we can just disable them at once. With the proposed change we don't need to change the separate timeout.c to one, or update the comment, as it should now look the same as 14. I also attached my heavily-WIP patches for the ExprEvalStep issues, I accidentally had only included a small part of the contents of the json fix. Greetings, Andres Freund
>From 5fc220ce4ed8406e7c6852c6a6c59fb74a6e3f82 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Fri, 17 Jun 2022 12:51:20 -0700 Subject: [PATCH v2 1/4] Add already-disabled fastpath to disable_timeout(). Otherwise callers that might call disable_timeout() frequently need to check get_timeout_active() to avoid GetCurrentTimestamp() and other overheads in disable_timeout(). Needed to avoid get_timeout_active() check in the subsequent commit fixing a small performance regression. --- src/backend/utils/misc/timeout.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c index 6f5e08bc302..d1219afa614 100644 --- a/src/backend/utils/misc/timeout.c +++ b/src/backend/utils/misc/timeout.c @@ -692,6 +692,11 @@ disable_timeout(TimeoutId id, bool keep_indicator) Assert(all_timeouts_initialized); Assert(all_timeouts[id].timeout_handler != NULL); + /* fast path for an already disabled timer */ + if (!all_timeouts[id].active && + (!all_timeouts[id].indicator || keep_indicator)) + return; + /* Disable timeout interrupts for safety. */ disable_alarm(); -- 2.35.1.677.gabf474a5dd
>From 41ff50e1264d2d32bc35b8b2c6c4ca375c90017a Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Fri, 17 Jun 2022 12:48:34 -0700 Subject: [PATCH v2 2/4] WIP: pgstat: reduce timer overhead. While at it, also update assertion in pgstat_report_stat() to be more precise. Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/tcop/postgres.c | 47 ++++++++++++++++++----------- src/backend/utils/activity/pgstat.c | 2 +- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 8b6b5bbaaab..a3aa9063dc9 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3371,10 +3371,13 @@ ProcessInterrupts(void) IdleSessionTimeoutPending = false; } - if (IdleStatsUpdateTimeoutPending) + /* + * If there are pending stats updates and we currently are truly idle + * (matching the conditions in PostgresMain(), report stats now. + */ + if (IdleStatsUpdateTimeoutPending && + DoingCommandRead && !IsTransactionOrTransactionBlock()) { - /* timer should have been disarmed */ - Assert(!IsTransactionBlock()); IdleStatsUpdateTimeoutPending = false; pgstat_report_stat(true); } @@ -4051,7 +4054,6 @@ PostgresMain(const char *dbname, const char *username) volatile bool send_ready_for_query = true; bool idle_in_transaction_timeout_enabled = false; bool idle_session_timeout_enabled = false; - bool idle_stats_update_timeout_enabled = false; AssertArg(dbname != NULL); AssertArg(username != NULL); @@ -4428,13 +4430,30 @@ PostgresMain(const char *dbname, const char *username) if (notifyInterruptPending) ProcessNotifyInterrupt(false); - /* Start the idle-stats-update timer */ + /* + * Check if we need to report stats. If pgstat_report_stat() + * decides it's too soon to flush out pending stats / lock + * contention prevented reporting, it'll tell us when we + * should try to report stats again (so that stats updates + * aren't unduly delayed if the connection goes idle for a + * long time). We only enable the timeout if we don't already + * have a timeout in progress, because we don't disable the + * timeout below. enable_timeout_after() needs to determine + * the current timestamp, which can have a negative + * performance impact. That's OK because pgstat_report_stat() + * won't have us wake up sooner than a prior call. + */ stats_timeout = pgstat_report_stat(false); if (stats_timeout > 0) { - idle_stats_update_timeout_enabled = true; - enable_timeout_after(IDLE_STATS_UPDATE_TIMEOUT, - stats_timeout); + if (!get_timeout_active(IDLE_STATS_UPDATE_TIMEOUT)) + enable_timeout_after(IDLE_STATS_UPDATE_TIMEOUT, + stats_timeout); + } + else + { + /* all stats flushed, no need for the timeout */ + disable_timeout(IDLE_STATS_UPDATE_TIMEOUT, false); } set_ps_display("idle"); @@ -4470,10 +4489,9 @@ PostgresMain(const char *dbname, const char *username) firstchar = ReadCommand(&input_message); /* - * (4) turn off the idle-in-transaction, idle-session and - * idle-stats-update timeouts if active. We do this before step (5) - * so that any last-moment timeout is certain to be detected in step - * (5). + * (4) turn off the idle-in-transaction and idle-session timeouts if + * active. We do this before step (5) so that any last-moment timeout + * is certain to be detected in step (5). * * At most one of these timeouts will be active, so there's no need to * worry about combining the timeout.c calls into one. @@ -4488,11 +4506,6 @@ PostgresMain(const char *dbname, const char *username) disable_timeout(IDLE_SESSION_TIMEOUT, false); idle_session_timeout_enabled = false; } - if (idle_stats_update_timeout_enabled) - { - disable_timeout(IDLE_STATS_UPDATE_TIMEOUT, false); - idle_stats_update_timeout_enabled = false; - } /* * (5) disable async signal conditions again. diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index ad5cf6fb915..826008be8b2 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -570,7 +570,7 @@ pgstat_report_stat(bool force) bool nowait; pgstat_assert_is_up(); - Assert(!IsTransactionBlock()); + Assert(!IsTransactionOrTransactionBlock()); /* "absorb" the forced flush even if there's nothing to flush */ if (pgStatForceNextFlush) -- 2.35.1.677.gabf474a5dd
>From c757eaaed0f3eb2e78a869f358111396716f81fa Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Thu, 16 Jun 2022 18:28:39 -0700 Subject: [PATCH v2 3/4] WIP: expression eval: fix EEOP_HASHED_SCALARARRAYOP state width. Author: Reviewed-By: Discussion: https://postgr.es/m/20220616233130.rparivafipt6d...@alap3.anarazel.de Backpatch: --- src/include/executor/execExpr.h | 5 ----- src/backend/executor/execExpr.c | 10 ---------- src/backend/executor/execExprInterp.c | 18 ++++++++++++++---- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index e34db8c93cb..2fba1450c65 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -582,12 +582,7 @@ typedef struct ExprEvalStep struct ScalarArrayOpExprHashTable *elements_tab; FmgrInfo *finfo; /* function's lookup data */ FunctionCallInfo fcinfo_data; /* arguments etc */ - /* faster to access without additional indirection: */ - PGFunction fn_addr; /* actual call address */ FmgrInfo *hash_finfo; /* function's lookup data */ - FunctionCallInfo hash_fcinfo_data; /* arguments etc */ - /* faster to access without additional indirection: */ - PGFunction hash_fn_addr; /* actual call address */ } hashedscalararrayop; /* for EEOP_XMLEXPR */ diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 2831e7978b5..4128248be40 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1204,7 +1204,6 @@ ExecInitExprRec(Expr *node, ExprState *state, FunctionCallInfo fcinfo; AclResult aclresult; FmgrInfo *hash_finfo; - FunctionCallInfo hash_fcinfo; Oid cmpfuncid; /* @@ -1263,16 +1262,10 @@ ExecInitExprRec(Expr *node, ExprState *state, if (OidIsValid(opexpr->hashfuncid)) { hash_finfo = palloc0(sizeof(FmgrInfo)); - hash_fcinfo = palloc0(SizeForFunctionCallInfo(1)); fmgr_info(opexpr->hashfuncid, hash_finfo); fmgr_info_set_expr((Node *) node, hash_finfo); - InitFunctionCallInfoData(*hash_fcinfo, hash_finfo, - 1, opexpr->inputcollid, NULL, - NULL); scratch.d.hashedscalararrayop.hash_finfo = hash_finfo; - scratch.d.hashedscalararrayop.hash_fcinfo_data = hash_fcinfo; - scratch.d.hashedscalararrayop.hash_fn_addr = hash_finfo->fn_addr; /* Evaluate scalar directly into left function argument */ ExecInitExprRec(scalararg, state, @@ -1292,11 +1285,8 @@ ExecInitExprRec(Expr *node, ExprState *state, scratch.d.hashedscalararrayop.inclause = opexpr->useOr; scratch.d.hashedscalararrayop.finfo = finfo; scratch.d.hashedscalararrayop.fcinfo_data = fcinfo; - scratch.d.hashedscalararrayop.fn_addr = finfo->fn_addr; scratch.d.hashedscalararrayop.hash_finfo = hash_finfo; - scratch.d.hashedscalararrayop.hash_fcinfo_data = hash_fcinfo; - scratch.d.hashedscalararrayop.hash_fn_addr = hash_finfo->fn_addr; ExprEvalPushStep(state, &scratch); } diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index eaec697bb38..77e1cb1b7b4 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -217,6 +217,7 @@ typedef struct ScalarArrayOpExprHashTable { saophash_hash *hashtab; /* underlying hash table */ struct ExprEvalStep *op; + FunctionCallInfo hash_fcinfo_data; /* arguments etc */ } ScalarArrayOpExprHashTable; /* Define parameters for ScalarArrayOpExpr hash table code generation. */ @@ -3474,13 +3475,13 @@ static uint32 saop_element_hash(struct saophash_hash *tb, Datum key) { ScalarArrayOpExprHashTable *elements_tab = (ScalarArrayOpExprHashTable *) tb->private_data; - FunctionCallInfo fcinfo = elements_tab->op->d.hashedscalararrayop.hash_fcinfo_data; + FunctionCallInfo fcinfo = elements_tab->hash_fcinfo_data; Datum hash; fcinfo->args[0].value = key; fcinfo->args[0].isnull = false; - hash = elements_tab->op->d.hashedscalararrayop.hash_fn_addr(fcinfo); + hash = elements_tab->op->d.hashedscalararrayop.hash_finfo->fn_addr(fcinfo); return DatumGetUInt32(hash); } @@ -3502,7 +3503,7 @@ saop_hash_element_match(struct saophash_hash *tb, Datum key1, Datum key2) fcinfo->args[1].value = key2; fcinfo->args[1].isnull = false; - result = elements_tab->op->d.hashedscalararrayop.fn_addr(fcinfo); + result = elements_tab->op->d.hashedscalararrayop.finfo->fn_addr(fcinfo); return DatumGetBool(result); } @@ -3575,6 +3576,15 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco op->d.hashedscalararrayop.elements_tab = elements_tab; elements_tab->op = op; + elements_tab->hash_fcinfo_data = palloc0(SizeForFunctionCallInfo(1)); + InitFunctionCallInfoData(*elements_tab->hash_fcinfo_data, + op->d.hashedscalararrayop.hash_finfo, + 1, + /* FIXME */ + ((OpExpr*)op->d.hashedscalararrayop.hash_finfo->fn_expr)->inputcollid, + NULL, + NULL); + /* * Create the hash table sizing it according to the number of elements * in the array. This does assume that the array has no duplicates. @@ -3669,7 +3679,7 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco fcinfo->args[1].value = (Datum) 0; fcinfo->args[1].isnull = true; - result = op->d.hashedscalararrayop.fn_addr(fcinfo); + result = op->d.hashedscalararrayop.finfo->fn_addr(fcinfo); resultnull = fcinfo->isnull; /* -- 2.35.1.677.gabf474a5dd
>From 740b70d61f6f48929567dff84fc7ddcebef1551e Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Thu, 16 Jun 2022 18:33:42 -0700 Subject: [PATCH v2 4/4] WIP: expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size. Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/include/executor/execExpr.h | 111 ++++++++++++++------------ src/backend/executor/execExpr.c | 93 +++++++++++---------- src/backend/executor/execExprInterp.c | 104 ++++++++++++------------ 3 files changed, 168 insertions(+), 140 deletions(-) diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index 2fba1450c65..95f05ddaa59 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -22,6 +22,8 @@ struct ExprEvalStep; struct SubscriptingRefState; struct ScalarArrayOpExprHashTable; struct JsonbValue; +struct JsonExprState; +struct JsonConstructorExprState; /* Bits in ExprState->flags (see also execnodes.h for public flag bits): */ /* expression's interpreter has been initialized */ @@ -671,16 +673,7 @@ typedef struct ExprEvalStep /* for EEOP_JSON_CONSTRUCTOR */ struct { - JsonConstructorExpr *constructor; - Datum *arg_values; - bool *arg_nulls; - Oid *arg_types; - struct - { - int category; - Oid outfuncid; - } *arg_type_cache; /* cache for datum_to_json[b]() */ - int nargs; + struct JsonConstructorExprState *jcstate; } json_constructor; /* for EEOP_IS_JSON */ @@ -692,45 +685,7 @@ typedef struct ExprEvalStep /* for EEOP_JSONEXPR */ struct { - JsonExpr *jsexpr; /* original expression node */ - - struct - { - FmgrInfo func; /* 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 - { - 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 */ + struct JsonExprState *jsestate; } jsonexpr; } d; @@ -777,6 +732,64 @@ typedef struct SubscriptExecSteps ExecEvalSubroutine sbs_fetch_old; /* fetch old value for assignment */ } SubscriptExecSteps; +/* EEOP_JSON_CONSTRUCTOR state, too big to inline */ +typedef struct JsonConstructorExprState +{ + JsonConstructorExpr *constructor; + Datum *arg_values; + bool *arg_nulls; + Oid *arg_types; + struct + { + int category; + Oid outfuncid; + } *arg_type_cache; /* cache for datum_to_json[b]() */ + int nargs; +} JsonConstructorExprState; + +/* EEOP_JSONEXPR state, too big to inline */ +typedef struct JsonExprState +{ + JsonExpr *jsexpr; /* original expression node */ + + struct + { + FmgrInfo func; /* 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 + { + 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 */ +} JsonExprState; /* functions in execExpr.c */ extern void ExprEvalPushStep(ExprState *es, const ExprEvalStep *s); diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 4128248be40..ca0b4cefb0f 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -2460,32 +2460,38 @@ ExecInitExprRec(Expr *node, ExprState *state, } else { + JsonConstructorExprState *jcstate; + + jcstate = palloc0(sizeof(JsonConstructorExprState)); + scratch.opcode = EEOP_JSON_CONSTRUCTOR; - scratch.d.json_constructor.constructor = ctor; - scratch.d.json_constructor.arg_values = palloc(sizeof(Datum) * nargs); - scratch.d.json_constructor.arg_nulls = palloc(sizeof(bool) * nargs); - scratch.d.json_constructor.arg_types = palloc(sizeof(Oid) * nargs); - scratch.d.json_constructor.nargs = nargs; + scratch.d.json_constructor.jcstate = jcstate; + + jcstate->constructor = ctor; + jcstate->arg_values = palloc(sizeof(Datum) * nargs); + jcstate->arg_nulls = palloc(sizeof(bool) * nargs); + jcstate->arg_types = palloc(sizeof(Oid) * nargs); + jcstate->nargs = nargs; foreach(lc, args) { Expr *arg = (Expr *) lfirst(lc); - scratch.d.json_constructor.arg_types[argno] = exprType((Node *) arg); + jcstate->arg_types[argno] = exprType((Node *) arg); if (IsA(arg, Const)) { /* Don't evaluate const arguments every round */ Const *con = (Const *) arg; - scratch.d.json_constructor.arg_values[argno] = con->constvalue; - scratch.d.json_constructor.arg_nulls[argno] = con->constisnull; + jcstate->arg_values[argno] = con->constvalue; + jcstate->arg_nulls[argno] = con->constisnull; } else { ExecInitExprRec(arg, state, - &scratch.d.json_constructor.arg_values[argno], - &scratch.d.json_constructor.arg_nulls[argno]); + &jcstate->arg_values[argno], + &jcstate->arg_nulls[argno]); } argno++; } @@ -2496,14 +2502,14 @@ ExecInitExprRec(Expr *node, ExprState *state, bool is_jsonb = ctor->returning->format->format_type == JS_FORMAT_JSONB; - scratch.d.json_constructor.arg_type_cache = - palloc(sizeof(*scratch.d.json_constructor.arg_type_cache) * nargs); + jcstate->arg_type_cache = + palloc(sizeof(*jcstate->arg_type_cache) * nargs); for (int i = 0; i < nargs; i++) { int category; Oid outfuncid; - Oid typid = scratch.d.json_constructor.arg_types[i]; + Oid typid = jcstate->arg_types[i]; if (is_jsonb) { @@ -2522,8 +2528,8 @@ ExecInitExprRec(Expr *node, ExprState *state, category = (int) jscat; } - scratch.d.json_constructor.arg_type_cache[i].outfuncid = outfuncid; - scratch.d.json_constructor.arg_type_cache[i].category = category; + jcstate->arg_type_cache[i].outfuncid = outfuncid; + jcstate->arg_type_cache[i].category = category; } } @@ -2562,41 +2568,44 @@ ExecInitExprRec(Expr *node, ExprState *state, case T_JsonExpr: { JsonExpr *jexpr = castNode(JsonExpr, node); + JsonExprState *jsestate = palloc0(sizeof(JsonExprState)); ListCell *argexprlc; ListCell *argnamelc; scratch.opcode = EEOP_JSONEXPR; - scratch.d.jsonexpr.jsexpr = jexpr; + scratch.d.jsonexpr.jsestate = jsestate; - scratch.d.jsonexpr.formatted_expr = - palloc(sizeof(*scratch.d.jsonexpr.formatted_expr)); + jsestate->jsexpr = jexpr; + + jsestate->formatted_expr = + palloc(sizeof(*jsestate->formatted_expr)); ExecInitExprRec((Expr *) jexpr->formatted_expr, state, - &scratch.d.jsonexpr.formatted_expr->value, - &scratch.d.jsonexpr.formatted_expr->isnull); + &jsestate->formatted_expr->value, + &jsestate->formatted_expr->isnull); - scratch.d.jsonexpr.pathspec = - palloc(sizeof(*scratch.d.jsonexpr.pathspec)); + jsestate->pathspec = + palloc(sizeof(*jsestate->pathspec)); ExecInitExprRec((Expr *) jexpr->path_spec, state, - &scratch.d.jsonexpr.pathspec->value, - &scratch.d.jsonexpr.pathspec->isnull); + &jsestate->pathspec->value, + &jsestate->pathspec->isnull); - scratch.d.jsonexpr.res_expr = - palloc(sizeof(*scratch.d.jsonexpr.res_expr)); + jsestate->res_expr = + palloc(sizeof(*jsestate->res_expr)); - scratch.d.jsonexpr.result_expr = jexpr->result_coercion + jsestate->result_expr = jexpr->result_coercion ? ExecInitExprWithCaseValue((Expr *) jexpr->result_coercion->expr, state->parent, - &scratch.d.jsonexpr.res_expr->value, - &scratch.d.jsonexpr.res_expr->isnull) + &jsestate->res_expr->value, + &jsestate->res_expr->isnull) : NULL; - scratch.d.jsonexpr.default_on_empty = !jexpr->on_empty ? NULL : + jsestate->default_on_empty = !jexpr->on_empty ? NULL : ExecInitExpr((Expr *) jexpr->on_empty->default_expr, state->parent); - scratch.d.jsonexpr.default_on_error = + jsestate->default_on_error = ExecInitExpr((Expr *) jexpr->on_error->default_expr, state->parent); @@ -2607,11 +2616,11 @@ ExecInitExprRec(Expr *node, ExprState *state, /* lookup the result type's input function */ getTypeInputInfo(jexpr->returning->typid, &typinput, - &scratch.d.jsonexpr.input.typioparam); - fmgr_info(typinput, &scratch.d.jsonexpr.input.func); + &jsestate->input.typioparam); + fmgr_info(typinput, &jsestate->input.func); } - scratch.d.jsonexpr.args = NIL; + jsestate->args = NIL; forboth(argexprlc, jexpr->passing_values, argnamelc, jexpr->passing_names) @@ -2630,11 +2639,11 @@ ExecInitExprRec(Expr *node, ExprState *state, var->value = (Datum) 0; var->isnull = true; - scratch.d.jsonexpr.args = - lappend(scratch.d.jsonexpr.args, var); + jsestate->args = + lappend(jsestate->args, var); } - scratch.d.jsonexpr.cache = NULL; + jsestate->cache = NULL; if (jexpr->coercions) { @@ -2643,13 +2652,13 @@ ExecInitExprRec(Expr *node, ExprState *state, Datum *caseval; bool *casenull; - scratch.d.jsonexpr.coercion_expr = - palloc(sizeof(*scratch.d.jsonexpr.coercion_expr)); + jsestate->coercion_expr = + palloc(sizeof(*jsestate->coercion_expr)); - caseval = &scratch.d.jsonexpr.coercion_expr->value; - casenull = &scratch.d.jsonexpr.coercion_expr->isnull; + caseval = &jsestate->coercion_expr->value; + casenull = &jsestate->coercion_expr->isnull; - for (cstate = &scratch.d.jsonexpr.coercions.null, + for (cstate = &jsestate->coercions.null, coercion = &jexpr->coercions->null; coercion <= &jexpr->coercions->composite; coercion++, cstate++) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 77e1cb1b7b4..9913907cec8 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4520,39 +4520,40 @@ ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op, ExprContext *econtext) { Datum res; - JsonConstructorExpr *ctor = op->d.json_constructor.constructor; + JsonConstructorExprState *jcstate = op->d.json_constructor.jcstate; + JsonConstructorExpr *ctor = jcstate->constructor; bool is_jsonb = ctor->returning->format->format_type == JS_FORMAT_JSONB; bool isnull = false; if (ctor->type == JSCTOR_JSON_ARRAY) res = (is_jsonb ? jsonb_build_array_worker : - json_build_array_worker) (op->d.json_constructor.nargs, - op->d.json_constructor.arg_values, - op->d.json_constructor.arg_nulls, - op->d.json_constructor.arg_types, - op->d.json_constructor.constructor->absent_on_null); + json_build_array_worker) (jcstate->nargs, + jcstate->arg_values, + jcstate->arg_nulls, + jcstate->arg_types, + ctor->absent_on_null); else if (ctor->type == JSCTOR_JSON_OBJECT) res = (is_jsonb ? jsonb_build_object_worker : - json_build_object_worker) (op->d.json_constructor.nargs, - op->d.json_constructor.arg_values, - op->d.json_constructor.arg_nulls, - op->d.json_constructor.arg_types, - op->d.json_constructor.constructor->absent_on_null, - op->d.json_constructor.constructor->unique); + json_build_object_worker) (jcstate->nargs, + jcstate->arg_values, + jcstate->arg_nulls, + jcstate->arg_types, + ctor->absent_on_null, + ctor->unique); else if (ctor->type == JSCTOR_JSON_SCALAR) { - if (op->d.json_constructor.arg_nulls[0]) + if (jcstate->arg_nulls[0]) { res = (Datum) 0; isnull = true; } else { - Datum value = op->d.json_constructor.arg_values[0]; - int category = op->d.json_constructor.arg_type_cache[0].category; - Oid outfuncid = op->d.json_constructor.arg_type_cache[0].outfuncid; + Datum value = jcstate->arg_values[0]; + int category = jcstate->arg_type_cache[0].category; + Oid outfuncid = jcstate->arg_type_cache[0].outfuncid; if (is_jsonb) res = to_jsonb_worker(value, category, outfuncid); @@ -4562,14 +4563,14 @@ ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op, } else if (ctor->type == JSCTOR_JSON_PARSE) { - if (op->d.json_constructor.arg_nulls[0]) + if (jcstate->arg_nulls[0]) { res = (Datum) 0; isnull = true; } else { - Datum value = op->d.json_constructor.arg_values[0]; + Datum value = jcstate->arg_values[0]; text *js = DatumGetTextP(value); if (is_jsonb) @@ -4637,14 +4638,17 @@ ExecEvalJsonExprCoercion(ExprEvalStep *op, ExprContext *econtext, Datum res, bool *isNull, void *p, bool *error) { ExprState *estate = p; + JsonExprState *jsestate; if (estate) /* coerce using specified expression */ return ExecEvalExpr(estate, econtext, isNull); - if (op->d.jsonexpr.jsexpr->op != JSON_EXISTS_OP) + jsestate = op->d.jsonexpr.jsestate; + + if (jsestate->jsexpr->op != JSON_EXISTS_OP) { - JsonCoercion *coercion = op->d.jsonexpr.jsexpr->result_coercion; - JsonExpr *jexpr = op->d.jsonexpr.jsexpr; + JsonCoercion *coercion = jsestate->jsexpr->result_coercion; + JsonExpr *jexpr = jsestate->jsexpr; Jsonb *jb = *isNull ? NULL : DatumGetJsonbP(res); if ((coercion && coercion->via_io) || @@ -4654,25 +4658,25 @@ ExecEvalJsonExprCoercion(ExprEvalStep *op, ExprContext *econtext, /* strip quotes and call typinput function */ char *str = *isNull ? NULL : JsonbUnquote(jb); - return InputFunctionCall(&op->d.jsonexpr.input.func, str, - op->d.jsonexpr.input.typioparam, + return InputFunctionCall(&jsestate->input.func, str, + jsestate->input.typioparam, jexpr->returning->typmod); } else if (coercion && coercion->via_populate) return json_populate_type(res, JSONBOID, jexpr->returning->typid, jexpr->returning->typmod, - &op->d.jsonexpr.cache, + &jsestate->cache, econtext->ecxt_per_query_memory, isNull); } - if (op->d.jsonexpr.result_expr) + if (jsestate->result_expr) { - op->d.jsonexpr.res_expr->value = res; - op->d.jsonexpr.res_expr->isnull = *isNull; + jsestate->res_expr->value = res; + jsestate->res_expr->isnull = *isNull; - res = ExecEvalExpr(op->d.jsonexpr.result_expr, econtext, isNull); + res = ExecEvalExpr(jsestate->result_expr, econtext, isNull); } return res; @@ -4905,7 +4909,8 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext, { ExecEvalJsonExprContext *cxt = pcxt; JsonPath *path = cxt->path; - JsonExpr *jexpr = op->d.jsonexpr.jsexpr; + JsonExprState *jsestate = op->d.jsonexpr.jsestate; + JsonExpr *jexpr = jsestate->jsexpr; ExprState *estate = NULL; bool empty = false; Datum res = (Datum) 0; @@ -4914,7 +4919,7 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext, { case JSON_QUERY_OP: res = JsonPathQuery(item, path, jexpr->wrapper, &empty, error, - op->d.jsonexpr.args); + jsestate->args); if (error && *error) { *resnull = true; @@ -4927,7 +4932,7 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext, { struct JsonCoercionState *jcstate; JsonbValue *jbv = JsonPathValue(item, path, &empty, error, - op->d.jsonexpr.args); + jsestate->args); if (error && *error) return (Datum) 0; @@ -4950,8 +4955,8 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext, /* Use coercion from SQL/JSON item type to the output type */ res = ExecPrepareJsonItemCoercion(jbv, - op->d.jsonexpr.jsexpr->returning, - &op->d.jsonexpr.coercions, + jsestate->jsexpr->returning, + &jsestate->coercions, &jcstate); if (jcstate->coercion && @@ -4983,27 +4988,27 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext, /* coerce using specific expression */ estate = jcstate->estate; - op->d.jsonexpr.coercion_expr->value = res; - op->d.jsonexpr.coercion_expr->isnull = *resnull; + jsestate->coercion_expr->value = res; + jsestate->coercion_expr->isnull = *resnull; break; } case JSON_EXISTS_OP: { bool exists = JsonPathExists(item, path, - op->d.jsonexpr.args, + jsestate->args, error); *resnull = error && *error; res = BoolGetDatum(exists); - if (!op->d.jsonexpr.result_expr) + if (!jsestate->result_expr) return res; /* coerce using result expression */ - estate = op->d.jsonexpr.result_expr; - op->d.jsonexpr.res_expr->value = res; - op->d.jsonexpr.res_expr->isnull = *resnull; + estate = jsestate->result_expr; + jsestate->res_expr->value = res; + jsestate->res_expr->isnull = *resnull; break; } @@ -5039,11 +5044,11 @@ ExecEvalJsonExpr(ExprEvalStep *op, ExprContext *econtext, * Execute DEFAULT expression as a coercion expression, because * its result is already coerced to the target type. */ - estate = op->d.jsonexpr.default_on_empty; + estate = jsestate->default_on_empty; else /* Execute ON EMPTY behavior */ res = ExecEvalJsonBehavior(econtext, jexpr->on_empty, - op->d.jsonexpr.default_on_empty, + jsestate->default_on_empty, resnull); } @@ -5076,7 +5081,8 @@ void ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext) { ExecEvalJsonExprContext cxt; - JsonExpr *jexpr = op->d.jsonexpr.jsexpr; + JsonExprState *jsestate = op->d.jsonexpr.jsestate; + JsonExpr *jexpr = jsestate->jsexpr; Datum item; Datum res = (Datum) 0; JsonPath *path; @@ -5088,7 +5094,7 @@ ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext) *op->resnull = true; /* until we get a result */ *op->resvalue = (Datum) 0; - if (op->d.jsonexpr.formatted_expr->isnull || op->d.jsonexpr.pathspec->isnull) + if (jsestate->formatted_expr->isnull || jsestate->pathspec->isnull) { /* execute domain checks for NULLs */ (void) ExecEvalJsonExprCoercion(op, econtext, res, op->resnull, @@ -5098,11 +5104,11 @@ ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext) return; } - item = op->d.jsonexpr.formatted_expr->value; - path = DatumGetJsonPathP(op->d.jsonexpr.pathspec->value); + item = jsestate->formatted_expr->value; + path = DatumGetJsonPathP(jsestate->pathspec->value); /* reset JSON path variable contexts */ - foreach(lc, op->d.jsonexpr.args) + foreach(lc, jsestate->args) { JsonPathVariableEvalContext *var = lfirst(lc); @@ -5110,7 +5116,7 @@ ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext) var->evaluated = false; } - needSubtrans = ExecEvalJsonNeedsSubTransaction(jexpr, &op->d.jsonexpr.coercions); + needSubtrans = ExecEvalJsonNeedsSubTransaction(jexpr, &jsestate->coercions); cxt.path = path; cxt.error = throwErrors ? NULL : &error; @@ -5125,7 +5131,7 @@ ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext) { /* Execute ON ERROR behavior */ res = ExecEvalJsonBehavior(econtext, jexpr->on_error, - op->d.jsonexpr.default_on_error, + jsestate->default_on_error, op->resnull); /* result is already coerced in DEFAULT behavior case */ -- 2.35.1.677.gabf474a5dd
