fix JSONPath inconsistencies

[N3XT14]

@JyotinderSingh.

Sorry for the delay. I was busy with something urgent at work and couldn't give enough time.

Have a look at this PR. It's not completed yet but good portion of it is completed. Unfortunately, I am unable to find how to enable the resp3 protocol.

Also, in the Data Link shared the multi path examples seems incorrect in the array example.

Currently, I am testing it on my end but since certain parts such as RESP3 and custom formats are pending I am not asking for a merge yet.

I wanted to also open up a discussion on certain outputs that Redis returns such as for example "accessing negative index on out of bounds for an arrray example", etc.

Should we be following that as I believe we should instead return error for such cases instead of nil or the way redis handles.

Fixes #1002

[JyotinderSingh]

We're not looking to support RESP3 at this stage. Let's stick with RESP2.

Is RESP3 support a blocker in any way?

[N3XT14]

Nope, definitely not a blocker.

[JyotinderSingh]

Nope, definitely not a blocker.

In that case we can remove the related code and limit this PR to just the jsonpath changes.

[JyotinderSingh]

Looks like the tests and linter are failing, could you please take a look? Also, is this PR ready for review?

[N3XT14]

Looks like the tests and linter are failing, could you please take a look? Also, is this PR ready for review?

Hi Yes,

Apologies, I was traveling the entire week so couldn't push.

I have pushed the latest changes. Test cases required changes in output structure mainly. I ran linter on my local before pushing it through some warnings and error at other parts of the code. I am not sure why it didn't caught attention earlier. Currently, GitHub actions is seeking approval

[JyotinderSingh]

Looks like the tests and linter are failing, could you please take a look? Also, is this PR ready for review?

Hi Yes,

Apologies, I was traveling the entire week so couldn't push.

I have pushed the latest changes. Test cases required changes in output structure mainly. I ran linter on my local before pushing it through some warnings and error at other parts of the code. I am not sure why it didn't caught attention earlier. Currently, GitHub actions is seeking approval

Thanks for the update! Since you've never pushed a commit into the repository before your PRs require a manual github actions run. I've started the CI for this change.

[JyotinderSingh]

Looks like there are some test failure, could you please take a look.

[N3XT14]

Looks like there are some test failure, could you please take a look.

There are two test cases failing:

1. JSON.MGET in resp: This issue likely stems from a mismatch in the output interface, which occurred during migration. Since my code doesn't directly impact the MGET test cases, I believe the failure is related to the migration process rather than any recent changes in the code.

2. JSON.GET for UnsupportedJSONPathOperations (including regex): Previously, regex was not supported in the JSONPath operations, but now it is. I have updated the test cases to reflect this change. However, another issue arises from the parsing logic in the ParseCommand function, which is located in the parsecommand.go file. The function currently only considers double quotes to determine if subsequent characters belong to the same path. In earlier test cases, single quotes were used, causing the parser to incorrectly treat them as separate paths. I have updated the test cases to use double quotes, but we need to improve the handling of quotes in the ParseCommand function to avoid similar issues in the future.

[lucifercr07]

@N3XT14 any updates on this? Please let us know if any help required on this.

[N3XT14]

Hi @lucifercr07. The implementation and test cases are working.

Latest commit: https://github.com/DiceDB/dice/pull/1266/commits/d5fa22256ad54f15222a97651f9fe848eac1da17

Furthermore, I am waiting for a response on the above two questions mentioned in previous comment.

[JyotinderSingh]

Hi @N3XT14, could you please rebase the PR on the latest commit? I'm wondering if some issues would automatically get fixed.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

:x: Yoswal
:x: N3XT14
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[arpitbbhayani]

this is obsolete now. Closing the PR to keep things clean. Thanks for working on the patch.