Subject: json/jsonb cleanup + FmgrInfo caching

From: Robert Haas <[email-masked]>
Date: Thu, 2 Jul 2026 12:25:23 -0400

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.

Thanks,

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Andrew Dunstan <[email-masked]>
Date: Wed, 8 Jul 2026 11:16:07 -0400

> 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 == 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 = 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

---

From: Robert Haas <[email-masked]>
Date: Thu, 9 Jul 2026 11:15:46 -0400

> Looks pretty good.

Thanks for looking.

> The old `add_json()`/`add_jsonb()` helpers (removed in 0004/0005) checked `val_type == 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.

I definitely don't want to show "cache lookup failed" errors, but I
don't think that's reachable. If I'm wrong and it is, we should add a
guard. Do you have a test case, by any chance?

>        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.

Right. I think it is a niche case, but I also don't think this is
really costing us anything. As far as I can tell from my benchmarking
to date, fmgr_info() is kind of expensive, but fmgr_info_copy() is
pretty cheap. So, we could arrange to share a single flinfo for every
argument of the VARIADIC list, but the savings are pretty limited
because we wouldn't save fmgr_info() calls, only fmgr_info_copy()
calls. And on the downside json{,b}_build_{array,object}_worker would
all need to support two modes, one where there is an fcinfo per Datum
and a second where there is one fcinfo for all Datums. There's a
code-complexity argument against that, but it would also have some
cost in CPU cycles, because every call would have to branch into one
path or the other. We'd be slowing down both variadic and non-variadic
calls slightly by adding those branches, in the hopes that saving
fmgr_info_copy() calls in the non-variadic case would be valuable
enough to make it worthwhile. Somebody can certainly try that, but my
guess is that there won't be any performance benefit worth caring
about and the code will be uglier.

---

From: Tom Lane <[email-masked]>
Date: Thu, 09 Jul 2026 11:22:12 -0400

Robert Haas <[email-masked]> writes:
> calls.

The hole in that argument is that fmgr_info_copy() does

	dstinfo->fn_extra = NULL;

so that if the function-to-be-called caches anything in fn_extra,
it will have to populate that cache for each argument.  That could be
quite expensive.  It might still not justify complicating the setup
code, but I don't think it's as clear-cut as you suggest.  Perhaps
some benchmarking is in order.

			regards, tom lane