Re: BUG #19736: `JSON_OBJECT ... ABSENT ON NULL` accepts invalid keys when the value is NULL

Hi Dmitry,

Thanks for the suggestions. V2 is attached.

I found the same issue in json_object_agg_strict() and
jsonb_object_agg_strict() and extended the fix to those workers,
also covering JSON_OBJECTAGG with ABSENT ON NULL.

V2 shares the category checks through json_validate_key_category()
and clarifies that key restrictions apply even to omitted pairs.

For JSONTYPE_JSONB, json_categorize_type(..., false, ...) maps jsonb
to JSONTYPE_JSON, so v1 already covered it in the json path.
The shared helper explicitly handles both categories.

All 239 core regression tests pass on macOS/arm64 with assertions
enabled, on master at 7787aec8ab00, including the added aggregate tests.
I also verified that omitted keys still avoid output conversion without
uniqueness checking, while WITH UNIQUE KEYS retains its behavior.

Thanks,
Yeongjae Min

On Fri, 16 Oct 2026 22:41:27 +0200, Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
> > On Sun, Oct 11, 2026 at 07:26:02PM +0900, 민영재 wrote:
> > I reproduced both examples on master at 9e7d152488f1.
> >
> > In json_build_object_worker(), the ABSENT ON NULL skip happens before
> > NULL-key validation. Both json and jsonb paths also skip the scalar-key
> > checks when omitting an entry without a uniqueness check.
> >
> > The attached patch moves the json NULL-key check before the skip and
> > validates the key type before omitting an entry in either output path.
> > It avoids calling output or cast functions for omitted keys and preserves
> > the existing uniqueness checks.
> >
> > Regression tests cover NULL, array, composite, json and jsonb keys with
> > NULL values. The new tests fail without the fix; with the patch, make
> > check passes all 239 tests on macOS/arm64 with assertions enabled.
>
> Thanks for investigating. I think it's worth to clarify the
> documentation a bit -- one can argue that we don't need to verify keys
> if the pair is going to be skipped, and only the wording happen to
> insist on it. Currently it says:
>
> <replaceable>key_expression</replaceable> is a scalar expression
> defining the <acronym>JSON</acronym> key, which is
> converted to the <type>text</type> type.
> It cannot be <literal>NULL</literal> nor can it
> belong to a type that has a cast to the <type>json</type> type.
>
> Maybe extending it to "It cannot be ..., even if the pair is going to be
> eventually omitted due to ABSENT ON NULL".
>
> It also probably makes sense to verify if there are more such examples.
> E.g. looks like jsonb_object_agg_strict exhibit something similar:
>
> -- jsonb_object_agg_transfn_worker tests if the key is NULL
> select jsonb_object_agg_strict(NULL, NULL);
> ERROR: 22023: field name must not be null
>
> -- it also checks if the key is not a composite or json, if the
> -- value is not NULL
> select jsonb_object_agg_strict('{"a": 1}'::jsonb, 'value');
> ERROR: 22023: key value must be scalar, not array, composite, or json
>
> -- but it doesn't do so if the value is NULL
> select jsonb_object_agg_strict('{"a": 1}'::jsonb, NULL);
> jsonb_object_agg_strict
> -------------------------
> {}
> (1 row)
>
> Regarding the proposed patch, maybe the condition on tcategory could be
> factored out to not repeat it few times (including one in the
> datum_to_jsonb_internal). It's also not clear to me why JSONTYPE_JSONB
> is not included in json_build_object_worker, shouldn't it be included
> since it has a cast to json?