From aa6e46daf5304e8d9e66fefc1a5bd77622ec6402 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Sun, 31 Jan 2021 23:51:06 +0300 Subject: [PATCH] Throw error when assigning jsonb scalar instead of a composite object During the jsonb subscripting assignment, the provided path might assume an object or an array where the source jsonb has a scalar value. Initial subscripting assignment logic will skip such an update operation with no message shown. This commit makes it throw an error to indicate this type of situation. Discussion: https://postgr.es/m/CA%2Bq6zcV8qvGcDXurwwgUbwACV86Th7G80pnubg42e-p9gsSf%3Dg%40mail.gmail.com Discussion: https://postgr.es/m/CA%2Bq6zcX3mdxGCgdThzuySwH-ApyHHM-G4oB1R0fn0j2hZqqkLQ%40mail.gmail.com Discussion: https://postgr.es/m/CA%2Bq6zcVDuGBv%3DM0FqBYX8DPebS3F_0KQ6OVFobGJPM507_SZ_w%40mail.gmail.com Discussion: https://postgr.es/m/CA%2Bq6zcVovR%2BXY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA%40mail.gmail.com Author: Dmitry Dolgov Reviewed-by: Tom Lane, Arthur Zakirov, Pavel Stehule, Dian M Fay Reviewed-by: Andrew Dunstan, Chapman Flack, Merlin Moncure, Peter Geoghegan Reviewed-by: Alvaro Herrera, Jim Nasby, Josh Berkus, Victor Wagner Reviewed-by: Aleksander Alekseev, Robert Haas, Oleg Bartunov --- doc/src/sgml/json.sgml | 39 +++++++++++++++++++++++------ src/backend/utils/adt/jsonfuncs.c | 29 +++++++++++++++++++++ src/test/regress/expected/jsonb.out | 27 ++++++++++++++++++++ src/test/regress/sql/jsonb.sql | 17 +++++++++++++ 4 files changed, 104 insertions(+), 8 deletions(-) diff --git a/doc/src/sgml/json.sgml b/doc/src/sgml/json.sgml index 07bd19f9742..e16dd6973d2 100644 --- a/doc/src/sgml/json.sgml +++ b/doc/src/sgml/json.sgml @@ -614,8 +614,24 @@ SELECT jdoc->'guid', jdoc->'name' FROM api WHERE jdoc @> '{"tags": ["qu The result of a subscripting expression is always of the jsonb data type. + + UPDATE statements may use subscripting in the + SET clause to modify jsonb values. Subscript + paths must be traversible for all affected values insofar as they exist. For + instance, the path val['a']['b']['c'] can be traversed all + the way to c if every val, + val['a'], and val['a']['b'] is an + object. If any val['a'] or val['a']['b'] + is not defined, it will be created as an empty object and filled as + necessary. However, if any val itself or one of the + intermediary values is defined as a non-object such as a string, number, or + jsonb null, traversal cannot proceed so + an error is raised and the transaction aborted. + + An example of subscripting syntax: + -- Extract object value by key @@ -631,6 +647,10 @@ SELECT ('[1, "2", null]'::jsonb)[1]; -- value must be of the jsonb type as well UPDATE table_name SET jsonb_field['key'] = '1'; +-- This will raise an error if any record's jsonb_field['a']['b'] is something +-- other than an object. For example, the value {"a": 1} has no 'b' key. +UPDATE table_name SET jsonb_field['a']['b']['c'] = '1'; + -- Filter records using a WHERE clause with subscripting. Since the result of -- subscripting is jsonb, the value we compare it against must also be jsonb. -- The double quotes make "value" also a valid jsonb string. @@ -639,8 +659,9 @@ SELECT * FROM table_name WHERE jsonb_field['key'] = '"value"'; jsonb assignment via subscripting handles a few edge cases differently from jsonb_set. When a source jsonb - is NULL, assignment via subscripting will proceed as if - it was an empty JSON object: + value is NULL, assignment via subscripting will proceed + as if it was an empty JSON value of the type (object or array) implied by the + subscript key: -- Where jsonb_field was NULL, it is now {"a": 1} @@ -661,17 +682,19 @@ UPDATE table_name SET jsonb_field[2] = '2'; A jsonb value will accept assignments to nonexistent subscript - paths as long as the last existing path key is an object or an array. Since - the final subscript is not traversed, it may be an object key. Nested arrays - will be created and NULL-padded according to the path until - the value can be placed appropriately. + paths as long as the last existing element to be traversed is an object or + array, as implied by the corresponding subscript (the element indicated by + the last subscript in the path is not traversed and may be anything). Nested + array and object structures will be created, and in the former case + null-padded, as specified by the subscript path until the + assigned value can be placed. -- Where jsonb_field was {}, it is now {'a': [{'b': 1}]} UPDATE table_name SET jsonb_field['a'][0]['b'] = '1'; --- Where jsonb_field was [], it is now [{'a': 1}] -UPDATE table_name SET jsonb_field[0]['a'] = '1'; +-- Where jsonb_field was [], it is now [null, {'a': 1}] +UPDATE table_name SET jsonb_field[1]['a'] = '1'; diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index a0e1266e571..215a10f16ef 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -4931,6 +4931,21 @@ setPath(JsonbIterator **it, Datum *path_elems, switch (r) { case WJB_BEGIN_ARRAY: + + /* + * If instructed complain about attempts to replace whithin a raw + * scalar value. This happens even when current level is equal to + * path_len, because the last path key should also correspond to + * an object or an array, not raw scalar. + */ + if ((op_type & JB_PATH_FILL_GAPS) && (level <= path_len - 1) && + v.val.array.rawScalar) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("cannot replace existing key"), + errdetail("The path assumes key is a composite object, " + "but it is a scalar value."))); + (void) pushJsonbValue(st, r, NULL); setPathArray(it, path_elems, path_nulls, path_len, st, level, newval, v.val.array.nElems, op_type); @@ -4948,6 +4963,20 @@ setPath(JsonbIterator **it, Datum *path_elems, break; case WJB_ELEM: case WJB_VALUE: + + /* + * If instructed complain about attempts to replace whithin a + * scalar value. This happens even when current level is equal to + * path_len, because the last path key should also correspond to + * an object or an array, not an element or value. + */ + if ((op_type & JB_PATH_FILL_GAPS) && (level <= path_len - 1)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("cannot replace existing key"), + errdetail("The path assumes key is a composite object, " + "but it is a scalar value."))); + res = pushJsonbValue(st, r, &v); break; default: diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index 5b5510c4fdc..cf0ce0e44cc 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -5134,6 +5134,33 @@ select * from test_jsonb_subscript; 1 | {"a": [null, {"c": [null, null, 1]}]} (1 row) +-- trying replace assuming a composite object, but it's an element or a value +delete from test_jsonb_subscript; +insert into test_jsonb_subscript values (1, '{"a": 1}'); +update test_jsonb_subscript set test_json['a']['b'] = '1'; +ERROR: cannot replace existing key +DETAIL: The path assumes key is a composite object, but it is a scalar value. +update test_jsonb_subscript set test_json['a']['b']['c'] = '1'; +ERROR: cannot replace existing key +DETAIL: The path assumes key is a composite object, but it is a scalar value. +update test_jsonb_subscript set test_json['a'][0] = '1'; +ERROR: cannot replace existing key +DETAIL: The path assumes key is a composite object, but it is a scalar value. +update test_jsonb_subscript set test_json['a'][0]['c'] = '1'; +ERROR: cannot replace existing key +DETAIL: The path assumes key is a composite object, but it is a scalar value. +update test_jsonb_subscript set test_json['a'][0][0] = '1'; +ERROR: cannot replace existing key +DETAIL: The path assumes key is a composite object, but it is a scalar value. +-- trying replace assuming a composite object, but it's a raw scalar +delete from test_jsonb_subscript; +insert into test_jsonb_subscript values (1, 'null'); +update test_jsonb_subscript set test_json[0] = '1'; +ERROR: cannot replace existing key +DETAIL: The path assumes key is a composite object, but it is a scalar value. +update test_jsonb_subscript set test_json[0][0] = '1'; +ERROR: cannot replace existing key +DETAIL: The path assumes key is a composite object, but it is a scalar value. -- jsonb to tsvector select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb); to_tsvector diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index 0320db0ea46..1a9d21741fa 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -1371,6 +1371,23 @@ insert into test_jsonb_subscript values (1, '{"a": []}'); update test_jsonb_subscript set test_json['a'][1]['c'][2] = '1'; select * from test_jsonb_subscript; +-- trying replace assuming a composite object, but it's an element or a value + +delete from test_jsonb_subscript; +insert into test_jsonb_subscript values (1, '{"a": 1}'); +update test_jsonb_subscript set test_json['a']['b'] = '1'; +update test_jsonb_subscript set test_json['a']['b']['c'] = '1'; +update test_jsonb_subscript set test_json['a'][0] = '1'; +update test_jsonb_subscript set test_json['a'][0]['c'] = '1'; +update test_jsonb_subscript set test_json['a'][0][0] = '1'; + +-- trying replace assuming a composite object, but it's a raw scalar + +delete from test_jsonb_subscript; +insert into test_jsonb_subscript values (1, 'null'); +update test_jsonb_subscript set test_json[0] = '1'; +update test_jsonb_subscript set test_json[0][0] = '1'; + -- jsonb to tsvector select to_tsvector('{"a": "aaa bbb ddd ccc", "b": ["eee fff ggg"], "c": {"d": "hhh iii"}}'::jsonb);