| From: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
|---|---|
| To: | 민영재 <dudwo990413(at)gmail(dot)com> |
| Cc: | pgsql-bugs(at)lists(dot)postgresql(dot)org, theshallow27(at)gmail(dot)com |
| Subject: | Re: BUG #19736: `JSON_OBJECT ... ABSENT ON NULL` accepts invalid keys when the value is NULL |
| Date: | 2026-10-16 20:41:27 |
| Message-ID: | atKHv08UIzPGHv91@ddolgov-thinkpadt14sgen1.rmtde.csb |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs pgsql-hackers |
> 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?
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Masahiko Sawada | 2026-10-16 20:49:23 | Re: Add a hook for handling logical decoding messages on subscribers. |
| Previous Message | Jacob Champion | 2026-10-16 20:16:33 | Re: Reapply graceful socket shutdown on Windows (revert 29992a6a509) |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Sami Imseih | 2026-10-16 19:49:13 | Re: [MASSMAIL] pg_stat_statements replaces SQL/JSON ON EMPTY / ON ERROR keywords with parameters |