feat: JSON.MERGE plus unit and integration tests by patrickn2 · Pull Request #95 · valkey-io/valkey-json

patrickn2 · 2025-12-30T19:31:17Z

JSON.MERGE Command
Followed instructions here https://redis.io/docs/latest/commands/json.merge/

Closes #74.

roshkhatri · 2025-12-30T19:46:15Z

Can you please add --sign off to the commits?: https://github.com/valkey-io/valkey-json/pull/95/checks?check_run_id=59176701849

patrickn2 · 2025-12-30T21:54:27Z

Can you please add --sign off to the commits?: https://github.com/valkey-io/valkey-json/pull/95/checks?check_run_id=59176701849

I guess I made a mess trying to fix it
Could you take a look?

roshkhatri · 2025-12-30T22:01:28Z

Can you please add --sign off to the commits?: https://github.com/valkey-io/valkey-json/pull/95/checks?check_run_id=59176701849

I guess I made a mess trying to fix it Could you take a look?

Yeah, its okay , you can just run these commands and it should be fine:

git rebase HEAD~3 --signoff
git push --force-with-lease origin feat--JSON.MERGE-Command

patrickn2 · 2025-12-30T22:19:25Z

Can you please add --sign off to the commits?: https://github.com/valkey-io/valkey-json/pull/95/checks?check_run_id=59176701849

I guess I made a mess trying to fix it Could you take a look?

Yeah, its okay , you can just run these commands and it should be fine:
git rebase HEAD~3 --signoff
git push --force-with-lease origin feat--JSON.MERGE-Command

Done

roshkhatri

Great PR, just some suggestions and may need to add more tests

roshkhatri · 2026-01-06T20:36:47Z

The failing CI would be fixed once we merge the latest unstable into this branch

roshkhatri · 2026-01-06T20:43:19Z

I am not sure though why these are failing: https://github.com/valkey-io/valkey-json/actions/runs/20278225578/job/58232842208
here the cloning into valkey is working but on this PR there seems to be some issue

sarthakaggarwal97

Few comments from my end. Thank you for the change.

sarthakaggarwal97 · 2026-02-06T22:32:26Z

@patrickn2 thanks for the fixes. Allow me some time to review again!

mkmkme

Hey! Here are some opinionated drive-by comments on the PR. Could you have a look please?

roshkhatri

I dont have more to add to what @sarthakaggarwal97 and @mkmkme
I think just rebasing onto unstable should work

roshkhatri · 2026-03-02T16:26:24Z

@patrickn2, if possible, can you please take a look at the feedback?
Valkey will be releasing 9.1 rc1 soon and we can release new version for valkey-json, so we can have valkey-json in the next bundle release.

patrickn2 · 2026-03-06T13:24:51Z

I guess I fixed everything guys
Thank you for your review @mkmkme @roshkhatri

…ck Nogueira <patricknn@gmail.com> Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…it and add unit tests for depth limit scenarios Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…d improve memory tracking. Update unit tests for merge values. Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…ses, and add unit tests for various merge scenarios Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…ts for various merge scenarios including array and object replacements Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…, mixed updates, error handling, and no-op scenarios Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

- Introduced a new configuration key `json.max-path-limit` to set the maximum nesting depth for JSON documents. - Updated README.md to include details about the new configuration option, its default value, and usage instructions. - Modified merge_values function to ensure proper handling of empty objects during merging. - Adjusted the command info for JSON.MERGE to reflect a new parameter value. Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…fixed them Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…nd add a new unit test for object replacement in merging - Moved the check for empty objects in merge_values to ensure proper handling during merging. - Added a unit test to verify that an object replaces an array at a specified key in the merge process. Signed-off-by: patricknn@gmail.com <patricknn@gmail.com> Made-with: Cursor Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…ormance - Changed key type from std::string to std::string_view to reduce unnecessary copies. - Updated key member check in merge_values to utilize the new key type. Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

…inimize unnecessary copies. Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

Signed-off-by: patrick.nogueira@fox.com Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

roshkhatri · 2026-03-06T21:32:36Z

There were some stray changes in the history, removed and pushed it, should be easier to review the PR now

Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>

Co-authored-by: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com> Signed-off-by: Patrick Nery Nogueira <119355744+patrickn2@users.noreply.github.com>

sarthakaggarwal97

Apologies for getting to this much later than I thought I would. I was playing with this change with AI. I think this needs one more pass.

I can reproduce two issues:

