[GitHub] [geode] DonalEvans commented on a change in pull request #6715: GEODE-9379: Implement ZREVRANGEBYSCORE

[GitBox <gi...@apache.org>]

DonalEvans commented on a change in pull request #6715:
URL: https://github.com/apache/geode/pull/6715#discussion_r677723634



##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =

Review comment:
       This field is never used and can be removed.

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -310,18 +311,62 @@ long zcount(SortedSetRangeOptions rangeOptions) {
     return getRange(min, max, withScores, false);
   }
 
+  List<byte[]> zrevrangebyscore(SortedSetRangeOptions rangeOptions, boolean withScores) {
+    List<byte[]> result = new ArrayList<>();

Review comment:
       This assignment should be moved closer to the point at which the variable is used, as it's possible that we return from this method before we need it.

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "((", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "(("))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "(a", "(b"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "str", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "str"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "NaN"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+  }
+
+  @Test
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
+    jedis.zadd(KEY, 1.0, MEMBER_BASE_NAME);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "1", "2", "WITSCOREZ"))
+            .hasMessageContaining(ERROR_SYNTAX);
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenNonExistentKey() {
+    assertThat(jedis.zrevrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenMaxLessThanMin() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range -inf >= score >= +inf
+    assertThat(jedis.zrevrangeByScore(KEY, "-inf", "+inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnElement_givenRangeIncludingScore() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range inf >= score >= -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "-inf"))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 2 <= score <= 3
+    assertThat(jedis.zrevrangeByScore(KEY, score + 2, score + 1)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", -10.0);
+    map.put("member2", 1.0);
+    map.put("member3", 10.0);
+
+    jedis.zadd(KEY, map);
+
+    // Range -5 <= score <= 15
+    assertThat(jedis.zrevrangeByScore(KEY, "15", "-5"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore() {
+    Map<String, Double> map = new HashMap<>();
+    double score = 1;
+    map.put("member1", score);
+    map.put("member2", score);
+    map.put("member3", score);
+
+    jedis.zadd(KEY, map);
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member3", "member2", "member1");
+  }
+
+  @Test
+  public void shouldReturnRange_basicExclusivity() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member0", 0.0);
+    map.put("member1", 1.0);
+    map.put("member2", 2.0);
+    map.put("member3", 3.0);
+    map.put("member4", 4.0);
+
+    jedis.zadd(KEY, map);
+
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "(1.0"))
+        .containsExactly("member2");
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "1.0"))
+        .containsExactly("member2", "member1");
+    assertThat(jedis.zrevrangeByScore(KEY, "3.0", "(1.0"))
+        .containsExactly("member3", "member2");
+  }
+
+  private Map<String, Double> getExclusiveTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", Double.NEGATIVE_INFINITY);
+    map.put("member2", 1.0);
+    map.put("member3", Double.POSITIVE_INFINITY);
+    return map;
+  }
+
+  @Test
+  public void shouldReturnRange_givenExclusiveMin() {
+    Map<String, Double> map = getExclusiveTestMap();
+
+    jedis.zadd(KEY, map);
+
+    // Range +inf >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "+inf", "(-inf"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenExclusiveMinAndMaxEqualToScore() {
+    double score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    String scoreExclusive = "(" + score;
+    assertThat(jedis.zrevrangeByScore(KEY, scoreExclusive, scoreExclusive)).isEmpty();
+  }
+
+  @Test
+  // Using only "(" as either the min or the max is equivalent to "(0"
+  public void shouldReturnRange_givenLeftParenOnlyForMinOrMax() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("slightlyLessThanZero", -0.01);
+    map.put("zero", 0.0);
+    map.put("slightlyMoreThanZero", 0.01);
+
+    jedis.zadd(KEY, map);
+
+    // Range inf >= score > 0
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "(")).containsExactly("slightlyMoreThanZero");
+
+    // Range 0 >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "(", "-inf")).containsExactly("slightlyLessThanZero");
+  }
+
+  private void createZSetRangeTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("a", Double.NEGATIVE_INFINITY);
+    map.put("b", 1d);
+    map.put("c", 2d);
+    map.put("d", 3d);
+    map.put("e", 4d);
+    map.put("f", 5d);
+    map.put("g", Double.POSITIVE_INFINITY);
+
+    jedis.zadd(KEY, map);
+  }
+
+  @Test
+  public void shouldReturnRange_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 0, 2))
+        .containsExactly("f", "e");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 3))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 10))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 20, 10)).isEmpty();

Review comment:
       This case should probably be broken out into its own test, since it's more of a special case (offset is greater than the size of the range).

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {

Review comment:
       This test could be simplified by using the `@Parameters` annotation to pass all of the arguments, since the behaviour for each is always the same.

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "((", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "(("))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "(a", "(b"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "str", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "str"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "NaN"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+  }
+
+  @Test
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
+    jedis.zadd(KEY, 1.0, MEMBER_BASE_NAME);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "1", "2", "WITSCOREZ"))
+            .hasMessageContaining(ERROR_SYNTAX);
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenNonExistentKey() {
+    assertThat(jedis.zrevrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenMaxLessThanMin() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range -inf >= score >= +inf
+    assertThat(jedis.zrevrangeByScore(KEY, "-inf", "+inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnElement_givenRangeIncludingScore() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range inf >= score >= -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "-inf"))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 2 <= score <= 3
+    assertThat(jedis.zrevrangeByScore(KEY, score + 2, score + 1)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", -10.0);
+    map.put("member2", 1.0);
+    map.put("member3", 10.0);
+
+    jedis.zadd(KEY, map);
+
+    // Range -5 <= score <= 15
+    assertThat(jedis.zrevrangeByScore(KEY, "15", "-5"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore() {
+    Map<String, Double> map = new HashMap<>();
+    double score = 1;
+    map.put("member1", score);
+    map.put("member2", score);
+    map.put("member3", score);
+
+    jedis.zadd(KEY, map);
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member3", "member2", "member1");
+  }
+
+  @Test
+  public void shouldReturnRange_basicExclusivity() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member0", 0.0);
+    map.put("member1", 1.0);
+    map.put("member2", 2.0);
+    map.put("member3", 3.0);
+    map.put("member4", 4.0);
+
+    jedis.zadd(KEY, map);
+
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "(1.0"))
+        .containsExactly("member2");
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "1.0"))
+        .containsExactly("member2", "member1");
+    assertThat(jedis.zrevrangeByScore(KEY, "3.0", "(1.0"))
+        .containsExactly("member3", "member2");
+  }
+
+  private Map<String, Double> getExclusiveTestMap() {

Review comment:
       Could this private method be moved to the bottom of the class? Just personal preference, but it makes things neater, I think.

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "((", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "(("))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "(a", "(b"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "str", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "str"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "NaN"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+  }
+
+  @Test
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
+    jedis.zadd(KEY, 1.0, MEMBER_BASE_NAME);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "1", "2", "WITSCOREZ"))
+            .hasMessageContaining(ERROR_SYNTAX);
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenNonExistentKey() {
+    assertThat(jedis.zrevrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenMaxLessThanMin() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range -inf >= score >= +inf
+    assertThat(jedis.zrevrangeByScore(KEY, "-inf", "+inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnElement_givenRangeIncludingScore() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range inf >= score >= -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "-inf"))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 2 <= score <= 3
+    assertThat(jedis.zrevrangeByScore(KEY, score + 2, score + 1)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", -10.0);
+    map.put("member2", 1.0);
+    map.put("member3", 10.0);
+
+    jedis.zadd(KEY, map);
+
+    // Range -5 <= score <= 15
+    assertThat(jedis.zrevrangeByScore(KEY, "15", "-5"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore() {
+    Map<String, Double> map = new HashMap<>();
+    double score = 1;
+    map.put("member1", score);
+    map.put("member2", score);
+    map.put("member3", score);
+
+    jedis.zadd(KEY, map);
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member3", "member2", "member1");
+  }
+
+  @Test
+  public void shouldReturnRange_basicExclusivity() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member0", 0.0);
+    map.put("member1", 1.0);
+    map.put("member2", 2.0);
+    map.put("member3", 3.0);
+    map.put("member4", 4.0);
+
+    jedis.zadd(KEY, map);
+
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "(1.0"))
+        .containsExactly("member2");
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "1.0"))
+        .containsExactly("member2", "member1");
+    assertThat(jedis.zrevrangeByScore(KEY, "3.0", "(1.0"))
+        .containsExactly("member3", "member2");
+  }
+
+  private Map<String, Double> getExclusiveTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", Double.NEGATIVE_INFINITY);
+    map.put("member2", 1.0);
+    map.put("member3", Double.POSITIVE_INFINITY);
+    return map;
+  }
+
+  @Test
+  public void shouldReturnRange_givenExclusiveMin() {
+    Map<String, Double> map = getExclusiveTestMap();
+
+    jedis.zadd(KEY, map);
+
+    // Range +inf >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "+inf", "(-inf"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenExclusiveMinAndMaxEqualToScore() {
+    double score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    String scoreExclusive = "(" + score;
+    assertThat(jedis.zrevrangeByScore(KEY, scoreExclusive, scoreExclusive)).isEmpty();
+  }
+
+  @Test
+  // Using only "(" as either the min or the max is equivalent to "(0"
+  public void shouldReturnRange_givenLeftParenOnlyForMinOrMax() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("slightlyLessThanZero", -0.01);
+    map.put("zero", 0.0);
+    map.put("slightlyMoreThanZero", 0.01);
+
+    jedis.zadd(KEY, map);
+
+    // Range inf >= score > 0
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "(")).containsExactly("slightlyMoreThanZero");
+
+    // Range 0 >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "(", "-inf")).containsExactly("slightlyLessThanZero");
+  }
+
+  private void createZSetRangeTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("a", Double.NEGATIVE_INFINITY);
+    map.put("b", 1d);
+    map.put("c", 2d);
+    map.put("d", 3d);
+    map.put("e", 4d);
+    map.put("f", 5d);
+    map.put("g", Double.POSITIVE_INFINITY);
+
+    jedis.zadd(KEY, map);
+  }
+
+  @Test
+  public void shouldReturnRange_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 0, 2))
+        .containsExactly("f", "e");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 3))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 10))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 20, 10)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_withScores_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    Set<Tuple> firstExpected = new LinkedHashSet<>();
+    firstExpected.add(new Tuple("f", 5d));
+    firstExpected.add(new Tuple("e", 4d));
+
+    Set<Tuple> secondExpected = new LinkedHashSet<>();
+    secondExpected.add(new Tuple("d", 3d));
+    secondExpected.add(new Tuple("c", 2d));
+    secondExpected.add(new Tuple("b", 1d));
+
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 0, 0))
+        .isEmpty();
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 0, 2))
+        .containsExactlyElementsOf(firstExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, 3))
+        .containsExactlyElementsOf(secondExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, 10))
+        .containsExactlyElementsOf(secondExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, -1))
+        .containsExactlyElementsOf(secondExpected);

