Subject: [PATCH v2] Validate JSON object keys before omitting null values JSON_OBJECT with ABSENT ON NULL can skip key validation when it omits an entry with a null value. The json path accepts null keys, and both json and jsonb paths can accept non-scalar keys. The strict object aggregate workers have the same non-scalar-key validation gap. Move the json constructor's null-key check before omission, and validate key categories in the skipped-entry paths of constructors and aggregates. Factor the existing category checks into json_validate_key_category(), shared with datum_to_json_internal() and datum_to_jsonb_internal(). This preserves the existing error messages and SQLSTATEs and avoids invoking output or cast functions for omitted entries without uniqueness checks. Clarify that key restrictions apply even when ABSENT ON NULL omits the pair, and document the restrictions for strict object aggregates. Add regression coverage for constructors and aggregates, including array, composite, json/jsonb and user-defined JSON-cast keys. Reported in BUG #19736: https://www.postgresql.org/message-id/19736-3a44617ea5534c77@postgresql.org diff --git a/doc/src/sgml/func/func-aggregate.sgml b/doc/src/sgml/func/func-aggregate.sgml index c366989..462f720 100644 --- a/doc/src/sgml/func/func-aggregate.sgml +++ b/doc/src/sgml/func/func-aggregate.sgml @@ -434,8 +434,10 @@ Collects all the key/value pairs into a JSON object. Key arguments are coerced to text; value arguments are converted as per to_json or to_jsonb. - The key cannot be null. If the - value is null then the pair is skipped. + The key must be a non-null scalar and cannot + belong to a type that has a cast to json, even if the + pair is skipped. If the value is null then + the pair is skipped. Duplicate keys are handled as for json_object_agg and jsonb_object_agg, after removing any pairs with null values. @@ -495,8 +497,10 @@ Collects all the key/value pairs into a JSON object. Key arguments are coerced to text; value arguments are converted as per to_json or to_jsonb. - The key cannot be null. If the - value is null then the pair is skipped. + The key must be a non-null scalar and cannot + belong to a type that has a cast to json, even if the + pair is skipped. If the value is null then + the pair is skipped. If there is a duplicate key (including pairs with null values) an error is thrown. diff --git a/doc/src/sgml/func/func-json.sgml b/doc/src/sgml/func/func-json.sgml index 0763ec8..2d2c92e 100644 --- a/doc/src/sgml/func/func-json.sgml +++ b/doc/src/sgml/func/func-json.sgml @@ -724,6 +724,8 @@ converted to the text type. It cannot be NULL nor can it belong to a type that has a cast to the json type. + These restrictions apply even if the pair is omitted from the output + due to ABSENT ON NULL. If WITH UNIQUE KEYS is specified, there must not be any duplicate key_expression. Any pair for which the value_expression diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 28e5f3c..e234e7c 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -193,14 +193,8 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, return; } - if (key_scalar && - (tcategory == JSONTYPE_ARRAY || - tcategory == JSONTYPE_COMPOSITE || - tcategory == JSONTYPE_JSON || - tcategory == JSONTYPE_CAST)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("key value must be scalar, not array, composite, or json"))); + if (key_scalar) + json_validate_key_category(tcategory); switch (tcategory) { @@ -1056,7 +1050,10 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo, * buffer to store the key name so that we can check it. */ if (!unique_keys) + { + json_validate_key_category(state->key_category); PG_RETURN_POINTER(state); + } out = json_unique_builder_get_throwawaybuf(&state->unique_check); } @@ -1220,6 +1217,12 @@ json_build_object_worker(int nargs, const Datum *args, const bool *nulls, const bool skip; int key_offset; + /* Keys must not be null, even if their values will be omitted. */ + if (nulls[i]) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("null value not allowed for object key"))); + /* Skip null values if absent_on_null */ skip = absent_on_null && nulls[i + 1]; @@ -1227,7 +1230,15 @@ json_build_object_worker(int nargs, const Datum *args, const bool *nulls, const { /* If key uniqueness check is needed we must save skipped keys */ if (!unique_keys) + { + JsonTypeCategory tcategory; + Oid outfuncoid; + + /* Validate the key type without converting the omitted key. */ + json_categorize_type(types[i], false, &tcategory, &outfuncoid); + json_validate_key_category(tcategory); continue; + } out = json_unique_builder_get_throwawaybuf(&unique_check); } @@ -1238,12 +1249,6 @@ json_build_object_worker(int nargs, const Datum *args, const bool *nulls, const out = result; } - /* process key */ - if (nulls[i]) - ereport(ERROR, - (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), - errmsg("null value not allowed for object key"))); - /* save key offset before appending it */ key_offset = out->len; diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index 29efd58..6e5adb4 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -645,19 +645,11 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, Assert(!key_scalar); jb.type = jbvNull; } - else if (key_scalar && - (tcategory == JSONTYPE_ARRAY || - tcategory == JSONTYPE_COMPOSITE || - tcategory == JSONTYPE_JSON || - tcategory == JSONTYPE_JSONB || - tcategory == JSONTYPE_CAST)) - { - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("key value must be scalar, not array, composite, or json"))); - } else { + if (key_scalar) + json_validate_key_category(tcategory); + switch (tcategory) { case JSONTYPE_ARRAY: @@ -1162,7 +1154,15 @@ jsonb_build_object_worker(int nargs, const Datum *args, const bool *nulls, const /* we need to save skipped keys for the key uniqueness check */ if (skip && !unique_keys) + { + JsonTypeCategory tcategory; + Oid outfuncoid; + + /* Validate the key type without converting the omitted key. */ + json_categorize_type(types[i], true, &tcategory, &outfuncoid); + json_validate_key_category(tcategory); continue; + } add_jsonb(args[i], false, &result, types[i], true); @@ -1645,7 +1645,10 @@ jsonb_object_agg_transfn_worker(FunctionCallInfo fcinfo, skip = absent_on_null && PG_ARGISNULL(2); if (skip && !unique_keys) + { + json_validate_key_category(state->key_category); PG_RETURN_POINTER(state); + } /* * We run this code in the normal function context, so that we don't leak diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index adbafd0..7560aa3 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -6065,6 +6065,24 @@ json_categorize_type(Oid typoid, bool is_jsonb, } } +/* + * Check that a JSON type category is valid for an object key. + * This must also be checked when a NULL value causes the pair to be omitted. + * The category must have been obtained from json_categorize_type(). + */ +void +json_validate_key_category(JsonTypeCategory tcategory) +{ + if (tcategory == JSONTYPE_ARRAY || + tcategory == JSONTYPE_COMPOSITE || + tcategory == JSONTYPE_JSON || + tcategory == JSONTYPE_JSONB || + tcategory == JSONTYPE_CAST) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("key value must be scalar, not array, composite, or json"))); +} + /* * Check whether a type conversion to JSON or JSONB involves any mutable * functions. This recurses into container types (arrays, composites, diff --git a/src/include/utils/jsonfuncs.h b/src/include/utils/jsonfuncs.h index 27713be..1beecd9 100644 --- a/src/include/utils/jsonfuncs.h +++ b/src/include/utils/jsonfuncs.h @@ -83,6 +83,7 @@ typedef enum extern void json_categorize_type(Oid typoid, bool is_jsonb, JsonTypeCategory *tcategory, Oid *outfuncoid); +extern void json_validate_key_category(JsonTypeCategory tcategory); extern void json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable); extern Datum datum_to_json(Datum val, JsonTypeCategory tcategory, diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 30f9e10..4e495c2 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -539,6 +539,31 @@ SELECT JSON_OBJECT('a': '1', 'b': NULL, 'c': 2 ABSENT ON NULL); {"a" : "1", "c" : 2} (1 row) +-- Keys must be valid even when ABSENT ON NULL omits their values. +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL RETURNING json); +ERROR: null value not allowed for object key +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL WITH UNIQUE KEYS RETURNING json); +ERROR: null value not allowed for object key +SELECT JSON_OBJECT((ARRAY[1,2]) VALUE NULL::text ABSENT ON NULL RETURNING json); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT((ROW(1,2)) VALUE NULL::text ABSENT ON NULL RETURNING json); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT((json '1') VALUE NULL::text ABSENT ON NULL RETURNING json); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT((jsonb '1') VALUE NULL::text ABSENT ON NULL RETURNING json); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +ERROR: argument 1: key must not be null +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL WITH UNIQUE KEYS RETURNING jsonb); +ERROR: argument 1: key must not be null +SELECT JSON_OBJECT((ARRAY[1,2]) VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT((ROW(1,2)) VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT((json '1') VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT((jsonb '1') VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +ERROR: key value must be scalar, not array, composite, or json SELECT JSON_OBJECT(1: 1, '2': NULL, '3': 1, repeat('x', 1000): 1, 2: repeat('a', 100) WITH UNIQUE); ERROR: duplicate JSON object key value: "2" SELECT JSON_OBJECT(1: 1, '1': NULL WITH UNIQUE); @@ -607,6 +632,14 @@ $$ LANGUAGE sql IMMUTABLE; CREATE CAST (mood AS json) WITH FUNCTION mood_to_json(mood) AS IMPLICIT; SELECT JSON_OBJECT('happy'::mood: '123'::jsonb); ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT('happy'::mood: NULL ABSENT ON NULL RETURNING json); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECT('happy'::mood: NULL ABSENT ON NULL RETURNING jsonb); +ERROR: key value must be scalar, not array, composite, or json +SELECT json_object_agg_strict('happy'::mood, NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT jsonb_object_agg_strict('happy'::mood, NULL); +ERROR: key value must be scalar, not array, composite, or json DROP CAST (mood AS json); DROP FUNCTION mood_to_json; DROP TYPE mood; @@ -963,6 +996,27 @@ SELECT JSON_OBJECTAGG(NULL: 1); ERROR: null value not allowed for object key SELECT JSON_OBJECTAGG(NULL: 1 RETURNING jsonb); ERROR: field name must not be null +-- Aggregate keys must also be valid when NULL values are omitted. +SELECT json_object_agg_strict(ARRAY[1,2], NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT json_object_agg_strict(ROW(1,2), NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT json_object_agg_strict(json '1', NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT json_object_agg_strict(jsonb '1', NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECTAGG((jsonb '1'): NULL ABSENT ON NULL RETURNING json); +ERROR: key value must be scalar, not array, composite, or json +SELECT jsonb_object_agg_strict(ARRAY[1,2], NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT jsonb_object_agg_strict(ROW(1,2), NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT jsonb_object_agg_strict(json '1', NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT jsonb_object_agg_strict(jsonb '1', NULL); +ERROR: key value must be scalar, not array, composite, or json +SELECT JSON_OBJECTAGG((jsonb '1'): NULL ABSENT ON NULL RETURNING jsonb); +ERROR: key value must be scalar, not array, composite, or json SELECT JSON_OBJECTAGG(i: i), -- JSON_OBJECTAGG(i VALUE i), diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index 9fa203c..b1b5205 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -138,6 +138,20 @@ SELECT JSON_OBJECT('a': '1', 'b': NULL, 'c': 2); SELECT JSON_OBJECT('a': '1', 'b': NULL, 'c': 2 NULL ON NULL); SELECT JSON_OBJECT('a': '1', 'b': NULL, 'c': 2 ABSENT ON NULL); +-- Keys must be valid even when ABSENT ON NULL omits their values. +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL RETURNING json); +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL WITH UNIQUE KEYS RETURNING json); +SELECT JSON_OBJECT((ARRAY[1,2]) VALUE NULL::text ABSENT ON NULL RETURNING json); +SELECT JSON_OBJECT((ROW(1,2)) VALUE NULL::text ABSENT ON NULL RETURNING json); +SELECT JSON_OBJECT((json '1') VALUE NULL::text ABSENT ON NULL RETURNING json); +SELECT JSON_OBJECT((jsonb '1') VALUE NULL::text ABSENT ON NULL RETURNING json); +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +SELECT JSON_OBJECT((NULL::text) VALUE NULL::text ABSENT ON NULL WITH UNIQUE KEYS RETURNING jsonb); +SELECT JSON_OBJECT((ARRAY[1,2]) VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +SELECT JSON_OBJECT((ROW(1,2)) VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +SELECT JSON_OBJECT((json '1') VALUE NULL::text ABSENT ON NULL RETURNING jsonb); +SELECT JSON_OBJECT((jsonb '1') VALUE NULL::text ABSENT ON NULL RETURNING jsonb); + SELECT JSON_OBJECT(1: 1, '2': NULL, '3': 1, repeat('x', 1000): 1, 2: repeat('a', 100) WITH UNIQUE); SELECT JSON_OBJECT(1: 1, '1': NULL WITH UNIQUE); @@ -171,6 +185,10 @@ CREATE FUNCTION mood_to_json(mood) RETURNS json AS $$ $$ LANGUAGE sql IMMUTABLE; CREATE CAST (mood AS json) WITH FUNCTION mood_to_json(mood) AS IMPLICIT; SELECT JSON_OBJECT('happy'::mood: '123'::jsonb); +SELECT JSON_OBJECT('happy'::mood: NULL ABSENT ON NULL RETURNING json); +SELECT JSON_OBJECT('happy'::mood: NULL ABSENT ON NULL RETURNING jsonb); +SELECT json_object_agg_strict('happy'::mood, NULL); +SELECT jsonb_object_agg_strict('happy'::mood, NULL); DROP CAST (mood AS json); DROP FUNCTION mood_to_json; DROP TYPE mood; @@ -293,6 +311,18 @@ SELECT JSON_OBJECTAGG(NULL: 1); SELECT JSON_OBJECTAGG(NULL: 1 RETURNING jsonb); +-- Aggregate keys must also be valid when NULL values are omitted. +SELECT json_object_agg_strict(ARRAY[1,2], NULL); +SELECT json_object_agg_strict(ROW(1,2), NULL); +SELECT json_object_agg_strict(json '1', NULL); +SELECT json_object_agg_strict(jsonb '1', NULL); +SELECT JSON_OBJECTAGG((jsonb '1'): NULL ABSENT ON NULL RETURNING json); +SELECT jsonb_object_agg_strict(ARRAY[1,2], NULL); +SELECT jsonb_object_agg_strict(ROW(1,2), NULL); +SELECT jsonb_object_agg_strict(json '1', NULL); +SELECT jsonb_object_agg_strict(jsonb '1', NULL); +SELECT JSON_OBJECTAGG((jsonb '1'): NULL ABSENT ON NULL RETURNING jsonb); + SELECT JSON_OBJECTAGG(i: i), -- JSON_OBJECTAGG(i VALUE i),