Re: [PATCH]Fix planner's strictness check for JsonExpr

Hi Ethan,

On Wed, Nov 4, 2026 at 11:05 PM Ethan Tran <ethantran(at)quwin(dot)dev> wrote:
> Result coercion still runs after that shortcut. A RETURNING domain with
> NOT NULL can reject NULL and invoke EMPTY ARRAY ON ERROR, producing [].

You're right, thanks for catching this. My description in v2 was
wrong: the NULL produced for a NULL context item or path specification
doesn't go straight to the end, it falls through into the result
coercion step, and when RETURNING a domain the ON ERROR steps are
always emitted. So a NOT NULL domain turns the NULL into a soft error,
and a non-NULL ON ERROR behavior then replaces it. I can reproduce
both the inlining and the outer-join pull-up cases. With the attached
v3, all four queries in your domain-reproducer.sql return NULL.

> Should we preserve unconstrained-domain inlining, with cached plans
> rebuilt if constraints are added to the domain or its parents?

There is a precedent for that: eval_const_expressions() replaces a
CoerceToDomain with a RelabelType when the domain has no constraints,
and calls record_plan_type_dependency() so the plan is rebuilt if
constraints are added later. But contain_nonstrict_functions() has no
PlannerInfo to record such a dependency, so we'd need to change its
signature or add a variant, and pass the root through its callers.
That doesn't seem appropriate for a fix that is going to be
back-patched to v17.

Rather than adding a domain-specific condition on top of v2, v3
simplifies the rule. With a NULL context item or path specification,
a non-NULL result can only come from two places: the jsonpath
evaluation, which still runs if there are PASSING arguments, and the ON
EMPTY / ON ERROR behaviors. So v3 treats a JsonExpr as strict only if
it has no PASSING arguments and both behaviors are NULL or ERROR, and
otherwise as non-strict. It no longer checks whether the behavior
expression is a Const, or whether the RETURNING type is a domain. When
the rule passes, the walker just recurses into the inputs as usual,
since there are no PASSING arguments left to be mistaken for strict
inputs.

With ERROR ON ERROR a domain violation is raised as an error, and with
NULL ON ERROR the result is either NULL or the "could not coerce ON
ERROR expression" error. Neither is a wrong non-NULL result, and
raising an error for a NULL input is what we already accept for
CoerceToDomain, which is treated as strict.

The common case you measured, a STRICT SQL wrapper around JSON_VALUE()
or JSON_QUERY() with the default behaviors, is still inlined, including
when RETURNING a domain. What we give up compared to v2 are wrappers
with an explicit non-NULL behavior (DEFAULT <const>, EMPTY ARRAY, etc.),
and JSON_EXISTS() wrappers, since JSON_EXISTS defaults to FALSE ON
ERROR. Those are no longer inlined, unless they say ERROR ON ERROR. I
think that's an acceptable price for a rule that is easy to verify and
back-patch. If it matters, the more precise check with a plan
dependency could be done separately on master.

Could you check which ON ERROR behavior your 35 ms -> 196 ms benchmark
used? If it used the default, v3 should keep it inlined.

Greg, since v3 changes the rule again, another look would be welcome if
you have time.

Regards,
Zhenglong Li