Skip to content

Support HSTRLEN command. - #115

Merged
Tal Zaccai (TalZaccai) merged 11 commits into
microsoft:mainfrom
argsno:support-hstrlen
Mar 27, 2024
Merged

Support HSTRLEN command.#115
Tal Zaccai (TalZaccai) merged 11 commits into
microsoft:mainfrom
argsno:support-hstrlen

Conversation

@argsno

@argsno David Wong (argsno) commented Mar 24, 2024

Copy link
Copy Markdown
Contributor

Support HSTRLEN command.

Returns the string length of the value associated with field in the hash stored at key. If the key or the field do not exist, 0 is returned.

Interested in being able to deepen my understanding by implementing some small features. thx.

refer: #76

@Zzhiter Zhe Zhang (Zzhiter) mentioned this pull request Mar 25, 2024
Closed
TalZaccai
Tal Zaccai (TalZaccai) suggested changes Mar 25, 2024
View reviewed changes

@TalZaccai Tal Zaccai (TalZaccai) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work! Added a few comments :)
Thanks for your contribution!

Comment thread libs/server/API/GarnetWatchApi.cs Outdated
Comment thread libs/server/Objects/Hash/HashObjectImpl.cs Outdated
Comment thread test/Garnet.test/RespHashTests.cs
@badrishc

Badrish Chandramouli (badrishc) commented Mar 25, 2024

Copy link
Copy Markdown
Collaborator

Make sure you use the error handling logic introduced in this recent PR: #103

@argsno

David Wong (argsno) commented Mar 26, 2024

Copy link
Copy Markdown
Contributor Author

Make sure you use the error handling logic introduced in this recent PR: #103

Yes, I have modify the code to use the same handling logic

@TalZaccai Tal Zaccai (TalZaccai) linked an issue Mar 26, 2024 that may be closed by this pull request
API Coverage - Implement HSTRLEN #76
Closed
@yrajas

Yoganand Rajasekaran (yrajas) commented Mar 26, 2024

Copy link
Copy Markdown
Contributor

Please make sure to update documentation to indicate HSTRLEN is supported now.
Under website/docs/commands/api-compatibility.md

@argsno

David Wong (argsno) commented Mar 26, 2024

Copy link
Copy Markdown
Contributor Author

Please make sure to update documentation to indicate HSTRLEN is supported now. Under website/docs/commands/api-compatibility.md

thanks, The api-compatibility.md and data-structures.md file have been updated to reflect HSTRLEN support.

@TalZaccai

Tal Zaccai (TalZaccai) commented Mar 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

I'm encountering an issue that seems to happen subsequently to when an error is returned. Please take a look...
Note how after "wrong number of arguments" we're getting "unknown command" for the same input
image

@argsno

David Wong (argsno) commented Mar 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I'm encountering an issue that seems to happen subsequently to when an error is returned. Please take a look... Note how after "wrong number of arguments" we're getting "unknown command" for the same input image

After parsing the incorrect command, a return is missed. I apologize for the bug I introduced.

@TalZaccai

Tal Zaccai (TalZaccai) commented Mar 27, 2024

Copy link
Copy Markdown
Contributor

I'm encountering an issue that seems to happen subsequently to when an error is returned. Please take a look... Note how after "wrong number of arguments" we're getting "unknown command" for the same input image

After parsing the incorrect command, a return is missed. I apologize for the bug I introduced.

No worries at all!

TalZaccai
Tal Zaccai (TalZaccai) approved these changes Mar 27, 2024
View reviewed changes

@TalZaccai Tal Zaccai (TalZaccai) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Thanks again for your work!

@TalZaccai
Tal Zaccai (TalZaccai) merged commit 439e312 into microsoft:main Mar 27, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@TalZaccai Tal Zaccai (TalZaccai) TalZaccai approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@TalZaccai Tal Zaccai (TalZaccai)

Labels

API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

API Coverage - Implement HSTRLEN

5 participants

@argsno @badrishc @yrajas @TalZaccai @vazois