Recursive descent behaves differently from JSON.SET when the path matches both a parent and a child.
```
JSON.SET k . '{"a":{"a":1}}'
JSON.MERGE k '$..a' '5'
JSON.GET k
```
This returns {"a":{"a":5}}. For the same path, JSON.SET k '$..a' 5 gives {"a":5}. It looks like JSON.MERGE keeps applying child paths after the parent was already replaced.
The max document size check seems to miss multi-target merges. If the same patch is merged into two matched paths, JSON.MERGE can return OK even though the final document is over json.max-document-size.

sarthakaggarwal97 · 2026-06-22T22:36:25Z

We should plan to get this merged soon so we can release this command this year!

Signed-off-by: Patrick Nogueira <patricknn@gmail.com>

Signed-off-by: Patrick Nery Nogueira <119355744+patrickn2@users.noreply.github.com>

patrickn2 · 2026-06-30T21:25:26Z

Ok I fixed your issue @sarthakaggarwal97
I also did a double check against the RFC and fixed other edge cases.
please let me know if you guys need any other fix

patrickn2 · 2026-06-30T22:02:26Z

Fixing Lint errors

Signed-off-by: Patrick Nogueira <patricknn@gmail.com>

patrickn2 · 2026-06-30T22:05:29Z

Lint Errors fixed

roshkhatri

Hey, Thank you for working on this, my concerns were also inline with Sarthak's and they seem to be resolved.

Thanks @sarthakaggarwal97 for reviewing this, was waiting for it.

Thanks @patrickn2 for working on this.

roshkhatri · 2026-07-01T21:04:44Z

