Re: PATCH: jsonpath string methods: lower, upper, initcap, l/r/btrim, replace, split_part

On Mon, Dec 1, 2025 at 5:16 AM David E. Wheeler <david(at)justatheory(dot)com> wrote:
>
> On Nov 28, 2025, at 05:29, jian he <jian(dot)universality(at)gmail(dot)com> 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