Add the support of the HRANDFIELD command #1565

zevin02 · 2023-07-08T14:13:34Z

- Add parse for hrandfield - Implemented the case where the requested data is a positive number greater than the size of the hash. Signed-off-by: Zewen Xu <zevin9427@gmail.com>

src/commands/cmd_hash.cc

src/types/redis_hash.cc

- add test for hrandfield - Fix output (nil) when the key does not exist Signed-off-by: Zewen Xu <zevin9427@gmail.com>

src/types/redis_hash.cc

src/commands/cmd_hash.cc

src/types/redis_hash.cc

Yangsx-1 · 2023-07-09T17:03:02Z

You can also add some go tests.

src/commands/cmd_hash.cc

mapleFU

I didn't review the Hash::RandField implements, just some style nits

src/types/redis_hash.cc

zevin02 · 2023-07-10T11:35:45Z

You can also add some go tests.

I will do it later

- call the GetAll function to get all elements - add some cpp tests - wrap functions with lambdas - updated random selection Signed-off-by: Zewen Xu <zevin9427@gmail.com>

mapleFU

Looks much better now!

src/commands/cmd_hash.cc

src/types/redis_hash.cc

Signed-off-by: Zewen Xu <zevin9427@gmail.com>

mapleFU

Looks much better! I didn't check the tests carefully

src/types/redis_hash.cc

src/types/redis_hash.h

src/types/redis_hash.cc

PragmaTwice · 2023-07-14T08:40:19Z

Seems here are some clang-tidy reports that need to be fixed.

https://github.com/apache/kvrocks/actions/runs/5542464594/jobs/10117205769?pr=1565

mapleFU

Rest LGTM!

By the way, I noticed that you have written:

// TODO: Add test to verify randomness of the selected random fields

Would this be added in this patch or later?

src/commands/cmd_hash.cc

zevin02 · 2023-07-14T14:50:20Z

Rest LGTM!

By the way, I noticed that you have written:
// TODO: Add test to verify randomness of the selected random fields
Would this be added in this patch or later?

This patch will not implement that

tests/gocase/unit/type/hash/hash_test.go

mapleFU

cc @PragmaTwice @Yangsx-1 @git-hulk

tests/cppunit/types/hash_test.cc

Yangsx-1

Others are good to me. :)

src/commands/cmd_hash.cc

src/types/redis_hash.cc

Yangsx-1 · 2023-07-15T15:29:46Z

fixes :#1510

Add parse for hrandfield

Implemented the case where the requested data is a positive number greater than the size of the hash.

Close #1510 is better.

git-hulk · 2023-07-16T13:37:40Z

@zevin02 Thanks for your contribution and patience and greater thanks to reviewers: @mapleFU @PragmaTwice @Yangsx-1

Add the support of the HRANDFIELD command

1a1c918

- Add parse for hrandfield - Implemented the case where the requested data is a positive number greater than the size of the hash. Signed-off-by: Zewen Xu <zevin9427@gmail.com>

PragmaTwice reviewed Jul 8, 2023

View reviewed changes

src/commands/cmd_hash.cc Outdated Show resolved Hide resolved

Yangsx-1 reviewed Jul 8, 2023

View reviewed changes

src/commands/cmd_hash.cc Outdated Show resolved Hide resolved

mapleFU reviewed Jul 8, 2023

View reviewed changes

src/types/redis_hash.cc Outdated Show resolved Hide resolved

- implement hrandfield

501c4f2

- add test for hrandfield - Fix output (nil) when the key does not exist Signed-off-by: Zewen Xu <zevin9427@gmail.com>

PragmaTwice added the new-command label Jul 9, 2023

Yangsx-1 reviewed Jul 9, 2023

View reviewed changes

src/types/redis_hash.cc Outdated Show resolved Hide resolved

src/commands/cmd_hash.cc Outdated Show resolved Hide resolved

src/types/redis_hash.cc Outdated Show resolved Hide resolved

src/types/redis_hash.cc Outdated Show resolved Hide resolved

Yangsx-1 reviewed Jul 9, 2023

View reviewed changes

src/commands/cmd_hash.cc Show resolved Hide resolved

mapleFU reviewed Jul 10, 2023

View reviewed changes

src/types/redis_hash.cc Show resolved Hide resolved

src/types/redis_hash.cc Outdated Show resolved Hide resolved

src/types/redis_hash.cc Outdated Show resolved Hide resolved

src/types/redis_hash.cc Outdated Show resolved Hide resolved

PragmaTwice reviewed Jul 10, 2023

View reviewed changes

src/types/redis_hash.cc Outdated Show resolved Hide resolved

PragmaTwice reviewed Jul 10, 2023

View reviewed changes

src/types/redis_hash.cc Outdated Show resolved Hide resolved

- fix reply format

f50c639

- call the GetAll function to get all elements - add some cpp tests - wrap functions with lambdas - updated random selection Signed-off-by: Zewen Xu <zevin9427@gmail.com>

mapleFU reviewed Jul 10, 2023

View reviewed changes

zevin02 added 2 commits July 11, 2023 17:54

update

bfb3afd

Signed-off-by: Zewen Xu <zevin9427@gmail.com>

add go test for hrandfield

0112ef3

PragmaTwice requested a review from mapleFU July 11, 2023 17:56

mapleFU reviewed Jul 11, 2023

View reviewed changes

src/types/redis_hash.cc Outdated Show resolved Hide resolved

src/types/redis_hash.cc Show resolved Hide resolved

src/types/redis_hash.h Outdated Show resolved Hide resolved

src/types/redis_hash.cc Outdated Show resolved Hide resolved

zevin02 added 2 commits July 13, 2023 16:33

update

348e0f1

fix format

d04ad44

fix format

1c33d7f

mapleFU reviewed Jul 14, 2023

View reviewed changes

src/commands/cmd_hash.cc Show resolved Hide resolved

mapleFU reviewed Jul 14, 2023

View reviewed changes

tests/gocase/unit/type/hash/hash_test.go Outdated Show resolved Hide resolved

tests/gocase/unit/type/hash/hash_test.go Outdated Show resolved Hide resolved

update

91f9f79

mapleFU approved these changes Jul 15, 2023

View reviewed changes

PragmaTwice reviewed Jul 15, 2023

View reviewed changes

tests/cppunit/types/hash_test.cc Outdated Show resolved Hide resolved

update

874d977

Yangsx-1 reviewed Jul 15, 2023

View reviewed changes

src/commands/cmd_hash.cc Show resolved Hide resolved

src/commands/cmd_hash.cc Show resolved Hide resolved

src/types/redis_hash.cc Outdated Show resolved Hide resolved

src/types/redis_hash.cc Show resolved Hide resolved

update

fde499e

Yangsx-1 approved these changes Jul 15, 2023

View reviewed changes

PragmaTwice previously approved these changes Jul 15, 2023

View reviewed changes

update

e83a547

zevin02 dismissed PragmaTwice’s stale review via e83a547 July 16, 2023 09:05

git-hulk approved these changes Jul 16, 2023

View reviewed changes

Merge branch 'unstable' into hrandfield-command

776a756

git-hulk merged commit 0a2dcee into apache:unstable Jul 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the support of the HRANDFIELD command #1565

Add the support of the HRANDFIELD command #1565

zevin02 commented Jul 8, 2023 •

edited

Loading

Yangsx-1 commented Jul 9, 2023

mapleFU left a comment

zevin02 commented Jul 10, 2023

mapleFU left a comment

mapleFU left a comment

PragmaTwice commented Jul 14, 2023

mapleFU left a comment

zevin02 commented Jul 14, 2023

mapleFU left a comment

Yangsx-1 left a comment

Yangsx-1 commented Jul 15, 2023 •

edited

Loading

git-hulk commented Jul 16, 2023

Add the support of the HRANDFIELD command #1565

Add the support of the HRANDFIELD command #1565

Conversation

zevin02 commented Jul 8, 2023 • edited Loading

Yangsx-1 commented Jul 9, 2023

mapleFU left a comment

Choose a reason for hiding this comment

zevin02 commented Jul 10, 2023

mapleFU left a comment

Choose a reason for hiding this comment

mapleFU left a comment

Choose a reason for hiding this comment

PragmaTwice commented Jul 14, 2023

mapleFU left a comment

Choose a reason for hiding this comment

zevin02 commented Jul 14, 2023

mapleFU left a comment

Choose a reason for hiding this comment

Yangsx-1 left a comment

Choose a reason for hiding this comment

Yangsx-1 commented Jul 15, 2023 • edited Loading

git-hulk commented Jul 16, 2023

zevin02 commented Jul 8, 2023 •

edited

Loading

Yangsx-1 commented Jul 15, 2023 •

edited

Loading