Thread: pgsql: Don't include CaseTestExpr in JsonValueExpr.formatted_expr
Don't include CaseTestExpr in JsonValueExpr.formatted_expr A CaseTestExpr is currently being put into JsonValueExpr.formatted_expr as placeholder for the result of evaluating JsonValueExpr.raw_expr, which in turn is evaluated separately. Though, there's no need for this indirection if raw_expr itself can be embedded into formatted_expr and evaluated as part of evaluating the latter, especially as there is no special reason to evaluate it separately. So this commit makes it so. As a result, JsonValueExpr.raw_expr no longer needs to be evaluated in ExecInterpExpr(), eval_const_exprs_mutator() etc. and is now only used for displaying the original "unformatted" expression in ruleutils.c. While at it, this also removes the function makeCaseTestExpr(), because the code in makeJsonConstructorExpr() looks more readable without it IMO and isn't used by anyone else either. Finally, a note is added in the comment above CaseTestExpr's definition that JsonConstructorExpr is also using it. Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/b6e1157e7d339c4e20d68448125a4cef42b1ac9d Modified Files -------------- src/backend/executor/execExpr.c | 17 ++------------ src/backend/nodes/makefuncs.c | 4 ++++ src/backend/optimizer/util/clauses.c | 23 +++++-------------- src/backend/parser/parse_expr.c | 43 ++++++++++++++++++------------------ src/include/nodes/primnodes.h | 7 +++++- 5 files changed, 39 insertions(+), 55 deletions(-)
Hi Amit, On Thu, Jul 13, 2023 at 03:33:07AM +0000, Amit Langote wrote: > Don't include CaseTestExpr in JsonValueExpr.formatted_expr > > A CaseTestExpr is currently being put into > JsonValueExpr.formatted_expr as placeholder for the result of > evaluating JsonValueExpr.raw_expr, which in turn is evaluated > separately. Though, there's no need for this indirection if > raw_expr itself can be embedded into formatted_expr and evaluated > as part of evaluating the latter, especially as there is no > special reason to evaluate it separately. So this commit makes it > so. As a result, JsonValueExpr.raw_expr no longer needs to be > evaluated in ExecInterpExpr(), eval_const_exprs_mutator() etc. and > is now only used for displaying the original "unformatted" > expression in ruleutils.c. koel is unhappy after this commit: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=koel&dt=2023-07-13%2006%3A49%3A03 This code is not indented properly. -- Michael
Attachment
On Thu, Jul 13, 2023 at 5:12 PM Michael Paquier <michael@paquier.xyz> wrote: > > Hi Amit, > > On Thu, Jul 13, 2023 at 03:33:07AM +0000, Amit Langote wrote: > > Don't include CaseTestExpr in JsonValueExpr.formatted_expr > > > > A CaseTestExpr is currently being put into > > JsonValueExpr.formatted_expr as placeholder for the result of > > evaluating JsonValueExpr.raw_expr, which in turn is evaluated > > separately. Though, there's no need for this indirection if > > raw_expr itself can be embedded into formatted_expr and evaluated > > as part of evaluating the latter, especially as there is no > > special reason to evaluate it separately. So this commit makes it > > so. As a result, JsonValueExpr.raw_expr no longer needs to be > > evaluated in ExecInterpExpr(), eval_const_exprs_mutator() etc. and > > is now only used for displaying the original "unformatted" > > expression in ruleutils.c. > > koel is unhappy after this commit: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=koel&dt=2023-07-13%2006%3A49%3A03 > This code is not indented properly. Andrew just mentioned that to me off-list too. I've just pushed a fix. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com