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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> >> 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. >>> >>
v2-0001-Fix-JSON_SERIALIZE-coercion-placeholder-type-for-.patch
Description: Binary data
