On Fri, Mar 25, 2022 at 1:30 PM Andrew Dunstan <and...@dunslane.net> wrote:
> > On 3/22/22 10:55, Daniel Gustafsson wrote: > >> On 22 Mar 2022, at 16:31, Andrew Dunstan <and...@dunslane.net> wrote: > >> I'm planning on pushing the functions patch set this week and json-table > >> next week. > > My comments from 30827b3c-edf6-4d41-bbf1-298181874...@yesql.se are yet > to be > > addressed (or at all responded to) in this patchset. I'll paste the > ones which > > still apply to make it easier: > > > > > I think I have fixed all those. See attached. I haven't prepared a new > patch set for SQL/JSON functions because there's just one typo to fix, > but I won't forget it. Please let me know if there's anything else you see. > > > At this stage I think I have finished with the actual code, and I'm > concentrating on improving the docs a bit. > > > cheers > > > andrew > > > -- > Andrew Dunstan > EDB: https://www.enterprisedb.com Hi, w.r.t. 0001-SQL-JSON-functions-without-sql_json-GUC-v59.patch : + Datum *innermost_caseval = state->innermost_caseval; + bool *innermost_isnull = state->innermost_casenull; + + state->innermost_caseval = resv; + state->innermost_casenull = resnull; + + ExecInitExprRec(jve->formatted_expr, state, resv, resnull); + + state->innermost_caseval = innermost_caseval; + state->innermost_casenull = innermost_isnull; Code similar to the above block appears at least twice. Maybe extract into a helper func to reuse code. + * Evaluate a JSON path variable caching computed value. + */ +int +EvalJsonPathVar(void *cxt, char *varName, int varNameLen, Please add description for return value in the comment. + if (formatted && IsA(formatted, Const)) + return formatted; If formatted is NULL, it cannot be Const. So the if can be simplified: + if (IsA(formatted, Const)) For transformJsonConstructorOutput(), it seems the variable have_json is not used - you can drop the variable. + * Coerce a expression in JSON DEFAULT behavior to the target output type. a expression -> an expression Cheers
