On 2022-06-23 Th 21:51, Andres Freund wrote:
> Hi,
>
> On 2022-06-23 16:38:12 +0900, Michael Paquier wrote:
>> On Tue, Jun 21, 2022 at 05:41:07PM -0400, Andrew Dunstan wrote:
>>> On 2022-06-21 Tu 17:25, Andres Freund wrote:
>>>> On 2022-06-21 17:11:33 -0400, Andrew Dunstan wrote:
>>>>> I and a couple of colleagues have looked it over. As far as it goes the
>>>>> json fix looks kosher to me. I'll play with it some more.
>>>> Cool.
>>>>
>>>> Any chance you could look at fixing the "structure" of the generated
>>>> expression "program". The recursive ExecEvalExpr() calls are really not
>>>> ok...
>> By how much does the size of ExprEvalStep go down once you don't
>> inline the JSON structures as of 0004 in [1]? And what of 0003?
> 0004 gets us back to 64 bytes, if 0003 is applied first. 0003 alone doesn't
> yield a size reduction, because obviously 0004 is the bigger problem. Applying
> just 0004 you end up with 88 bytes.
>
>
>> The JSON portions seem like the largest portion of the cake, though both are
>> must-fixes.
> Yep.
>
>
>>> Yes, but I don't guarantee to have a fix in time for Beta2.
>> IMHO, it would be nice to get something done for beta2. Now the
>> thread is rather fresh and I guess that more performance study is
>> required even for 0004, so..
> I don't think there's a whole lot of performance study needed for 0004 - the
> current state is obviously wrong.
>
> 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.
>
>
>> Waiting for beta3 would a better move at this stage. Is somebody confident
>> enough in the patches proposed?
> 0001 is the one that needs to most careful analysis, I think. 0002 I'd be fine
> with pushing after reviewing it again. For 0003 David's approach might be
> better or worse, it doesn't matter much I think. 0004 is ok I think, perhaps
> with the exception of quibbling over some naming decisions?
>
>
The attached very small patch applies on top of your 0002 and deals with
the FmgrInfo complaint.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
From ed2118c63e298e5e5872db5003fa342c33aa7505 Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <and...@dunslane.net>
Date: Wed, 22 Jun 2022 12:00:47 -0400
Subject: [PATCH 2/2] 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 1f65daf203..d8608a3c58 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2623,11 +2623,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 80c5e04dd5..cbdcbbce53 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4648,7 +4648,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 15532813ae..c0ba8998f2 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -759,7 +759,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
