Add support for JSON
#165 Added support for JSON.SET and JSON.GET operations; (jsonpath operations is wip)
@JyotinderSingh the existing Encode function seems to be sophisticated enough for handling the encoding of the json strings to RESP2.
However, in RESP3 standard; things have been changed a bit in case of handling of maps
Kindly take a look at the following docs/discussion: https://redis.io/docs/latest/develop/data-types/json/resp3/ https://github.com/RedisJSON/RedisJSON/issues/1090 https://redis.io/docs/latest/commands/json.set/
Thanks for the changes @Maveric-k07.
Please rebase this PR on the latest master to resolve any conflicts. Will review this once that is done.
@Maveric-k07 Regarding RESP3, we only aim to support RESP2 for now.
@JyotinderSingh Alright, let me know if it looks good; I will keep working on #166
Thanks for the contribution.
I have some thoughts:
We can probably consider replacing the standard encoding/json library with github.com/valyala/fastjson for both GET and SET operations. fastjson provides efficient parsing and manipulation of JSON objects.
Also, with this library it may be better to store JSON as []byte instead of interface{}. This can help reduce memory usage and will also align better with fastjson's API.
JSONGET
var parser fastjson.Parser
obj := Get(key)
// validations ...
jsonBytes, ok := obj.Value.([]byte)
if !ok {
...
}
v, err := parser.ParseBytes(jsonBytes)
JSONSET
Also, considering the upcoming features we plan to implement, such as JSON Path support (#166) the fastjson library can support that natively (https://pkg.go.dev/github.com/valyala/fastjson#example-Value.Set)
I'm just considering a path separated json path provided to us in the args, but we can implement the official redis standard in the actual PR
var parser fastjson.Parser
key := args[0]
path := args[1:len(args)-1]
jsonValue := args[len(args)-1]
obj := Get(key)
var v *fastjson.Value
if obj != nil {
// type and encoding validation ...
jsonBytes, _ := obj.Value.([]byte)
v, _ = parser.ParseBytes(jsonBytes)
}
if v == nil {
v = fastjson.MustParse("{}")
}
newValue, err := parser.Parse(jsonValue)
if len(path) == 0 {
v = newValue
} else {
err = v.Set(path, newValue)
if err != nil {
return Encode(errors.New("ERR invalid path"), false)
}
}
Would like to know your thoughts around this.
@arpitbbhayani what do you think about this?
@JyotinderSingh Thank you for your detailed feedback.
I agree that storing JSON as []byte instead of interface{} is a good approach. It will reduce memory usage and provide better compatibility with fastjson's API. This change aligns well with future implementations.
Adding below a POC for the suggestions using fastjson; all test cases passing in tests/json_test.go passing.
var parser fastjson.Parser
// evalJSONGET retrieves a JSON value stored at the specified key
func evalJSONGET(args []string) []byte {
if len(args) < 1 {
return Encode(errors.New("ERR wrong number of arguments for 'JSON.GET' command"), false)
}
key := args[0]
obj := Get(key)
if obj == nil || hasExpired(obj) {
return RESP_NIL
}
objType, _ := ExtractTypeEncoding(obj)
if objType != OBJ_TYPE_JSON {
return Encode(errors.New("WRONGTYPE Operation against a key holding the wrong kind of value"), false)
}
jsonBytes, ok := obj.Value.([]byte)
if !ok {
return Encode(errors.New("ERR invalid JSON value"), false)
}
v, err := parser.ParseBytes(jsonBytes)
if err != nil {
return Encode(errors.New("ERR invalid JSON"), false)
}
return Encode(v.String(), false)
}
// evalJSONSET stores a JSON value at the specified key
func evalJSONSET(args []string) []byte {
if len(args) < 3 {
return Encode(errors.New("ERR wrong number of arguments for 'JSON.SET' command"), false)
}
key := args[0]
// path := args[1 : len(args)-1]
jsonStr := args[len(args)-1]
v, err := parser.Parse(jsonStr)
if err != nil {
return Encode(errors.New("ERR invalid JSON"), false)
}
jsonBytes := v.MarshalTo(nil)
obj := NewObj(jsonBytes, -1, OBJ_TYPE_JSON, OBJ_ENCODING_JSON)
Put(key, obj)
return RESP_OK
}
As for the JSON PATH operations, I think we can build on-top of your idea and converge our way to the redis standard in #166
@Maveric-k07 @JyotinderSingh I would recommend let's go for a quick implementation for now and store interface{}, once we hit the roadblock then we can always optimize.
My concern is with query time capabilities. I think it would be better if we store JSON as map[string]interface{} the reason being we will make query time more efficient as we need not parse and query the JSON objects. Let me know if my understanding about the approaches has some gaps.
Also, I have updated the tutorial with the changes discussed last week. https://dicedb-docs.netlify.app/tutorials/realtime-leaderboard/
Take a look.
@JyotinderSingh would like to know your thoughts on it please, to finalize which way we are going.
I've been working on implementation of the jsonpath operations using fastjson and had trouble getting it working properly. I stumbled upon another project ajson that is providing us with out-of-the-box support for jsonpath operations, which we can use to ship quickly for the GET, SET and partial update via SET operations. Other JSON commands supported by redis can be added as well. Kindly take a look and let me know if i should go ahead with the implementation with ajson.
@JyotinderSingh would like to know your thoughts on it please, to finalize which way we are going.
I've been working on implementation of the jsonpath operations using fastjson and had trouble getting it working properly. I stumbled upon another project ajson that is providing us with out-of-the-box support for jsonpath operations, which we can use to ship quickly for the GET, SET and partial update via SET operations. Other JSON commands supported by redis can be added as well. Kindly take a look and let me know if i should go ahead with the implementation with
ajson.
I think we can proceed with Arpit's suggestion here. Let's pick up the feature extension work (path, performance) in a separate PR where we can evaluate our different options better. Let's also discuss that in the upcoming weekly sync.
Please proceed with implementing this as a map for now.
@JyotinderSingh I have gone with interface{} for storing the json in our application as redis' JSON.SET supports primitive datatypes as well:
JSON.SET message $ '"Hello, World!"'
JSON.SET booleanTrue $ true
JSON.SET numbers $ '[1,2,3,4,5]'
Support for these would not have been possible by having map[string]interface{}. Do let me know if I am missing something. All tests are passing and I also have a python script for testing out the flow of SET and GET operations.
I am very close to finishing the #166 using ajson, as a POC.
@JyotinderSingh I have gone with interface{} for storing the json in our application as redis' JSON.SET supports primitive datatypes as well:
JSON.SET message $ '"Hello, World!"' JSON.SET booleanTrue $ true JSON.SET numbers $ '[1,2,3,4,5]'Support for these would not have been possible by having map[string]interface{}. Do let me know if I am missing something. All tests are passing and I also have a python script for testing out the flow of SET and GET operations.
I am very close to finishing the #166 using ajson, as a POC.
Thanks for the changes. It makes sense to use an interface{}. Overall the PR looks good.
Also, I'm not sure why a python script is needed to test the operations, we already have the required go infrastructure for E2E testing in our repo.
Have also left a few comments.
Also, I'm not sure why a python script is needed to test the operations, we already have the required go infrastructure for E2E testing in our repo.
@JyotinderSingh The python script is temporary dev script that i am using for testing various cases in the JSON.SET implementation, will not be added to the repo. Had to rebase the branch, apologies.
I have made changes to address your suggestions above. Please check. Thanks