On 3/22/22 10:55, Daniel Gustafsson wrote:
>> On 22 Mar 2022, at 16:31, Andrew Dunstan <andrew@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-2981818744A8@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