Review comment:
       This case should probably be broken out into its own test, since it's a special case (negative count returns all elements in range).

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -310,18 +311,62 @@ long zcount(SortedSetRangeOptions rangeOptions) {
     return getRange(min, max, withScores, false);
   }
 
+  List<byte[]> zrevrangebyscore(SortedSetRangeOptions rangeOptions, boolean withScores) {

Review comment:
       This method should also be overridden in `NullRedisSortedSet` to return an empty list.

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeByScoreExecutor.java
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.redis.internal.netty.Coder.bytesToLong;
+import static org.apache.geode.redis.internal.netty.Coder.equalsIgnoreCaseBytes;
+import static org.apache.geode.redis.internal.netty.Coder.isNaN;
+import static org.apache.geode.redis.internal.netty.Coder.narrowLongToInt;
+import static org.apache.geode.redis.internal.netty.StringBytesGlossary.bLIMIT;
+import static org.apache.geode.redis.internal.netty.StringBytesGlossary.bWITHSCORES;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.executor.AbstractExecutor;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+public abstract class AbstractZRangeByScoreExecutor extends AbstractExecutor {
+  @Override
+  public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
+    RedisSortedSetCommands redisSortedSetCommands = context.getSortedSetCommands();

Review comment:
       This should be moved closer to the place that it's used, since it's possible we return before we need it.

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "((", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "(("))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "(a", "(b"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "str", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "str"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "NaN"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+  }
+
+  @Test
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
+    jedis.zadd(KEY, 1.0, MEMBER_BASE_NAME);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "1", "2", "WITSCOREZ"))
+            .hasMessageContaining(ERROR_SYNTAX);
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenNonExistentKey() {
+    assertThat(jedis.zrevrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenMaxLessThanMin() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range -inf >= score >= +inf
+    assertThat(jedis.zrevrangeByScore(KEY, "-inf", "+inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnElement_givenRangeIncludingScore() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range inf >= score >= -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "-inf"))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 2 <= score <= 3
+    assertThat(jedis.zrevrangeByScore(KEY, score + 2, score + 1)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", -10.0);
+    map.put("member2", 1.0);
+    map.put("member3", 10.0);
+
+    jedis.zadd(KEY, map);
+
+    // Range -5 <= score <= 15
+    assertThat(jedis.zrevrangeByScore(KEY, "15", "-5"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore() {
+    Map<String, Double> map = new HashMap<>();
+    double score = 1;
+    map.put("member1", score);
+    map.put("member2", score);
+    map.put("member3", score);
+
+    jedis.zadd(KEY, map);
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member3", "member2", "member1");
+  }
+
+  @Test
+  public void shouldReturnRange_basicExclusivity() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member0", 0.0);
+    map.put("member1", 1.0);
+    map.put("member2", 2.0);
+    map.put("member3", 3.0);
+    map.put("member4", 4.0);
+
+    jedis.zadd(KEY, map);
+
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "(1.0"))
+        .containsExactly("member2");
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "1.0"))
+        .containsExactly("member2", "member1");
+    assertThat(jedis.zrevrangeByScore(KEY, "3.0", "(1.0"))
+        .containsExactly("member3", "member2");
+  }
+
+  private Map<String, Double> getExclusiveTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", Double.NEGATIVE_INFINITY);
+    map.put("member2", 1.0);
+    map.put("member3", Double.POSITIVE_INFINITY);
+    return map;
+  }
+
+  @Test
+  public void shouldReturnRange_givenExclusiveMin() {
+    Map<String, Double> map = getExclusiveTestMap();
+
+    jedis.zadd(KEY, map);
+
+    // Range +inf >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "+inf", "(-inf"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenExclusiveMinAndMaxEqualToScore() {
+    double score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    String scoreExclusive = "(" + score;
+    assertThat(jedis.zrevrangeByScore(KEY, scoreExclusive, scoreExclusive)).isEmpty();
+  }
+
+  @Test
+  // Using only "(" as either the min or the max is equivalent to "(0"
+  public void shouldReturnRange_givenLeftParenOnlyForMinOrMax() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("slightlyLessThanZero", -0.01);
+    map.put("zero", 0.0);
+    map.put("slightlyMoreThanZero", 0.01);
+
+    jedis.zadd(KEY, map);
+
+    // Range inf >= score > 0
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "(")).containsExactly("slightlyMoreThanZero");
+
+    // Range 0 >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "(", "-inf")).containsExactly("slightlyLessThanZero");
+  }
+
+  private void createZSetRangeTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("a", Double.NEGATIVE_INFINITY);
+    map.put("b", 1d);
+    map.put("c", 2d);
+    map.put("d", 3d);
+    map.put("e", 4d);
+    map.put("f", 5d);
+    map.put("g", Double.POSITIVE_INFINITY);
+
+    jedis.zadd(KEY, map);
+  }
+
+  @Test
+  public void shouldReturnRange_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 0, 2))
+        .containsExactly("f", "e");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 3))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 10))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 20, 10)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_withScores_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    Set<Tuple> firstExpected = new LinkedHashSet<>();
+    firstExpected.add(new Tuple("f", 5d));
+    firstExpected.add(new Tuple("e", 4d));
+
+    Set<Tuple> secondExpected = new LinkedHashSet<>();
+    secondExpected.add(new Tuple("d", 3d));
+    secondExpected.add(new Tuple("c", 2d));
+    secondExpected.add(new Tuple("b", 1d));
+
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 0, 0))
+        .isEmpty();
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 0, 2))
+        .containsExactlyElementsOf(firstExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, 3))
+        .containsExactlyElementsOf(secondExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, 10))
+        .containsExactlyElementsOf(secondExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, -1))
+        .containsExactlyElementsOf(secondExpected);
+  }
+
+  @Test
+  public void shouldReturnProperError_givenLimitWithWrongFormat() {

Review comment:
       Could we also have a test case where the `LIMIT` argument is misspelled?

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)

