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
JSON Path and RESP3 #1090
Comments
@Didas-git, thank you. The "extra array" makes sense once you consider that a non-legacy path can have multiple matches. Consider the following:
Now, this would be the result under both RESP2 and RESP3:
Now, in your example, under RESP2:
but this is equivalent to While under RESP3:
but this is equivalent to So this is not about RESP2 vs., RESP3, but about our decision to change the default path under RESP3 to There are multiple reasons to prefer 'non-legacy' paths. Just to mention one of them: the current format is Note: to clarify the terminology: a non-legacy path is any path that is equals to |
No, the issue is about having no way to reproduce the old behavior. You mentioned how |
I mentioned that non-legacy paths can have multiple matches, not |
I saw you added more to the message so this is a reply to that. |
I appologise for missunderstanding that part, however, that still doesnt make much sense to me, how can they have multiple matches? A path should only represent 1 value since they are unique. |
I added a simple example above:
As you can see - Here is another example (recursive descent):
|
This is something we will consider: obsolete all legacy path expressions, except |
I dont think we mean the same when we say matches, |
Maybe this is just a terminology issue. Anyway, we need to traverse the JSON tree, find matches, and return them as an array. This is why in the general case non-legacy paths should have that 'extra array' in the reply. |
I will wait for a follow up on that then. And on the note of documentation, i think that the default behavior changes for |
It makes sense but specially when fetching the root (fetching the entire object) it just brings an undesired "abstraction" and extra work to the user. Perhaps it could be checked if the path is just |
Note the following change in the 2.6 GA release notes: Instead of:
We now have:
|
What about the default behavior changes in some of the commands? Will that be reverted back or added to the documentation somehwere? The change of the main/default prefix was not documented anywhere and since |
We are not going to revert it. And yes, we still need to document RESP3 (which is true for everything in Redis, not just JSON). |
Thats what i guessed... |
In RedisJSON 2.6 RESP3 support was added and with this came a frightening change which was the deprecation notice for "legacy paths".
This brings some problems to the table that were not covered in the changelogs that i think are crucial for users to know because they will break a lot of code when switching over to RESP3.
The problem
The main problem here is that when the deprecated paths get removed there will be no way to retrieve the information without it being wrapped in an array, and this gets worse if your root is an array, this also introduces an issue where commands such as
JSON.GET
default to$
instead of.
and this is what will break a lot of code.Let me exemplify what this translates to in code.
Base:
RESP2:
RESP3:
With that said, that is not the only problem getting rid of "legacy paths" will cause.
Another big problem is in the behavior of
JSON.MGET
Using
.
JSON.MGET ObjectExample ArrayExample "."
Using
$
JSON.MGET ObjectExample ArrayExample "$"
There are also other commands that will get affected by this but i think this examples are enough to get my point.
While the addition of RESP3 is welcome the changes that came along are not that desired, while removing unnecessary features is good what we are seeing here is the removal of a core feature that will just leave users hanging with no choices.
If this doesn't get addressed i can see a lot of issues being opened in the future regarding it because its not what users want, users want to be able to get their data without any additional work, returning an array that will always be discarded is a waste of memory, bandwidth and performance.
The Solution(s)
With that said, I'm not saying that we should reverse de deprecation warning but instead that at least an alternative is given.
I was searching for a while on the JSON Path "spec" and it seems like there is not real standard that everyone follows and each one has their own twist or difference so i don't think we should sit here and start arguing that x spec says y and instead we should focus on a way to provide the feature that is being removed in another way because user satisfaction should be the priority over a "spec" that changes depending on what source you look at.
To find a solution we have to keep somethings in mind:
The first idea that comes to mind would be allowing
$*
since this syntax resemblesXPath
just like the majority ofJSONPath
does. I think this is the simplest and fastest solution however there might be some complications that I'm not aware that could make this not viable.One option that came to mind while writing this was to allow "top level"
@
but it might interfere withRediSearch
.Talking about RediSearch we also have the option of adding something like RediSearch's
DIALECT
option but to instead control how$
will behave when alone, however this will most likely go against one of the points mentioned above.Conclusion
I hope this gets addressed and i would highly appreciate that anyone who reads this gives their own honest opinion, i see this as something that both the community and the devs should agree on before changing so we don't run into the same problem again.
The text was updated successfully, but these errors were encountered: