Hi Ethan, > Yes, the benchmark used default NULL ON EMPTY/ON ERROR. > > On my rerun, v3 now keeps it inlined without any performance issues.
Thanks for rerunning it.
> Would UNKNOWN ON ERROR be worth including, since it is another NULL
> fallback?
Yes, it's the same as NULL ON ERROR, and including it is a one-line
change, so v4 does that. It only applies to JSON_EXISTS, which has no
RETURNING clause, so the domain case doesn't come up. v4 adds a test
that a STRICT SQL function using JSON_EXISTS(... UNKNOWN ON ERROR) is
inlined.
> Constant NULL defaults also seem to fit the NULL-or-error rule,
> but these could be left for a follow-up.
Agreed, I'd leave those for a follow-up. DEFAULT NULL does the same
as NULL, so there's little reason to write it, and I'd rather keep the
back-patched rule to checking the behavior types only.
v4 is also rebased over 627c4df3b73 ("SQL/JSON: Fix NULL result leakage
in JSON_VALUE() RETURNING json/jsonb"), which added regression tests at
the same spot in sqljson_queryfuncs.sql; cfbot was reporting that v3 no
longer applied.
Regards,
Zhenglong Li
v4-0001-Fix-planner-s-strictness-check-for-JsonExpr.patch
Description: Binary data