Review comment:
       This test doesn't seem to use the `@Parameters` annotation anywhere, so this `@RunWith` is unnecessary. However I think there are probably a few tests that might benefit from parameterization, so maybe it should stay.

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/SortedSetRangeOptions.java
##########
@@ -20,10 +20,10 @@
 import java.util.Arrays;
 
 public class SortedSetRangeOptions {
-  private final double minDouble;
-  private final boolean minExclusive;
-  private final double maxDouble;
-  private final boolean maxExclusive;
+  private final double startDouble;
+  private final boolean startExclusive;
+  private final double endDouble;
+  private final boolean endExclusive;

Review comment:
       Rather than renaming all of these fields and methods, would it be simpler to just pass the appropriate arguments to the constructor in `ZRevRangeByScoreExecutor`? That is, if we know that `commandElements.get(2)` is the max, then use that as the second argument in the constructor rather than the first, using the `isRev()` method to switch the behaviour.

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "((", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "(("))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "(a", "(b"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "str", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "str"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "NaN"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+  }
+
+  @Test
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
+    jedis.zadd(KEY, 1.0, MEMBER_BASE_NAME);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "1", "2", "WITSCOREZ"))
+            .hasMessageContaining(ERROR_SYNTAX);
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenNonExistentKey() {
+    assertThat(jedis.zrevrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenMaxLessThanMin() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range -inf >= score >= +inf
+    assertThat(jedis.zrevrangeByScore(KEY, "-inf", "+inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnElement_givenRangeIncludingScore() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range inf >= score >= -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "-inf"))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 2 <= score <= 3
+    assertThat(jedis.zrevrangeByScore(KEY, score + 2, score + 1)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", -10.0);
+    map.put("member2", 1.0);
+    map.put("member3", 10.0);
+
+    jedis.zadd(KEY, map);
+
+    // Range -5 <= score <= 15
+    assertThat(jedis.zrevrangeByScore(KEY, "15", "-5"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore() {
+    Map<String, Double> map = new HashMap<>();
+    double score = 1;
+    map.put("member1", score);
+    map.put("member2", score);
+    map.put("member3", score);
+
+    jedis.zadd(KEY, map);
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member3", "member2", "member1");
+  }
+
+  @Test
+  public void shouldReturnRange_basicExclusivity() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member0", 0.0);
+    map.put("member1", 1.0);
+    map.put("member2", 2.0);
+    map.put("member3", 3.0);
+    map.put("member4", 4.0);
+
+    jedis.zadd(KEY, map);
+
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "(1.0"))
+        .containsExactly("member2");
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "1.0"))
+        .containsExactly("member2", "member1");
+    assertThat(jedis.zrevrangeByScore(KEY, "3.0", "(1.0"))
+        .containsExactly("member3", "member2");
+  }
+
+  private Map<String, Double> getExclusiveTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", Double.NEGATIVE_INFINITY);
+    map.put("member2", 1.0);
+    map.put("member3", Double.POSITIVE_INFINITY);
+    return map;
+  }
+
+  @Test
+  public void shouldReturnRange_givenExclusiveMin() {
+    Map<String, Double> map = getExclusiveTestMap();
+
+    jedis.zadd(KEY, map);
+
+    // Range +inf >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "+inf", "(-inf"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenExclusiveMinAndMaxEqualToScore() {
+    double score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    String scoreExclusive = "(" + score;
+    assertThat(jedis.zrevrangeByScore(KEY, scoreExclusive, scoreExclusive)).isEmpty();
+  }
+
+  @Test
+  // Using only "(" as either the min or the max is equivalent to "(0"
+  public void shouldReturnRange_givenLeftParenOnlyForMinOrMax() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("slightlyLessThanZero", -0.01);
+    map.put("zero", 0.0);
+    map.put("slightlyMoreThanZero", 0.01);
+
+    jedis.zadd(KEY, map);
+
+    // Range inf >= score > 0
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "(")).containsExactly("slightlyMoreThanZero");
+
+    // Range 0 >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "(", "-inf")).containsExactly("slightlyLessThanZero");
+  }
+
+  private void createZSetRangeTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("a", Double.NEGATIVE_INFINITY);
+    map.put("b", 1d);
+    map.put("c", 2d);
+    map.put("d", 3d);
+    map.put("e", 4d);
+    map.put("f", 5d);
+    map.put("g", Double.POSITIVE_INFINITY);
+
+    jedis.zadd(KEY, map);
+  }
+
+  @Test
+  public void shouldReturnRange_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 0, 2))
+        .containsExactly("f", "e");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 3))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 10))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 20, 10)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_withScores_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    Set<Tuple> firstExpected = new LinkedHashSet<>();
+    firstExpected.add(new Tuple("f", 5d));
+    firstExpected.add(new Tuple("e", 4d));
+
+    Set<Tuple> secondExpected = new LinkedHashSet<>();
+    secondExpected.add(new Tuple("d", 3d));
+    secondExpected.add(new Tuple("c", 2d));
+    secondExpected.add(new Tuple("b", 1d));
+
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 0, 0))
+        .isEmpty();

