MIME-Version: 1.0
References: 
 <CA+TgmoaiothgQrw9OtgsMzBUCnqJ2jdaGTbS6o3fkjCd+LfzWw@mail.gmail.com>
In-Reply-To: 
 <CA+TgmoaiothgQrw9OtgsMzBUCnqJ2jdaGTbS6o3fkjCd+LfzWw@mail.gmail.com>
From: Andrew Dunstan <andrew@dunslane.net>
Date: Wed, 8 Jul 2026 11:16:07 -0400
Message-ID: 
 <CAD5tBc+LK2V6m-BDDD2KUCn3y9zLhO6FsY5VFTRS-+4L5tht3A@mail.gmail.com>
Subject: Re: json/jsonb cleanup + FmgrInfo caching
To: Robert Haas <robertmhaas@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="00000000000048360406561afd45"
Archived-At: 
 <https://www.postgresql.org/message-id/CAD5tBc%2BLK2V6m-BDDD2KUCn3y9zLhO6FsY5VFTRS-%2B4L5tht3A%40mail.gmail.com>
Precedence: bulk

--00000000000048360406561afd45
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Jul 2, 2026 at 12:25=E2=80=AFPM Robert Haas <robertmhaas@gmail.com>=
 wrote:

> Hi,
>
> While working on my "sandboxing untrusted code" project, I found
> myself investigating how the JSON and JSONB code calls type output and
> cast functions. I feel like this needs some cleanup in order to avoid
> blocking that project, and it turns out that there are also
> significant opportunities to improve performance, so here are some
> patches. One caveat: one of these patches causes a small backward
> compatibility break, because the current behavior is wrong. See below
> for full details.
>
> First, a quick performance demonstration:
>
> CREATE TABLE hstores AS SELECT hstore('k', g::text) x FROM
> generate_series(1,4000000) g;
> SELECT any_value(json_build_object('a', x)) FROM hstores;
>
> Unpatched, median-of-three runs was 398.571 ms. Patched, 193.714 ms.
> That's a 2.05x speedup, which is the highest I observed in my test
> queries. What I found is that this technique shows the largest gains
> with user-defined types like hstore, smaller gains with built-in types
> like int, and the smallest gains of all with container types such as a
> user-defined record type. Generally, jsonb benefited more than json.
> The aggregates - json_agg and jsonb_agg - showed very little benefit
> or even small regressions, but I'm pretty confident that the
> regressions are simply noise that goes away with a sufficiently large
> number of sufficiently-careful test runs. Everything else gets faster,
> often by 50%+. Leaving aside the aggregates which are expected to show
> little or no benefit, here's one of the less-sympathetic cases:
>
> CREATE TABLE ints AS SELECT x FROM generate_series(1,4000000) x;
> SELECT any_value(to_json(x)) FROM ints;
>
> The speedup is smaller here because it's json rather than jsonb and
> because it's int rather than hstore, but it's still 117.461 ms
> unpatched vs. 89.895 patched, a 30% speedup.
>
> OK, now let's go through the patches:
>
> 0001 refactors the json_categorize_type() function to initialize an
> FmgrInfo instead of returning a base function OID. All of the built-in
> JSON aggregates are updated to cache this FmgrInfo across calls, but
> it really doesn't save much. In the process of working on this
> refactoring it came to light that the current behavior of
> json_check_mutability() is incorrect: it erroneously treats record,
> anyarray, and anycompatiblearray as immutable when in fact they should
> be treated as stable. This refactoring preserves that incorrect
> behavior.
>
> 0002 fixes the bug discovered during the development of 0001 by
> removing the special case. AFAICT, the anyarray and anycompatiblearray
> cases are unreachable, but the record case is reachable, and the
> included test case shows how this could hypothetically matter. It
> seems unlikely we'll inconvenience any significant number of users by
> changing this, but in theory somebody's upgrade could fail.
>
> 0003 moves some code around to avoid problems with circular header
> dependencies, creating new files jsontypes.c/h.
>
> 0004 refactors the SQL-level JSON constructors -- JSON_OBJECT,
> JSON_ARRAY, and JSON_SCALAR -- to make use of the new type caching
> infrastructure.
>
> 0005 refactors the SQL-callable functions similarly. This means
> to_json(b), json(b)_build_object, and json(b)_build_array.
>
>
>

Looks pretty good.

The old `add_json()`/`add_jsonb()` helpers (removed in 0004/0005) checked
`val_type =3D=3D InvalidOid` and raised a clean
`ERRCODE_INVALID_PARAMETER_VALUE` / "could not determine input data type"
error. Now I think we'd get something like "cache lookup failed for type
0", which is rather more opaque. Not sure if that matters.

For VARIADIC arrays,

   for (int i =3D 0; i < nargs; ++i)
       fmgr_info_copy(&(*outflinfos)[i], &jcache->flinfos[0],
CurrentMemoryContext);

is going to copy the same flinfo for every array element for every row, The
comment says that it's done that way to avoid complicating the code, and
that seems reasonable. I don't know how often these functions are used with
explicit VARIADIC array arguments, so it might be a niche case.

cheers

andrew

--00000000000048360406561afd45
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, Jul 2, =
2026 at 12:25=E2=80=AFPM Robert Haas &lt;<a href=3D"mailto:robertmhaas@gmai=
l.com">robertmhaas@gmail.com</a>&gt; wrote:<br></div><blockquote class=3D"g=
mail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204=
,204,204);padding-left:1ex">Hi,<br>
<br>
While working on my &quot;sandboxing untrusted code&quot; project, I found<=
br>
myself investigating how the JSON and JSONB code calls type output and<br>
cast functions. I feel like this needs some cleanup in order to avoid<br>
blocking that project, and it turns out that there are also<br>
significant opportunities to improve performance, so here are some<br>
patches. One caveat: one of these patches causes a small backward<br>
compatibility break, because the current behavior is wrong. See below<br>
for full details.<br>
<br>
First, a quick performance demonstration:<br>
<br>
CREATE TABLE hstores AS SELECT hstore(&#39;k&#39;, g::text) x FROM<br>
generate_series(1,4000000) g;<br>
SELECT any_value(json_build_object(&#39;a&#39;, x)) FROM hstores;<br>
<br>
Unpatched, median-of-three runs was 398.571 ms. Patched, 193.714 ms.<br>
That&#39;s a 2.05x speedup, which is the highest I observed in my test<br>
queries. What I found is that this technique shows the largest gains<br>
with user-defined types like hstore, smaller gains with built-in types<br>
like int, and the smallest gains of all with container types such as a<br>
user-defined record type. Generally, jsonb benefited more than json.<br>
The aggregates - json_agg and jsonb_agg - showed very little benefit<br>
or even small regressions, but I&#39;m pretty confident that the<br>
regressions are simply noise that goes away with a sufficiently large<br>
number of sufficiently-careful test runs. Everything else gets faster,<br>
often by 50%+. Leaving aside the aggregates which are expected to show<br>
little or no benefit, here&#39;s one of the less-sympathetic cases:<br>
<br>
CREATE TABLE ints AS SELECT x FROM generate_series(1,4000000) x;<br>
SELECT any_value(to_json(x)) FROM ints;<br>
<br>
The speedup is smaller here because it&#39;s json rather than jsonb and<br>
because it&#39;s int rather than hstore, but it&#39;s still 117.461 ms<br>
unpatched vs. 89.895 patched, a 30% speedup.<br>
<br>
OK, now let&#39;s go through the patches:<br>
<br>
0001 refactors the json_categorize_type() function to initialize an<br>
FmgrInfo instead of returning a base function OID. All of the built-in<br>
JSON aggregates are updated to cache this FmgrInfo across calls, but<br>
it really doesn&#39;t save much. In the process of working on this<br>
refactoring it came to light that the current behavior of<br>
json_check_mutability() is incorrect: it erroneously treats record,<br>
anyarray, and anycompatiblearray as immutable when in fact they should<br>
be treated as stable. This refactoring preserves that incorrect<br>
behavior.<br>
<br>
0002 fixes the bug discovered during the development of 0001 by<br>
removing the special case. AFAICT, the anyarray and anycompatiblearray<br>
cases are unreachable, but the record case is reachable, and the<br>
included test case shows how this could hypothetically matter. It<br>
seems unlikely we&#39;ll inconvenience any significant number of users by<b=
r>
changing this, but in theory somebody&#39;s upgrade could fail.<br>
<br>
0003 moves some code around to avoid problems with circular header<br>
dependencies, creating new files jsontypes.c/h.<br>
<br>
0004 refactors the SQL-level JSON constructors -- JSON_OBJECT,<br>
JSON_ARRAY, and JSON_SCALAR -- to make use of the new type caching<br>
infrastructure.<br>
<br>
0005 refactors the SQL-callable functions similarly. This means<br>
to_json(b), json(b)_build_object, and json(b)_build_array.<br>
<br><br></blockquote><div><br></div><div><br></div><div>Looks pretty good.=
=C2=A0</div><div><br></div><div>The old `add_json()`/`add_jsonb()` helpers =
(removed in 0004/0005) checked `val_type =3D=3D InvalidOid` and raised a cl=
ean `ERRCODE_INVALID_PARAMETER_VALUE` / &quot;could not determine input dat=
a=C2=A0type&quot; error. Now I think we&#39;d get something like=C2=A0&quot=
;cache lookup failed for type 0&quot;, which is rather more opaque. Not sur=
e if that matters.</div><div><br></div><div>For VARIADIC arrays,</div><div>=
<br></div><div>=C2=A0 =C2=A0for (int i =3D 0; i &lt; nargs; ++i)<br>=C2=A0 =
=C2=A0 =C2=A0 =C2=A0fmgr_info_copy(&amp;(*outflinfos)[i], &amp;jcache-&gt;f=
linfos[0], CurrentMemoryContext);<br><br></div><div>is going to copy the sa=
me flinfo for every array element for every row, The comment says that it&#=
39;s done that way to avoid complicating the code, and that seems reasonabl=
e. I don&#39;t know how often these functions are used with explicit VARIAD=
IC array arguments, so it might be a niche case.</div><div><br></div><div>c=
heers</div><div><br></div><div>andrew</div></div></div>

--00000000000048360406561afd45--