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>

This reverts commit ac5a0e4. Signed-off-by: Roshan Khatri <rvkhatri@amazon.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>

patrickn2 mentioned this pull request Dec 30, 2025

Support JSON.MERGE #74

Open

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

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 and others added 10 commits March 6, 2026 21:21

chore: rebasing cmakelists and build.sh

83e0513

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

Revert "Simplify offline build (valkey-io#73)" (valkey-io#93)

dea68d3

This reverts commit ac5a0e4. Signed-off-by: Roshan Khatri <rvkhatri@amazon.com> Signed-off-by: patrick.nogueira@fox.com <patrick.nogueira@fox.com>

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>

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

patrickn2 commented Dec 30, 2025 •

edited by enjoy-binbin

Loading

roshkhatri commented Dec 30, 2025 •

edited

Loading

sarthakaggarwal97 left a comment •

edited

Loading