Review comment:
       This case should probably be broken out into its own test, since it's a special case (count of zero returns an empty list).

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "((", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "(("))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "(a", "(b"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "str", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "str"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "NaN"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+  }
+
+  @Test
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
+    jedis.zadd(KEY, 1.0, MEMBER_BASE_NAME);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "1", "2", "WITSCOREZ"))
+            .hasMessageContaining(ERROR_SYNTAX);
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenNonExistentKey() {
+    assertThat(jedis.zrevrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenMaxLessThanMin() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range -inf >= score >= +inf
+    assertThat(jedis.zrevrangeByScore(KEY, "-inf", "+inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnElement_givenRangeIncludingScore() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range inf >= score >= -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "-inf"))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 2 <= score <= 3
+    assertThat(jedis.zrevrangeByScore(KEY, score + 2, score + 1)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", -10.0);
+    map.put("member2", 1.0);
+    map.put("member3", 10.0);
+
+    jedis.zadd(KEY, map);
+
+    // Range -5 <= score <= 15
+    assertThat(jedis.zrevrangeByScore(KEY, "15", "-5"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore() {
+    Map<String, Double> map = new HashMap<>();
+    double score = 1;
+    map.put("member1", score);
+    map.put("member2", score);
+    map.put("member3", score);
+
+    jedis.zadd(KEY, map);
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member3", "member2", "member1");
+  }
+
+  @Test
+  public void shouldReturnRange_basicExclusivity() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member0", 0.0);
+    map.put("member1", 1.0);
+    map.put("member2", 2.0);
+    map.put("member3", 3.0);
+    map.put("member4", 4.0);
+
+    jedis.zadd(KEY, map);
+
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "(1.0"))
+        .containsExactly("member2");
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "1.0"))
+        .containsExactly("member2", "member1");
+    assertThat(jedis.zrevrangeByScore(KEY, "3.0", "(1.0"))
+        .containsExactly("member3", "member2");
+  }
+
+  private Map<String, Double> getExclusiveTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", Double.NEGATIVE_INFINITY);
+    map.put("member2", 1.0);
+    map.put("member3", Double.POSITIVE_INFINITY);
+    return map;
+  }
+
+  @Test
+  public void shouldReturnRange_givenExclusiveMin() {
+    Map<String, Double> map = getExclusiveTestMap();
+
+    jedis.zadd(KEY, map);
+
+    // Range +inf >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "+inf", "(-inf"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenExclusiveMinAndMaxEqualToScore() {
+    double score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    String scoreExclusive = "(" + score;
+    assertThat(jedis.zrevrangeByScore(KEY, scoreExclusive, scoreExclusive)).isEmpty();
+  }
+
+  @Test
+  // Using only "(" as either the min or the max is equivalent to "(0"
+  public void shouldReturnRange_givenLeftParenOnlyForMinOrMax() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("slightlyLessThanZero", -0.01);
+    map.put("zero", 0.0);
+    map.put("slightlyMoreThanZero", 0.01);
+
+    jedis.zadd(KEY, map);
+
+    // Range inf >= score > 0
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "(")).containsExactly("slightlyMoreThanZero");
+
+    // Range 0 >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "(", "-inf")).containsExactly("slightlyLessThanZero");
+  }
+
+  private void createZSetRangeTestMap() {

Review comment:
       Could this private method be moved to the bottom of the class too please?

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeByScoreExecutor.java
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.redis.internal.netty.Coder.bytesToLong;
+import static org.apache.geode.redis.internal.netty.Coder.equalsIgnoreCaseBytes;
+import static org.apache.geode.redis.internal.netty.Coder.isNaN;
+import static org.apache.geode.redis.internal.netty.Coder.narrowLongToInt;
+import static org.apache.geode.redis.internal.netty.StringBytesGlossary.bLIMIT;
+import static org.apache.geode.redis.internal.netty.StringBytesGlossary.bWITHSCORES;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.executor.AbstractExecutor;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+
+public abstract class AbstractZRangeByScoreExecutor extends AbstractExecutor {
+  @Override
+  public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
+    RedisSortedSetCommands redisSortedSetCommands = context.getSortedSetCommands();
+
+    List<byte[]> commandElements = command.getProcessedCommand();
+
+    SortedSetRangeOptions rangeOptions;
+    boolean withScores = false;

Review comment:
       This should be moved closer to the place that it's used, since it's possible we return before we need it.

##########
File path: geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRevRangeByScoreIntegrationTest.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.executor.sortedset;
+
+import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_MIN_MAX_NOT_A_FLOAT;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
+import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
+import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junitparams.JUnitParamsRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.Tuple;
+
+import org.apache.geode.redis.RedisIntegrationTest;
+
+@RunWith(JUnitParamsRunner.class)
+public abstract class AbstractZRevRangeByScoreIntegrationTest implements RedisIntegrationTest {
+  private static final String MEMBER_BASE_NAME = "member";
+  private static final String KEY = "key";
+  private JedisCluster jedis;
+  private static final List<Double> scores =
+      Arrays.asList(Double.NEGATIVE_INFINITY, -10.5, 0.0, 10.5, Double.POSITIVE_INFINITY);
+
+  @Before
+  public void setUp() {
+    jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+  }
+
+  @After
+  public void tearDown() {
+    flushAll();
+    jedis.close();
+  }
+
+  @Test
+  public void shouldError_givenWrongNumberOfArguments() {
+    assertAtLeastNArgs(jedis, Protocol.Command.ZREVRANGEBYSCORE, 3);
+  }
+
+  @Test
+  public void shouldError_givenInvalidMinOrMax() {
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "notANumber", "notANumber"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "((", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "(("))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "(a", "(b"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "str", "1"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "str"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+    assertThatThrownBy(() -> jedis.zrevrangeByScore("fakeKey", "1", "NaN"))
+        .hasMessageContaining(ERROR_MIN_MAX_NOT_A_FLOAT);
+  }
+
+  @Test
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
+    jedis.zadd(KEY, 1.0, MEMBER_BASE_NAME);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "1", "2", "WITSCOREZ"))
+            .hasMessageContaining(ERROR_SYNTAX);
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenNonExistentKey() {
+    assertThat(jedis.zrevrangeByScore("fakeKey", "-inf", "inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenMaxLessThanMin() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range -inf >= score >= +inf
+    assertThat(jedis.zrevrangeByScore(KEY, "-inf", "+inf")).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnElement_givenRangeIncludingScore() {
+    jedis.zadd(KEY, 1, "member");
+
+    // Range inf >= score >= -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "-inf"))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnEmptyArray_givenRangeExcludingScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 2 <= score <= 3
+    assertThat(jedis.zrevrangeByScore(KEY, score + 2, score + 1)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_givenMinAndMaxEqualToScore() {
+    int score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithDifferentScores() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", -10.0);
+    map.put("member2", 1.0);
+    map.put("member3", 10.0);
+
+    jedis.zadd(KEY, map);
+
+    // Range -5 <= score <= 15
+    assertThat(jedis.zrevrangeByScore(KEY, "15", "-5"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleMembersWithTheSameScoreAndMinAndMaxEqualToScore() {
+    Map<String, Double> map = new HashMap<>();
+    double score = 1;
+    map.put("member1", score);
+    map.put("member2", score);
+    map.put("member3", score);
+
+    jedis.zadd(KEY, map);
+
+    // Range 1 <= score <= 1
+    assertThat(jedis.zrevrangeByScore(KEY, score, score))
+        .containsExactly("member3", "member2", "member1");
+  }
+
+  @Test
+  public void shouldReturnRange_basicExclusivity() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member0", 0.0);
+    map.put("member1", 1.0);
+    map.put("member2", 2.0);
+    map.put("member3", 3.0);
+    map.put("member4", 4.0);
+
+    jedis.zadd(KEY, map);
+
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "(1.0"))
+        .containsExactly("member2");
+    assertThat(jedis.zrevrangeByScore(KEY, "(3.0", "1.0"))
+        .containsExactly("member2", "member1");
+    assertThat(jedis.zrevrangeByScore(KEY, "3.0", "(1.0"))
+        .containsExactly("member3", "member2");
+  }
+
+  private Map<String, Double> getExclusiveTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("member1", Double.NEGATIVE_INFINITY);
+    map.put("member2", 1.0);
+    map.put("member3", Double.POSITIVE_INFINITY);
+    return map;
+  }
+
+  @Test
+  public void shouldReturnRange_givenExclusiveMin() {
+    Map<String, Double> map = getExclusiveTestMap();
+
+    jedis.zadd(KEY, map);
+
+    // Range +inf >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "+inf", "(-inf"))
+        .containsExactly("member3", "member2");
+  }
+
+  @Test
+  public void shouldReturnEmptyList_givenExclusiveMinAndMaxEqualToScore() {
+    double score = 1;
+    jedis.zadd(KEY, score, "member");
+
+    String scoreExclusive = "(" + score;
+    assertThat(jedis.zrevrangeByScore(KEY, scoreExclusive, scoreExclusive)).isEmpty();
+  }
+
+  @Test
+  // Using only "(" as either the min or the max is equivalent to "(0"
+  public void shouldReturnRange_givenLeftParenOnlyForMinOrMax() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("slightlyLessThanZero", -0.01);
+    map.put("zero", 0.0);
+    map.put("slightlyMoreThanZero", 0.01);
+
+    jedis.zadd(KEY, map);
+
+    // Range inf >= score > 0
+    assertThat(jedis.zrevrangeByScore(KEY, "inf", "(")).containsExactly("slightlyMoreThanZero");
+
+    // Range 0 >= score > -inf
+    assertThat(jedis.zrevrangeByScore(KEY, "(", "-inf")).containsExactly("slightlyLessThanZero");
+  }
+
+  private void createZSetRangeTestMap() {
+    Map<String, Double> map = new HashMap<>();
+
+    map.put("a", Double.NEGATIVE_INFINITY);
+    map.put("b", 1d);
+    map.put("c", 2d);
+    map.put("d", 3d);
+    map.put("e", 4d);
+    map.put("f", 5d);
+    map.put("g", Double.POSITIVE_INFINITY);
+
+    jedis.zadd(KEY, map);
+  }
+
+  @Test
+  public void shouldReturnRange_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 0, 2))
+        .containsExactly("f", "e");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 3))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 2, 10))
+        .containsExactly("d", "c", "b");
+    assertThat(jedis.zrevrangeByScore(KEY, "10", "0", 20, 10)).isEmpty();
+  }
+
+  @Test
+  public void shouldReturnRange_withScores_boundedByLimit() {
+    createZSetRangeTestMap();
+
+    Set<Tuple> firstExpected = new LinkedHashSet<>();
+    firstExpected.add(new Tuple("f", 5d));
+    firstExpected.add(new Tuple("e", 4d));
+
+    Set<Tuple> secondExpected = new LinkedHashSet<>();
+    secondExpected.add(new Tuple("d", 3d));
+    secondExpected.add(new Tuple("c", 2d));
+    secondExpected.add(new Tuple("b", 1d));
+
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 0, 0))
+        .isEmpty();
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 0, 2))
+        .containsExactlyElementsOf(firstExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, 3))
+        .containsExactlyElementsOf(secondExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, 10))
+        .containsExactlyElementsOf(secondExpected);
+    assertThat(jedis.zrevrangeByScoreWithScores(KEY, "10", "0", 2, -1))
+        .containsExactlyElementsOf(secondExpected);
+  }
+
+  @Test
+  public void shouldReturnProperError_givenLimitWithWrongFormat() {
+    createZSetRangeTestMap();
+
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "10", "0", "LIMIT"))
+            .hasMessageContaining(ERROR_SYNTAX);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "10", "0", "LIMIT",
+            "0"))
+                .hasMessageContaining(ERROR_SYNTAX);
+    assertThatThrownBy(
+        () -> jedis.sendCommand(KEY, Protocol.Command.ZREVRANGEBYSCORE, KEY, "10", "0",
+            "LIMIT", "0", "invalid"))
+                .hasMessageContaining(ERROR_NOT_INTEGER);
+  }
+
+  @Test
+  public void shouldReturnRange_givenMultipleCopiesOfWithscoresAndOrLimit() {

Review comment:
       Could we get a test of the behaviour for improperly-specified `LIMIT` with multiple `LIMIT`s? Like, have two `LIMIT`s specified, and assert on the expected error when the first one is wrong, and when the second one is wrong, with and without `WITHSCORES`.

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -310,18 +311,62 @@ long zcount(SortedSetRangeOptions rangeOptions) {
     return getRange(min, max, withScores, false);
   }
 
+  List<byte[]> zrevrangebyscore(SortedSetRangeOptions rangeOptions, boolean withScores) {
+    List<byte[]> result = new ArrayList<>();
+    AbstractOrderedSetEntry maxEntry =
+        new DummyOrderedSetEntry(rangeOptions.getStartDouble(), rangeOptions.isStartExclusive(),
+            false);
+    int maxIndex = scoreSet.indexOf(maxEntry);
+
+    AbstractOrderedSetEntry minEntry =
+        new DummyOrderedSetEntry(rangeOptions.getEndDouble(), rangeOptions.isEndExclusive(), true);
+    int minIndex = scoreSet.indexOf(minEntry);
+    if (minIndex > getSortedSetSize()) {
+      return Collections.emptyList();
+    }
+
+    if (minIndex == maxIndex) {
+      return Collections.emptyList();
+    }
+
+    // Okay, if we make it this far there's a potential range of things to return.
+    int count = Integer.MAX_VALUE;
+    if (rangeOptions.hasLimit()) {
+      count = rangeOptions.getCount();
+      maxIndex -= rangeOptions.getOffset();
+      if (maxIndex < 0) {
+        return Collections.emptyList();
+      }
+    }
+    Iterator<AbstractOrderedSetEntry> entryIterator =
+        scoreSet.getIndexRange(maxIndex - 1, Math.min(count, maxIndex - minIndex), true);
+
+    while (entryIterator.hasNext()) {
+      AbstractOrderedSetEntry entry = entryIterator.next();
+      if (rangeOptions.isStartExclusive() && entry.score == rangeOptions.getStartDouble()) {
+        continue;
+      }

Review comment:
       This check is not necessary since the range returned by `getIndexRange()` will by design not contain elements that it shouldn't.




-- 
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: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org