Re: BUG #19851: Adding a tautological CHECK(true) to a domain changes JSON_VALUE's result (silent NULL, value/type c

PG Bug reporting form <noreply(at)postgresql(dot)org> writes:
> Two domains with the *identical value set* — one declared without
> constraints, one with only a tautological `CHECK (true)` that can never
> reject anything — give *different results* for the same `JSON_VALUE`
> expression. `CHECK(true)` is a no-op semantically, so the two results must
> agree, and both must agree with the base type.

Yeah, this is a mess. The entire design of JsonExpr in this area
is a disaster, starting here:

/*
* Information about converting the result of jsonpath functions
* JsonPathQuery() and JsonPathValue() to the RETURNING type.
*/
bool use_io_coercion;
bool use_json_coercion;

Possibly, if these flags hadn't been so utterly undocumented,
transformJsonFuncExpr wouldn't be so confused about what it needs
to do with domains. But there are worse problems. The broken
logic is in parse_expr.c:

if (get_typtype(jsexpr->returning->typid) == TYPTYPE_DOMAIN &&
DomainHasConstraints(jsexpr->returning->typid, NULL))
jsexpr->use_json_coercion = true;
else
jsexpr->use_io_coercion = true;

Quite aside from whether this is the right thing, testing
DomainHasConstraints *at parse time* is insane. The situation
could be completely different by the time we execute the expression.
At the earliest this decision should be made in the planner; but
probably doing it at executor startup would be better.

Another reason for doing it later is that we cannot fix the behavior
in released versions given this structure: even if we end up still
using exactly two booleans, any stored JsonExpr will already have
locked down the values of these flags. What I'm contemplating as a
potential fix design is to remove these flags in master, leave them
present-but-unused in back branches, and adjust the runtime
expression representation instead.

I'm afraid now to look through the rest of the SQL/JSON logic for
similar errors.

regards, tom lane