+        bool covered_by_ancestor = false;
+        for (auto &other : rs) {
+            if (&other == &vInfo) continue;
+            if (path_is_ancestor(other.second, path)) {
+                covered_by_ancestor = true;
+                break;
+            }


Here, The time complexity would be O(N^2) for is any other path an ancestor of this path. we can mention that. Also N would be bounded by the doc size limits.

Not a blocker, just putting it out there.

sarthakaggarwal97 · 2026-07-09T17:08:37Z

We should have a documentation update as well for this.

roshkhatri · 2026-07-10T16:58:08Z

@patrickn2 Can you open a PR for this to the valkey-io/valkey-doc repository?

patrickn2 · 2026-07-10T19:46:24Z

@patrickn2 Can you open a PR for this to the valkey-io/valkey-doc repository?

Oh, Sure I can, let me work on it, I didn't know that you guys had this repo

patrickn2 · 2026-07-10T20:14:54Z

@patrickn2 Can you open a PR for this to the valkey-io/valkey-doc repository?

Let me know if this is enough valkey-io/valkey-doc#456

patrickn2 mentioned this pull request Dec 30, 2025

Support JSON.MERGE #74

Closed

patrickn2 force-pushed the feat--JSON.MERGE-Command branch from 9301a02 to 3bb9a5d Compare December 30, 2025 22:18

roshkhatri reviewed Jan 6, 2026

View reviewed changes

patrickn2 force-pushed the feat--JSON.MERGE-Command branch 2 times, most recently from b53bf08 to 9713e53 Compare January 6, 2026 14:37

roshkhatri reviewed Jan 6, 2026

View reviewed changes

Comment thread src/CMakeLists.txt Outdated

sarthakaggarwal97 reviewed Feb 4, 2026

View reviewed changes

Comment thread src/json/dom.cc Outdated

Comment thread src/json/dom.cc Outdated

Comment thread src/json/dom.cc Outdated

Comment thread src/json/json.cc Outdated

Comment thread src/json/dom.cc Outdated

patrickn2 force-pushed the feat--JSON.MERGE-Command branch 2 times, most recently from c8376e8 to c62cd5a Compare February 6, 2026 21:35

sarthakaggarwal97 reviewed Feb 6, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml

mkmkme reviewed Feb 21, 2026

View reviewed changes

roshkhatri reviewed Feb 27, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml

Comment thread tst/integration/conftest.py

patrickn2 force-pushed the feat--JSON.MERGE-Command branch from c8e2180 to bdbf4b4 Compare March 6, 2026 13:26

roshkhatri reviewed Mar 6, 2026

View reviewed changes

Comment thread build.sh Outdated

patrickn2 added 7 commits March 6, 2026 21:21

feat: JSON.MERGE plus unit and integration tests Signed-off-by: Patri…

10c5114

…ck Nogueira <patricknn@gmail.com> Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

feat: Enhance merge_values function to use configurable max depth lim…

86a3105

…it and add unit tests for depth limit scenarios Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

refactor: Simplify JSON merge logic by consolidating path handling an…

b9ccd9a

…d improve memory tracking. Update unit tests for merge values. Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

feat: Enhance merge_values function to handle empty and non-object ca…

2742eb6

…ses, and add unit tests for various merge scenarios Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

fix: Correct parameter name in merge_values function and add unit tes…

d6bc392

…ts for various merge scenarios including array and object replacements Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

feat: Add integration tests for JSON.MERGE command covering wildcards…

89c5ec5

…, mixed updates, error handling, and no-op scenarios Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

chore: Remove JSON.MERGE documentation files

19b8e0a

Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

patrickn2 added 8 commits March 6, 2026 21:21

fix: Some edge cases of the RFC where not being met. Added tests and …

9b8c1c9

…fixed them Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

fix: Restoring ci.yaml due bad merge conflict

6658284

Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

fix: Replaced std::string with std::string_view for key handling to m…

672caed

…inimize unnecessary copies. Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

fix: Fixed comment

96f6421

Signed-off-by: patrick.nogueira@fox.com Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

feat: Rolling it back to unstable version of files

804d248

Signed-off-by: patrick.nogueira@fox.com Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

roshkhatri force-pushed the feat--JSON.MERGE-Command branch from bdbf4b4 to b1f8569 Compare March 6, 2026 21:30

fix: Remove stray offline-build changes from issue-offline branch

c57b920

Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>

roshkhatri force-pushed the feat--JSON.MERGE-Command branch from 787e805 to c57b920 Compare March 6, 2026 22:31

roshkhatri reviewed Apr 14, 2026

View reviewed changes

Comment thread src/commands/json.merge.json Outdated

Update src/commands/json.merge.json

60cfc5c

Co-authored-by: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com> Signed-off-by: Patrick Nery Nogueira <119355744+patrickn2@users.noreply.github.com>

sarthakaggarwal97 reviewed Jun 22, 2026

View reviewed changes

patrickn2 and others added 3 commits June 30, 2026 17:07

feat: Enhance JSON merge functionality with ancestor path handling

0fcf73c

Signed-off-by: Patrick Nogueira <patricknn@gmail.com>

feat: Fixing edge cases with rfc7396

90f6512

Signed-off-by: Patrick Nogueira <patricknn@gmail.com>

Merge branch 'unstable' into feat--JSON.MERGE-Command

885d9a4

Signed-off-by: Patrick Nery Nogueira <119355744+patrickn2@users.noreply.github.com>

fix: Fixing Lint errors

124cfc4

Signed-off-by: Patrick Nogueira <patricknn@gmail.com>

roshkhatri approved these changes Jul 1, 2026

View reviewed changes

roshkhatri merged commit dcaa82c into valkey-io:unstable Jul 1, 2026
19 checks passed

roshkhatri added the release-notes label Jul 1, 2026

Uh oh!

Conversation

patrickn2 commented Dec 30, 2025 • edited by enjoy-binbin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roshkhatri commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickn2 commented Dec 30, 2025

Uh oh!

roshkhatri commented Dec 30, 2025

Uh oh!

patrickn2 commented Dec 30, 2025

Uh oh!

roshkhatri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roshkhatri commented Jan 6, 2026

Uh oh!

roshkhatri commented Jan 6, 2026

Uh oh!

Uh oh!

sarthakaggarwal97 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sarthakaggarwal97 commented Feb 6, 2026

Uh oh!

Uh oh!

mkmkme left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roshkhatri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

roshkhatri commented Mar 2, 2026

Uh oh!

patrickn2 commented Mar 6, 2026

Uh oh!

Uh oh!

roshkhatri commented Mar 6, 2026

Uh oh!

Uh oh!

sarthakaggarwal97 left a comment

Choose a reason for hiding this comment

Uh oh!

sarthakaggarwal97 commented Jun 22, 2026

Uh oh!

patrickn2 commented Jun 30, 2026

Uh oh!

patrickn2 commented Jun 30, 2026

Uh oh!

patrickn2 commented Jun 30, 2026

Uh oh!

patrickn2 commented Dec 30, 2025 •

edited by enjoy-binbin

Loading

roshkhatri commented Dec 30, 2025 •

edited

Loading

sarthakaggarwal97 left a comment •

edited

Loading