DonalEvans commented on a change in pull request #6716:
URL: https://github.com/apache/geode/pull/6716#discussion_r675168845
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZRangeByScoreExecutor.java
##########
@@ -33,35 +30,35 @@
public class ZRangeByScoreExecutor extends AbstractExecutor {
@Override
public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
- RedisSortedSetCommands redisSortedSetCommands =
context.getSortedSetCommands();
-
List<byte[]> commandElements = command.getProcessedCommand();
- SortedSetRangeOptions rangeOptions;
- boolean withScores = false;
+ SortedSetScoreRangeOptions rangeOptions;
try {
byte[] minBytes = commandElements.get(2);
byte[] maxBytes = commandElements.get(3);
- rangeOptions = new SortedSetRangeOptions(minBytes, maxBytes);
+ rangeOptions = new SortedSetScoreRangeOptions(minBytes, maxBytes);
} catch (NumberFormatException ex) {
return RedisResponse.error(ERROR_MIN_MAX_NOT_A_FLOAT);
}
// Native redis allows multiple "withscores" and "limit ? ?" clauses; the
last "limit"
// clause overrides any previous ones
- if (commandElements.size() >= 5) {
- int currentCommandElement = 4;
+ // In order to match the error reporting behaviour of native Redis, the
argument list must be
+ // parsed in reverse order. Stop parsing at index = 4, since 0 is the
command name, 1 is the
+ // key, 2 is the min and 3 is the max
+ boolean withScores = false;
- while (currentCommandElement < commandElements.size()) {
+ if (commandElements.size() >= 5) {
Review comment:
I plan to do a major refactor of this code as part of the ZREVRANGEBYLEX
work, once all the other range commands are implemented, as you're right,
there's a lot of duplication and potential for simplification. I think we could
probably manage to have a single AbstractZRangeExecutor class, extended by the
ZRange, ZRevRange, ZRangeBYScore, ZRevRangeBYScore, ZRangeByLex and
ZRevRangeByLex executors that populates an AbstractSortedSetRangeOptions class,
extended by SortedSetRankRangeOptions, SortedSetScoreRangeOptions and
SortedSetLexRangeOptions classes. Maybe I'm overthinking it though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
