dragonflydb / dragonfly

A modern replacement for Redis and Memcached
https://www.dragonflydb.io/
Other
25.81k stars 948 forks source link

Some Json incompatibilities in the path specification #1523

Closed admtech closed 4 months ago

admtech commented 1 year ago

Hi,

Dragonfly still has some small Json incompatibilities in the path specification compared to the Redis Json module.

Here is the output from Redis: redis_version:7.0.11

127.0.0.1:6479> JSON.SET test $ '{"name":"Leonard Cohen","lastSeen":1478476800,"loggedOut": true}'
OK
127.0.0.1:6479> JSON.GET test .
"{\"name\":\"Leonard Cohen\",\"lastSeen\":1478476800,\"loggedOut\":true}"
127.0.0.1:6479> JSON.GET test $
"[{\"name\":\"Leonard Cohen\",\"lastSeen\":1478476800,\"loggedOut\":true}]"

127.0.0.1:6479> JSON.GET test $.name
"[\"Leonard Cohen\"]"
127.0.0.1:6479>
127.0.0.1:6479> JSON.GET test .name
"\"Leonard Cohen\""

127.0.0.1:6479> JSON.GET test .name $.lastSeen
"{\"$.lastSeen\":[1478476800],\".name\":[\"Leonard Cohen\"]}"
127.0.0.1:6479> JSON.GET test .name .lastSeen
"{\".lastSeen\":1478476800,\".name\":\"Leonard Cohen\"}"
27.0.0.1:6479> JSON.GET test $.name $.lastSeen
"{\"$.lastSeen\":[1478476800],\"$.name\":[\"Leonard Cohen\"]}"

Here is the output from Dragonfly: dfly_version:df-v1.5.0

127.0.0.1:6379> JSON.SET test $ '{"name":"Leonard Cohen","lastSeen":1478476800,"loggedOut": true}'
OK
127.0.0.1:6379> JSON.GET test .
"[{\"lastSeen\":1478476800,\"loggedOut\":true,\"name\":\"Leonard Cohen\"}]"
127.0.0.1:6379> JSON.GET test $
"[{\"lastSeen\":1478476800,\"loggedOut\":true,\"name\":\"Leonard Cohen\"}]"

127.0.0.1:6379> JSON.GET test $.name
"[\"Leonard Cohen\"]"
127.0.0.1:6379> JSON.GET test .name
(error) ERR syntax error

127.0.0.1:6379> JSON.GET test .name $.lastSeen
(error) ERR syntax error
127.0.0.1:6379> JSON.GET test .name .lastSeen
(error) ERR syntax error
127.0.0.1:6379> JSON.GET test $.name $.lastSeen
"{\"$.lastSeen\":[1478476800],\"$.name\":[\"Leonard Cohen\"]}"

They are the same commands under Redis and Dragonfly and yet the results are different. The guilty party must be the Path specification. So "JSON.GET test ." with a dot under Redis creates a different json array than JSON.GET test $". On Dragonfly they are the same.

When selecting single fields the dot does not work at all. "JSON.GET test .name .lastSeen" => (error) ERR syntax error

RedisInsight (2.2.8) uses the point as path specification for Json, the reading of Json content works only limited.

Dragonfly Version: Dragonfly Version

Redis Version: Redis Version

Under Dragonfly the content of a json is displayed with a [0] index in front of it and under Redis without the [0] index. Therefore under Dragonfly and RedisInsight the editing options of the dataset do not work.

Due to the wrong index [0] in the path, a field cannot be edited:

Dragonfly Version: Dragonfly Version

Redis Version: Redis Version

Test under:

Greetings Frank

royjacobson commented 1 year ago

Thanks for the report! I'm transferring the issue to the main dragonfly repo where we track those bugs.

iko1 commented 1 year ago

As far as I know, the DragonflyDB JSON module is aligned with the Json module of Elasticache and not with the JSON module of Redis. Also, the restricted path syntax is not supported by the DragonflyDB JSON module so you should use the enhanced path syntax.

admtech commented 1 year ago

@iko1

As far as I know, the DragonflyDB JSON module is aligned with the Json module of Elasticache and not with the JSON module of Redis. Also, the restricted path syntax is not supported by the DragonflyDB JSON module so you should use the enhanced path syntax.

I don't want to leave it at that: Dragonfly advertises itself as a Redis replacement, which makes sense because there is a lot of Redis software out there. So I was able to quickly integrate Dragonfly into our products. Without compatibility, I probably wouldn't have been able to do it as quickly.

So Dragonfly should also be Redis compatible in terms of structure. All other types work well, Json not quite yet. Compatibility with Elasticache doesn't help here. Talking about a Redis replacement, the official GUI RedisInsight should be fully supported. This also includes the path v1 syntax.

My thoughts on this. Greetings Frank

admtech commented 1 year ago

Is there any news about this here? It would be very handy for us developers if the official Redis GUI RedisInsight is supported. Currently we can see json content, but not edit or delete it (see screenshots above).

romange commented 1 year ago

the official GUI RedisInsight uses non-standard legacy syntax: https://redis.io/docs/data-types/json/path/#legacy-path-syntax

You can understand why we treat this request as low priority. Having said that if anyone contributes the fix, we will accept it.