-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
#499: Add support for command JSON.OBJLEN #593
#499: Add support for command JSON.OBJLEN #593
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@phaneesh707 thanks for contributing, have some minor comments. Please have a look once.
internal/eval/eval.go
Outdated
// Retrieve the object from the database | ||
obj := store.Get(key) | ||
if obj == nil { | ||
return diceerrors.NewErrWithMessage("Path '.' does not exist or not a json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
is not an invalid path. We should return the length of complete parent object if the arg contains .
as path
.
127.0.0.1:6379> JSON.OBJLEN doc .
(integer) 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So when i use JSON.OBJLEN doc .
the library is not able to parse it , gives this error - ERRR not terminated at 2 in .
, but it works fine for JSON.OBJLEN doc .person
or something similar.so should i just do this ?
if path == "."{
path = "$"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lucifercr07 any suggestion for ^^??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's tackle this separately. Created issue #603 for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these changes @phaneesh707. I have left a few comments.
Issue
Issue fixed : #499
Description
This PR introduces the new command
JSON.OBJLEN
Dice DB(CLI demo)

Redis

Testing details