mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
[fix](be) Keep following siblings when inserting nested JSON members (#69288)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
`JSON_SET` / `JSON_INSERT` (and the `jsonb_set` / `jsonb_insert`
spellings) silently dropped every member that followed a nested object
when they inserted a missing member into that object.
Reproduction:
```sql
mysql> SELECT JSON_SET('{"a":{},"b":2}', '$.a.c', 1);
+-----------------------------------------+
| json_set('{"a":{},"b":2}', '$.a.c', 1) |
+-----------------------------------------+
| {"a":{"c":1}} |
+-----------------------------------------+
```
The sibling member `"b"` is lost and no error is reported. The same
happens with a non-empty nested object (`{"a":{"d":4},"b":2}` with
`$.a.c` returns `{"a":{"d":4,"c":1}}`) and at deeper nesting
(`{"a":{"b":{}},"x":1}` with `$.a.b.c` returns `{"a":{"b":{"c":1}}}`,
dropping `"x"`).
Root cause: in `FunctionJsonbModify::write_json_value`
(`be/src/exprs/function/function_jsonb.cpp`), the object branch for
"last parent, member not present" opened and closed an extra object
inside the object that was already open:
- The extra `writeStartObject()` is rejected by the `JsonbWriter` state
machine: the current object is already open and waiting for a key, and
`verifyValueState()` only accepts `Object + WS_Key` or `Array +
WS_Value`. Its return value is ignored, so the call is a silent no-op.
- The extra `writeEndObject()` succeeds and closes the current object
early. The unconditional `writeEndObject()` at the end of the branch
then closes the enclosing object as well, so when the caller resumes
writing the remaining members of that object, its `writeKey` /
`writeValue` calls fail and their return values are ignored too.
- The writer's size counters have already been backfilled for the
prematurely closed object, so the result document passes validation and
the query succeeds with the fields missing.
Fix: remove the extra `writeStartObject()` / `writeEndObject()` pair and
append the new key/value directly to the already-open object; the
existing paired `writeEndObject()` closes it normally. After the fix:
```sql
SELECT JSON_SET('{"a":{},"b":2}', '$.a.c', 1); -- {"a":{"c":1},"b":2}
SELECT JSON_SET('{"a":{"d":4},"b":2}', '$.a.c', 1); -- {"a":{"d":4,"c":1},"b":2}
SELECT JSON_SET('{"a":{"b":{}},"x":1}', '$.a.b.c', 1); -- {"a":{"b":{"c":1}},"x":1}
```
Only the "nested object followed by sibling members" shape changes.
Inserts at the top level, inserts into an object inside an array,
`JSON_REPLACE` and `JSON_REMOVE` keep their current output and are
covered by control cases in the new unit test. The change does not
affect the JSONB binary format, RPC, metadata, configuration or the FE,
and existing data needs no migration.
### Release note
Fix `JSON_SET` and `JSON_INSERT` silently dropping the sibling members
that follow a nested object when a new member is inserted into that
nested object.
### Check List (For Author)
- Test
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason (add your reason)
- Behavior changed:
- [ ] No.
- [x] Yes. `JSON_SET` / `JSON_INSERT` now preserve all members of the
enclosing objects instead of dropping the members that follow a nested
object. This is the reported bug fix; top-level inserts, inserts into an
object inside an array, `JSON_REPLACE` and `JSON_REMOVE` are unchanged
and covered by control cases.
- Does this need documentation?
- [x] No.
### Testing details
**Unit test (BE)**
- New test
`FunctionJsonbTEST.JsonbModifyNestedMissingMemberKeepsFollowingSiblings`
in `be/test/exprs/function/function_jsonb_test.cpp`. It covers 7 data
rows (empty nested object, non-empty nested object, sibling written
before the target, two-level nesting, mixed-type siblings, empty key,
and an object element inside an array as a control) against all four
names `jsonb_insert`, `jsonb_set`, `json_insert`, `json_set`, plus a
`jsonb_replace` control.
- `./run-be-ut.sh --run
--filter=FunctionJsonbTEST.JsonbModifyNestedMissingMemberKeepsFollowingSiblings
-j 48` — PASSED (1 test).
- `./run-be-ut.sh --run --filter=FunctionJsonbTEST.* -j 48` — PASSED
(20/20 tests).
- Verification against the unfixed code: with the product change
temporarily reverted, the new test reports 20 assertion failures (5 data
rows x 4 function names), for example `result: {"a":{"c":1}}, expected
result: {"a":{"c":1},"b":2}`. The two control rows (sibling written
before the target; object inside an array) and the `jsonb_replace` case
still pass, which pins the failure to the changed branch.
**Regression test**
- `test_query_json_insert` and `test_query_json_set`
(`query_p0/sql_functions/json_function`) — both suites pass. Each suite
gains 5 cases: 4 constant SQL cases (minimal reproduction, non-empty
nested object, sibling-before control, two-level nesting) and 1 case
over a real `json` column with `ORDER BY id`.
- The checked-in `.out` files were generated by the regression framework
(not written by hand) and pass again on a re-run without regenerating
them.
**Formatting / static checks**
- `build-support/clang-format.sh` (clang-format 16): exit code 0, no
file changed.
- `build-support/check-build-hygiene.sh`: all checks passed.
- `build-support/run-clang-tidy.sh --build-dir be/build_ASAN --files
be/src/exprs/function/function_jsonb.cpp
be/test/exprs/function/function_jsonb_test.cpp`: could not complete in
the local environment because the installed clang-tidy cannot find
`stddef.h` (a local toolchain issue). All reported size/complexity
warnings are in functions that predate this change, and none is on a
changed line.
This commit is contained in:
1 parent
7872ff4f23
commit
cad3c8c8ec
6 files changed
+146
-2
No files matched your search
@@ -2252,10 +2252,15 @@ public:
|
||||
|
||||
if (is_last_parent && !value_written) {
|
||||
value_written = true;
|
||||
writer.writeStartObject();
|
||||
// The current object is already open above and is waiting for a key, so the
|
||||
// new member must be appended to it directly. Opening another object here
|
||||
// would be rejected by the writer, and closing one here would end the
|
||||
// current object early; the writeEndObject below would then close its
|
||||
// parent as well, silently dropping every member that follows the current
|
||||
// one. For example, {"a":{},"b":2} with path $.a.c used to return
|
||||
// {"a":{"c":1}} instead of {"a":{"c":1},"b":2}.
|
||||
writer.writeKey(last_leg->leg_ptr, static_cast<uint8_t>(last_leg->leg_len));
|
||||
writer.writeValue(value);
|
||||
writer.writeEndObject();
|
||||
}
|
||||
writer.writeEndObject();
|
||||
|
||||
|
||||
@@ -1117,6 +1117,49 @@ TEST(FunctionJsonbTEST, JsonbModifyMissingPathParent) {
|
||||
}
|
||||
}
|
||||
|
||||
TEST(FunctionJsonbTEST, JsonbModifyNestedMissingMemberKeepsFollowingSiblings) {
|
||||
InputTypeSet input_types = {PrimitiveType::TYPE_JSONB, PrimitiveType::TYPE_VARCHAR,
|
||||
PrimitiveType::TYPE_JSONB};
|
||||
|
||||
// Adding a missing member to a nested object must keep the members that follow that
|
||||
// object in its ancestors. Before the fix the enclosing object was closed one level
|
||||
// too early, so every following sibling was silently dropped.
|
||||
DataSet data_set = {
|
||||
// Nested empty object followed by a sibling.
|
||||
{{STRING(R"({"a":{},"b":2})"), STRING(R"($.a.c)"), STRING("1")},
|
||||
STRING(R"({"a":{"c":1},"b":2})")},
|
||||
// Non-empty nested object.
|
||||
{{STRING(R"({"a":{"d":4},"b":2})"), STRING(R"($.a.c)"), STRING("1")},
|
||||
STRING(R"({"a":{"d":4,"c":1},"b":2})")},
|
||||
// Control: a sibling written before the nested object was already correct.
|
||||
{{STRING(R"({"b":2,"a":{}})"), STRING(R"($.a.c)"), STRING("1")},
|
||||
STRING(R"({"b":2,"a":{"c":1}})")},
|
||||
// Two nested levels: the premature close dropped the sibling one level up.
|
||||
{{STRING(R"({"a":{"b":{}},"x":1})"), STRING(R"($.a.b.c)"), STRING("1")},
|
||||
STRING(R"({"a":{"b":{"c":1}},"x":1})")},
|
||||
// Following siblings of any type must be preserved.
|
||||
{{STRING(R"({"a":{},"b":[1,2],"c":{"d":3}})"), STRING(R"($.a.e)"), STRING("1")},
|
||||
STRING(R"({"a":{"e":1},"b":[1,2],"c":{"d":3}})")},
|
||||
// An empty key can be the inserted member.
|
||||
{{STRING(R"({"a":{},"b":2})"), STRING(R"($.a."")"), STRING("1")},
|
||||
STRING(R"({"a":{"":1},"b":2})")},
|
||||
// An object element inside an array was not affected; keep it as a guard.
|
||||
{{STRING(R"({"a":[{"x":1}],"b":2})"), STRING(R"($.a[0].y)"), STRING("1")},
|
||||
STRING(R"({"a":[{"x":1,"y":1}],"b":2})")},
|
||||
};
|
||||
for (const auto* func_name : {"jsonb_insert", "jsonb_set", "json_insert", "json_set"}) {
|
||||
static_cast<void>(check_function<DataTypeJsonb, true>(func_name, input_types, data_set));
|
||||
}
|
||||
|
||||
// jsonb_replace rewrites an existing member and already preserved its siblings.
|
||||
DataSet replace_data_set = {
|
||||
{{STRING(R"({"a":{"d":4},"b":2})"), STRING("$.a.d"), STRING("5")},
|
||||
STRING(R"({"a":{"d":5},"b":2})")},
|
||||
};
|
||||
static_cast<void>(
|
||||
check_function<DataTypeJsonb, true>("jsonb_replace", input_types, replace_data_set));
|
||||
}
|
||||
|
||||
TEST(FunctionJsonbTEST, JsonbModifyMemberPathOnArray) {
|
||||
InputTypeSet input_types = {PrimitiveType::TYPE_JSONB, PrimitiveType::TYPE_VARCHAR,
|
||||
PrimitiveType::TYPE_JSONB};
|
||||
|
||||
@@ -96,3 +96,20 @@
|
||||
-- !insert_odd_arity_ok --
|
||||
{"a":1} {"a":1,"b":2} {"a":1,"b":2}
|
||||
|
||||
-- !insert_nested_sibling --
|
||||
{"a":{"c":1},"b":2}
|
||||
|
||||
-- !insert_nested_sibling_nonempty --
|
||||
{"a":{"d":4,"c":1},"b":2}
|
||||
|
||||
-- !insert_nested_sibling_before --
|
||||
{"b":2,"a":{"c":1}}
|
||||
|
||||
-- !insert_deep_nested_sibling --
|
||||
{"a":{"b":{"c":1}},"x":1}
|
||||
|
||||
-- !insert_nested_sibling_json_col --
|
||||
1 {"a":{"c":1},"b":2}
|
||||
2 {"b":2,"a":{"c":1}}
|
||||
3 {"a":{"d":4,"c":1},"b":2}
|
||||
|
||||
@@ -88,3 +88,20 @@ null
|
||||
-- !set_odd_arity_ok --
|
||||
{"a":1} {"a":1,"b":2} {"a":1,"b":2}
|
||||
|
||||
-- !set_nested_sibling --
|
||||
{"a":{"c":1},"b":2}
|
||||
|
||||
-- !set_nested_sibling_nonempty --
|
||||
{"a":{"d":4,"c":1},"b":2}
|
||||
|
||||
-- !set_nested_sibling_before --
|
||||
{"b":2,"a":{"c":1}}
|
||||
|
||||
-- !set_deep_nested_sibling --
|
||||
{"a":{"b":{"c":1}},"x":1}
|
||||
|
||||
-- !set_nested_sibling_json_col --
|
||||
1 {"a":{"c":1},"b":2}
|
||||
2 {"b":2,"a":{"c":1}}
|
||||
3 {"a":{"d":4,"c":1},"b":2}
|
||||
|
||||
+31
@@ -150,4 +150,35 @@ suite("test_query_json_insert", "query,arrow_flight_sql") {
|
||||
exception "number of arguments must be odd, but got 4"
|
||||
}
|
||||
qt_insert_odd_arity_ok """select json_insert('{}', '\$.a', 1), json_insert('{}', '\$.a', 1, '\$.b', 2), jsonb_insert('{}', '\$.a', 1, '\$.b', 2);"""
|
||||
|
||||
// Inserting a missing member into a nested object must keep the members that follow it.
|
||||
order_qt_insert_nested_sibling """select json_insert('{"a":{},"b":2}', '\$.a.c', 1);"""
|
||||
order_qt_insert_nested_sibling_nonempty """select json_insert('{"a":{"d":4},"b":2}', '\$.a.c', 1);"""
|
||||
order_qt_insert_nested_sibling_before """select json_insert('{"b":2,"a":{}}', '\$.a.c', 1);"""
|
||||
order_qt_insert_deep_nested_sibling """select json_insert('{"a":{"b":{}},"x":1}', '\$.a.b.c', 1);"""
|
||||
|
||||
sql "DROP TABLE IF EXISTS test_query_json_insert_nested_sibling"
|
||||
sql """
|
||||
CREATE TABLE test_query_json_insert_nested_sibling (
|
||||
`id` int(11) not null,
|
||||
`j` json
|
||||
) ENGINE=OLAP
|
||||
DUPLICATE KEY(`id`)
|
||||
DISTRIBUTED BY HASH(`id`) BUCKETS 1
|
||||
PROPERTIES (
|
||||
"replication_allocation" = "tag.location.default: 1",
|
||||
"in_memory" = "false",
|
||||
"storage_format" = "V2"
|
||||
);
|
||||
"""
|
||||
sql """
|
||||
insert into test_query_json_insert_nested_sibling values
|
||||
(1, '{"a":{},"b":2}'),
|
||||
(2, '{"b":2,"a":{}}'),
|
||||
(3, '{"a":{"d":4},"b":2}');
|
||||
"""
|
||||
order_qt_insert_nested_sibling_json_col """
|
||||
select id, json_insert(j, '\$.a.c', 1) as v
|
||||
from test_query_json_insert_nested_sibling order by id;
|
||||
"""
|
||||
}
|
||||
+31
@@ -142,4 +142,35 @@ suite("test_query_json_set", "query") {
|
||||
exception "number of arguments must be odd, but got 4"
|
||||
}
|
||||
qt_set_odd_arity_ok """select json_set('{}', '\$.a', 1), json_set('{}', '\$.a', 1, '\$.b', 2), jsonb_set('{}', '\$.a', 1, '\$.b', 2);"""
|
||||
|
||||
// Setting a missing member on a nested object must keep the members that follow it.
|
||||
order_qt_set_nested_sibling """select json_set('{"a":{},"b":2}', '\$.a.c', 1);"""
|
||||
order_qt_set_nested_sibling_nonempty """select json_set('{"a":{"d":4},"b":2}', '\$.a.c', 1);"""
|
||||
order_qt_set_nested_sibling_before """select json_set('{"b":2,"a":{}}', '\$.a.c', 1);"""
|
||||
order_qt_set_deep_nested_sibling """select json_set('{"a":{"b":{}},"x":1}', '\$.a.b.c', 1);"""
|
||||
|
||||
sql "DROP TABLE IF EXISTS test_query_json_set_nested_sibling"
|
||||
sql """
|
||||
CREATE TABLE test_query_json_set_nested_sibling (
|
||||
`id` int(11) not null,
|
||||
`j` json
|
||||
) ENGINE=OLAP
|
||||
DUPLICATE KEY(`id`)
|
||||
DISTRIBUTED BY HASH(`id`) BUCKETS 1
|
||||
PROPERTIES (
|
||||
"replication_allocation" = "tag.location.default: 1",
|
||||
"in_memory" = "false",
|
||||
"storage_format" = "V2"
|
||||
);
|
||||
"""
|
||||
sql """
|
||||
insert into test_query_json_set_nested_sibling values
|
||||
(1, '{"a":{},"b":2}'),
|
||||
(2, '{"b":2,"a":{}}'),
|
||||
(3, '{"a":{"d":4},"b":2}');
|
||||
"""
|
||||
order_qt_set_nested_sibling_json_col """
|
||||
select id, json_set(j, '\$.a.c', 1) as v
|
||||
from test_query_json_set_nested_sibling order by id;
|
||||
"""
|
||||
}
|
||||
Reference in new issue
Block a user