-
Notifications
You must be signed in to change notification settings - Fork 683
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
GEODE-9950: Add LRANGE command #7389
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment deleted
...va/org/apache/geode/redis/internal/commands/executor/list/AbstractLRangeIntegrationTest.java
Outdated
Show resolved
Hide resolved
...va/org/apache/geode/redis/internal/commands/executor/list/AbstractLRangeIntegrationTest.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
0887879
to
7f8eaff
Compare
2fb8625
to
82faff1
Compare
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
11ed0b7
to
903932b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions, but none of them are mandatory. If you don't want to go to the trouble of refactoring the tests to be parameterized, then some clarification in the test names around whether the range being tested is valid (expected to contain elements) or invalid (expected to be empty) would be good, to avoid potential confusion.
...dis/src/main/java/org/apache/geode/redis/internal/commands/executor/list/LRangeExecutor.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
Outdated
Show resolved
Hide resolved
...va/org/apache/geode/redis/internal/commands/executor/list/AbstractLRangeIntegrationTest.java
Outdated
Show resolved
Hide resolved
903932b
to
45e041b
Compare
857b278
to
de81685
Compare
de81685
to
7230d15
Compare
This implements a version of the Redis LRANGE command, which is used for list data types. Associated tests were also added. Given a key, it returns a list of elements in the range (inclusive) for the specified start index and stop index. Both the start and stop indexes are zero based, which starts at the head of the list. Negative indexes start at the tail of the list. LINDEX was modified to use a new method created in this commit. The method converts negative indexes to the corresponding positive index.
implementation of iteration through list.
7230d15
to
8537733
Compare
This implements a version of the Redis LRANGE command, which is used for list data types. Associated tests were also added. Given a key, it returns a list of elements in the range (inclusive) for the specified start index and stop index. Both the start and stop indexes are zero based, which starts at the head of the list. Negative indexes start at the tail of the list. LINDEX was modified to use a new method created in this commit. The method converts negative indexes to the corresponding positive index.
This implements a version of the Redis LRANGE command, which is used for list data types. Associated tests were also added.
Given a key, it returns a list of elements in the range (inclusive) for the specified start index and stop index. Both the start and stop indexes are zero based, which starts at the head of the list. Negative indexes start at the tail of the list.
LINDEX was modified to use a new method created in this commit. The method converts negative indexes to the corresponding positive index.
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
Has your PR been rebased against the latest commit within the target branch (typically
develop
)?Is your initial contribution a single, squashed commit?
Does
gradlew build
run cleanly?Have you written or updated unit tests to verify your changes?
[NA] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?