Skip to content
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

#484: feat: add support for command [JSON.ARRLEN] #532

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

jujiale
Copy link
Contributor

@jujiale jujiale commented Sep 11, 2024

hello:
this pr is link #484

add json.arrlen command. refer to redis json.arrlen
add unit tests in eval_test.go

because JSON.ARRLEN never change the json value, so I think there is no need to add integration test in tests/json_test.go

the json.arrlen in redis behavior as below:
`

127.0.0.1:6379> JSON.SET user $ '{"age":13,"high":1.60,"pet":null,"language":["python","golang"],"flag":false,"partner":{"name":"tom","language":["rust"]}}'
OK
127.0.0.1:6379> JSON.ARRLEN user
(error) ERR Path '.' does not exist or not an array
127.0.0.1:6379> JSON.ARRLEN user $
1) (nil)
127.0.0.1:6379> JSON.ARRLEN user $.*
1) (nil)
2) (nil)
3) (nil)
4) (integer) 2
5) (nil)
6) (nil)
127.0.0.1:6379> JSON.ARRLEN user $..language
1) (integer) 2
2) (integer) 1
127.0.0.1:6379> JSON.ARRLEN user
(error) ERR Path '.' does not exist or not an array

`

in dice, keep consistent

@jujiale
Copy link
Contributor Author

jujiale commented Sep 11, 2024

@JyotinderSingh
hello. in dice test workflow. I find that TestExecuteQueryWithIncompatibleTypes/NullValue sometimes it result failed,as this pr run test failed.
I test in my local env, there is one failed in fivetimes, please have a check. thanks.

@JyotinderSingh
Copy link
Collaborator

@JyotinderSingh

hello. in dice test workflow. I find that TestExecuteQueryWithIncompatibleTypes/NullValue sometimes it result failed,as this pr run test failed.

I test in my local env, there is one failed in fivetimes, please have a check. thanks.

I will take a look

@JyotinderSingh
Copy link
Collaborator

@jujiale please pull the latest changes, I have removed this test since it was irrelevant.

@jujiale
Copy link
Contributor Author

jujiale commented Sep 11, 2024

@jujiale please pull the latest changes, I have removed this test since it was irrelevant.

done. Thank you for your prompt reply

@JyotinderSingh JyotinderSingh self-requested a review September 11, 2024 04:31
Copy link
Collaborator

@JyotinderSingh JyotinderSingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jujiale for adding support for JSON.ARRLEN! The changes look good and well tested.
LGTM.

@JyotinderSingh JyotinderSingh changed the title feat: add support for command [JSON.ARRLEN] #484: feat: add support for command [JSON.ARRLEN] Sep 11, 2024
@JyotinderSingh JyotinderSingh merged commit e56a0a7 into DiceDB:master Sep 11, 2024
2 checks passed
@jujiale jujiale deleted the feature-jsonarrlen branch September 12, 2024 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants