Re: [PATCH] Fix JSON_SERIALIZE() coercion placeholder type for jsonb input

Hi,

I tested the v3 patch for the JSON_SERIALIZE() issue with jsonb input.
Before applying the patch, I was able to reproduce the issue, where
UTF-8 conversion failed due to invalid byte sequences. After applying
the patch, the output was correct, and the UTF-8 conversion succeeded.
I also ran the regression tests, and all 245 tests passed. The build
completed successfully, and git diff --check reported no issues.
Please let me know if any further testing is needed.

Regards,
solai

On Mon, Oct 26, 2026 at 5:32 PM Nikhil Sontakke <nikkhils(at)gmail(dot)com> wrote:
>
> Thanks Matt,
>
> It all LGTM now. I think this patch can now be marked as "Ready For Committer".
>
> Regards,
> Nikhil
>
> On Thu, Aug 27, 2026 at 6:50 PM Matt Blewitt <mble(at)planetscale(dot)com> wrote:
>>
>> Hi Nikhil,
>>
>> Thanks for the follow-up review. Attached is v3 with the stricter argument assertion, explicit Node * cast, and implicit typmod-coercion test. I also rebased it onto current master and ran the full core regression suite with assertions enabled; all 243 tests passed.
>>
>> Cheers,
>>
>> Matt
>>
>> On Thu, Aug 27, 2026 at 10:45 AM Nikhil Sontakke <nikkhils(at)gmail(dot)com> wrote:
>>>
>>> Hi Matt,
>>>
>>> Thanks for updating the patch. The simplified tests now cover the default text and bytea coercion paths,
>>> and the patch passes the full regression suite. Maybe the code can be tightened further like the example below:
>>>
>>> @@ -3743,8 +3742,8 @@ makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type,
>>>
>>> if (type == JSCTOR_JSON_SERIALIZE)
>>> {
>>> - Assert(args != NIL);
>>> - cte->typeId = exprType(linitial(args));
>>> + Assert(list_length(args) == 1);
>>> + cte->typeId = exprType((Node *) linitial(args));
>>> }
>>> else
>>>
>>> Also, I think, we can expand the tests to consider the implicit typmod conversion case as well:
>>>
>>> +-- Test implicit typmod coercion with jsonb input
>>> +SELECT JSON_SERIALIZE('{ "a" : 1 } '::jsonb RETURNING varchar(2));
>>>
>>>
>>> If no backpatch is intended (which I think is the case here),
>>> then any concerns about previously stored expression trees does not apply for this patch.
>>>
>>> Regards,
>>> Nikhil
>>>
>>> On Wed, Aug 26, 2026 at 5:44 PM Matt Blewitt <mble(at)planetscale(dot)com> wrote:
>>>>
>>>> Hi Nikhil,
>>>>
>>>> Thanks - I've attached v2 with the revised test cases; there are no functional changes from v1.
>>>>
>>>> Cheers,
>>>>
>>>> Matt
>>>>
>>>> On Wed, Aug 26, 2026 at 9:07 AM Nikhil Sontakke <nikkhils(at)gmail(dot)com> wrote:
>>>>>
>>>>> Hi Matt,
>>>>>
>>>>> I reviewed the patch and tested it against current master.
>>>>>
>>>>> I built the patch with assertions enabled and ran the full core regression suite. All 245 tests passed. I also tested:
>>>>>
>>>>> typed jsonb parameters in prepared statements
>>>>> default text, varchar, bytea, and string-domain results
>>>>> NULL jsonb input
>>>>> use inside a view and subsequent deparsing
>>>>>
>>>>> These all behaved as expected. I did not find any security, WAL/recovery, logical-replication, or performance concerns introduced by the patch.
>>>>>
>>>>> I have one comment:
>>>>>
>>>>> Regression tests
>>>>> To me, some of the additional coverage seems redundant:
>>>>>
>>>>> The RETURNING int and RETURNING jsonb cases are rejected before reaching the changed code.
>>>>> The RETURNING jsonb error is already tested immediately above with a non-jsonb input.
>>>>> The new EXPLAIN cases verify deparsing but do not reveal which coercion function was selected.
>>>>>
>>>>> So, I think the test addition could be reduced to one default text case and one RETURNING bytea case.
>>>>>
>>>>> Otherwise, the implementation looks correct. It's simpler than converting every jsonb input to json every time.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Nikhil
>>>>>
>>>>>
>>>>> On Tue, Mar 17, 2026 at 12:18 AM Matt Blewitt <mble(at)planetscale(dot)com> wrote:
>>>>>>
>>>>>> Hi Zsolt,
>>>>>>
>>>>>> Thanks for testing that out and confirming it looks good against master. Submitted to commitfest for further review and integration consideration.
>>>>>>
>>>>>> Matt
>>>>>>
>>>>>> On Fri, Mar 13, 2026 at 11:20 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>>>>>>>
>>>>>>> Hello!
>>>>>>>
>>>>>>> This is a simple fix and it does what it says, it looks good to me.
>>>>>>>
>>>>>>> I did test it with a few more queries and compared it against master,
>>>>>>> all looks good.