mrhhsg opened a new pull request, #69288:
URL: https://github.com/apache/doris/pull/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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
