On Mon, Dec 1, 2025 at 5:16 AM David E. Wheeler <[email protected]> wrote:
>
> On Nov 28, 2025, at 05:29, jian he <[email protected]> wrote:
>
> > some "switch" in the attached patch does not preserve the JsonPathItemType
> > order
> > consistency, like executeItemOptUnwrapTarget.
>
> Well-spotted, thank you! Fixed in v15, attached.
>
hi.
seems no deparse regress tests, like:
create view vj as select jsonb_path_query('" hello "', '$.ltrim(" ")') as a;
\sv vj
that mean the changes in printJsonPathItem are not tested?
+ /* Create the appropriate jb value to return */
+ switch (jsp->type)
+ {
+ /* Cases for functions that return text */
+ case jpiStrReplace:
comments indentation should align with the word "case"?
executeStringInternalMethod:
+ tmp = pnstrdup(jb->val.string.val, jb->val.string.len);
+ str = CStringGetTextDatum(tmp);
+
+ /* Internal string functions that accept no arguments */
+ switch (jsp->type)
+ {
+ case jpiStrReplace:
+ {
+ char *from_str,
+ *to_str;
+ int from_len,
+ to_len;
+
+ jspGetLeftArg(jsp, &elem);
+ if (elem.type != jpiString)
+ elog(ERROR, "invalid jsonpath item type for .replace() from");
+
+ from_str = jspGetString(&elem, &from_len);
+
+ jspGetRightArg(jsp, &elem);
+ if (elem.type != jpiString)
+ elog(ERROR, "invalid jsonpath item type for .replace() to");
+
+ to_str = jspGetString(&elem, &to_len);
+
+ resStr = TextDatumGetCString(DirectFunctionCall3Coll(replace_text,
+ DEFAULT_COLLATION_OID,
+ CStringGetTextDatum(tmp),
+ CStringGetTextDatum(from_str),
+ CStringGetTextDatum(to_str)));
+
pnstrdup, CStringGetTextDatum copied twice for the same contend?
I think you can just
``
text *tmp = cstring_to_text_with_len(jb->val.string.val, jb->val.string.len);
Datum str = PointerGetDatum(tmp)
``
In the first main switch block, there's no need to call
``CStringGetTextDatum(tmp)``
because str is already a Datum. We can simply use str directly.
I noticed that almost all of them use DEFAULT_COLLATION_OID,
but jpiStrSplitPart uses C_COLLATION_OID.
--
jian
https://www.enterprisedb.com
