Skip to content

Ensure that JSON.GET returns Nil response #3470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 12, 2025
Merged

Ensure that JSON.GET returns Nil response #3470

merged 6 commits into from
Aug 12, 2025

Conversation

ofekshenawa
Copy link
Collaborator

@ofekshenawa ofekshenawa commented Aug 11, 2025

  • Updated JSONCmd.readReply to return redis.Nil if no results
  • Added a doc line for Val() and Expanded() methods of JSONCmd
  • Added a test case for non existent keys in json_test.go

Closes #2987

nic-gibson and others added 4 commits August 11, 2025 12:02
Ensure that JSON.GET returns redis.Nil for missing keys/paths,
making it consistent with other Redis commands.

- Restore proper nil detection logic in JSONCmd.readReply
- Add comprehensive test coverage for JSON nil scenarios
- Handle both non-existent keys and non-existent paths consistently
- Distinguish between empty arrays and nil responses
- Add documentation for Val() and Expanded() methods

Original work and problem identification by Nic Gibson.
Enhanced implementation with comprehensive testing and fixes
for the broken nil detection logic.

Fixes #2987
- Non-existent keys return redis.Nil (consistent with other Redis commands)
- Non-existent paths in existing keys return empty array '[]'
- Fix broken test that was using wrong doc1 reference
- Add comprehensive test coverage for JSON nil scenarios

This aligns with official Redis JSON.GET behavior:
- Missing keys should return nil error like other Redis commands
- Missing paths should return empty JSON array, not error
@ofekshenawa ofekshenawa marked this pull request as ready for review August 11, 2025 15:53
Copy link
Member

@ndyakov ndyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks good to me!

@ndyakov ndyakov added the bug label Aug 12, 2025
@ndyakov ndyakov merged commit 0b1e9f7 into master Aug 12, 2025
20 checks passed
@ndyakov ndyakov deleted the json-nil-response branch August 12, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants