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

Doubt about the HRANGE command #1082

Closed
1 of 2 tasks
ShooterIT opened this issue Nov 10, 2022 · 6 comments · Fixed by #1120
Closed
1 of 2 tasks

Doubt about the HRANGE command #1082

ShooterIT opened this issue Nov 10, 2022 · 6 comments · Fixed by #1120
Assignees
Labels
enhancement type enhancement

Comments

@ShooterIT
Copy link
Member

ShooterIT commented Nov 10, 2022

Search before asking

  • I had searched in the issues and found no similar issues.

Motivation

Currently, we implement HRANGE command, but i think the name is not suitable.

For zset, we have ZRANGEBYLEX which could get members with lexicographical ordering,
of course, ZRANGE has BYLEX option.
HRANGE also get fields with lexicographical ordering, so i think its name should be
HRANGEBYLEX aligned with ZRANGEBYLEX. If we use HRANGE, users may think it is
similar with ZRANGE which start and stop are the position in it.

What's more, i think the min and max arguments of HRANGE should start with
( or [, in order to specify if the range item is respectively exclusive or inclusive, also align
with ZRANGEBYLEX

Solution

I suggest to change its name to HRANGEBYLEX and the min and max arguments of it
should start with ( or [

of course, we can implement HRANGE command with different options, but now, i think it
is not urgent since there is no demand of HRANGE(the same semantic of ZRANGE).

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@ShooterIT ShooterIT added the enhancement type enhancement label Nov 10, 2022
@git-hulk
Copy link
Member

git-hulk commented Nov 10, 2022

This is an excellent proposition, from my side, I think the HRANGE command is similar to ZRANGE over ZRANGEBYLEX, so I regarded those parameters as start and stop when reviewing the PR. But I'm wondering if it's right to align them because ZSET is a sorted array and it makes sense to use the index as the start/stop but it's a bit confusing for HASH.

@PragmaTwice
Copy link
Member

PragmaTwice commented Nov 10, 2022

Good point, currently ZRANGE and HRANGE have different semantics (indexes v.s. elements) and argument types (integers v.s. strings), so I also think ZRANGEBYLEX is more concrete.

BTW cc @tanruixiang

@tanruixiang
Copy link
Member

tanruixiang commented Nov 10, 2022

I also misunderstood start and stop when I first started implementing HRANGE.I'll add hrange and hrangebylex this weekend.

@git-hulk
Copy link
Member

git-hulk commented Jan 5, 2023

@tanruixiang Any update?

@tanruixiang
Copy link
Member

@tanruixiang Any update?

I'm sorry for the long absence of updates due to illness. I will continue this work this weekend.

@git-hulk
Copy link
Member

git-hulk commented Jan 5, 2023

@tanruixiang Thanks for your feedback. No hurry, I'm just wondering if I missed anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement type enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants