GEODE-9582: make radish pattern match compatible with redis

GlobPattern no longer uses jdk regex but instead has a java implementation based on the native redis glob pattern matcher. @jdeppe-pivotal ran some basic JMH tests against a simple sometext* pattern (which is probably very common) and the new code appears to be slightly faster than the old jdk code. GlobPattern now also has a unit test.
apache · Sep 18, 2021 · c68a770 · c68a770
1 parent 78a481d
commit c68a770
Show file tree

Hide file tree

Showing 28 changed files with 299 additions and 242 deletions.
diff --git a/...est/java/org/apache/geode/redis/internal/executor/AbstractGlobPatternIntegrationTest.java b/...est/java/org/apache/geode/redis/internal/executor/AbstractGlobPatternIntegrationTest.java
@@ -252,4 +252,41 @@ public void escapedBackslash_matchesVerbatim() {
 
     assertThat(jedis.keys("\\\\*")).containsExactlyInAnyOrder("\\", "\\!");
   }
+
+  @Test
+  public void escapedC_matchesVerbatim() {
+    jedis.set("\\C", "value");
+    jedis.set("\\Ca", "value");
+    jedis.set("b\\C", "value");
+
+    assertThat(jedis.keys("\\\\C")).containsExactlyInAnyOrder("\\C");
+  }
+
+  @Test
+  public void patternMatchIsCaseSensitive() {
+    jedis.set("a", "value");
+    jedis.set("A", "value");
+
+    assertThat(jedis.keys("A")).containsExactlyInAnyOrder("A");
+    assertThat(jedis.keys("a")).containsExactlyInAnyOrder("a");
+    assertThat(jedis.keys("[a-z]")).containsExactlyInAnyOrder("a");
+    assertThat(jedis.keys("[Z-A]")).containsExactlyInAnyOrder("A");
+    assertThat(jedis.keys("[aA]")).containsExactlyInAnyOrder("A", "a");
+  }
+
+  @Test
+  public void doubleQuote_matchesVerbatim() {
+    String key = "\"quotedKey\"";
+    jedis.set(key, "value");
+    assertThat(jedis.keys("quotedKey")).isEmpty();
+    assertThat(jedis.keys("\"*\"")).containsExactlyInAnyOrder(key);
+  }
+
+  @Test
+  public void embeddedWildCardMatches() {
+    jedis.set("foobar", "value");
+    jedis.set("foo123bar", "value");
+    jedis.set("foo123bar!", "value");
+    assertThat(jedis.keys("foo******bar")).containsExactlyInAnyOrder("foobar", "foo123bar");
+  }
 }
diff --git a/...s-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java b/...s-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java
@@ -33,7 +33,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.tuple.ImmutablePair;
 
@@ -233,7 +232,8 @@ public Collection<byte[]> hkeys() {
     return result;
   }
 
-  public ImmutablePair<Integer, List<byte[]>> hscan(Pattern matchPattern, int count, int cursor) {
+  public ImmutablePair<Integer, List<byte[]>> hscan(GlobPattern matchPattern, int count,
+      int cursor) {
     // No need to allocate more space than it's possible to use given the size of the hash. We need
     // to add 1 to hlen() to ensure that if count > hash.size(), we return a cursor of 0
     long maximumCapacity = 2L * Math.min(count, hlen() + 1);
@@ -251,10 +251,10 @@ public ImmutablePair<Integer, List<byte[]>> hscan(Pattern matchPattern, int coun
     return new ImmutablePair<>(cursor, resultList);
   }
 
-  private void addIfMatching(Pattern matchPattern, List<byte[]> resultList, byte[] key,
+  private void addIfMatching(GlobPattern matchPattern, List<byte[]> resultList, byte[] key,
       byte[] value) {
     if (matchPattern != null) {
-      if (GlobPattern.matches(matchPattern, key)) {
+      if (matchPattern.matches(key)) {
         resultList.add(key);
         resultList.add(value);
       }

diff --git a/...src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java b/...src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java
@@ -19,11 +19,11 @@
 import java.math.BigDecimal;
 import java.util.Collection;
 import java.util.List;
-import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.tuple.Pair;
 
 import org.apache.geode.redis.internal.RegionProvider;
+import org.apache.geode.redis.internal.executor.GlobPattern;
 import org.apache.geode.redis.internal.executor.hash.RedisHashCommands;
 
 public class RedisHashCommandsFunctionExecutor extends RedisDataCommandsFunctionExecutor implements
@@ -92,7 +92,7 @@ public Collection<byte[]> hkeys(RedisKey key) {
   }
 
   @Override
-  public Pair<Integer, List<byte[]>> hscan(RedisKey key, Pattern matchPattern, int count,
+  public Pair<Integer, List<byte[]>> hscan(RedisKey key, GlobPattern matchPattern, int count,
       int cursor) {
     return stripedExecute(key,
         () -> getRedisHash(key, true)

diff --git a/...is-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java b/...is-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
@@ -32,7 +32,6 @@
 import java.util.Objects;
 import java.util.Random;
 import java.util.Set;
-import java.util.regex.Pattern;
 
 import it.unimi.dsi.fastutil.bytes.ByteArrays;
 import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -67,7 +66,7 @@ public class RedisSet extends AbstractRedisData {
    */
   public RedisSet() {}
 
-  Pair<BigInteger, List<Object>> sscan(Pattern matchPattern, int count, BigInteger cursor) {
+  Pair<BigInteger, List<Object>> sscan(GlobPattern matchPattern, int count, BigInteger cursor) {
     List<Object> returnList = new ArrayList<>();
     int size = members.size();
     BigInteger beforeCursor = new BigInteger("0");
@@ -81,7 +80,7 @@ Pair<BigInteger, List<Object>> sscan(Pattern matchPattern, int count, BigInteger
       }
 
       if (matchPattern != null) {
-        if (GlobPattern.matches(matchPattern, value)) {
+        if (matchPattern.matches(value)) {
           returnList.add(value);
           numElements++;
         }

diff --git a/.../src/main/java/org/apache/geode/redis/internal/data/RedisSetCommandsFunctionExecutor.java b/.../src/main/java/org/apache/geode/redis/internal/data/RedisSetCommandsFunctionExecutor.java
@@ -24,13 +24,13 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.regex.Pattern;
 
 import it.unimi.dsi.fastutil.bytes.ByteArrays;
 import it.unimi.dsi.fastutil.objects.ObjectOpenCustomHashSet;
 import org.apache.commons.lang3.tuple.Pair;
 
 import org.apache.geode.redis.internal.RegionProvider;
+import org.apache.geode.redis.internal.executor.GlobPattern;
 import org.apache.geode.redis.internal.executor.set.RedisSetCommands;
 
 public class RedisSetCommandsFunctionExecutor extends RedisDataCommandsFunctionExecutor implements
@@ -104,7 +104,7 @@ public Collection<byte[]> spop(RedisKey key, int popCount) {
   }
 
   @Override
-  public Pair<BigInteger, List<Object>> sscan(RedisKey key, Pattern matchPattern, int count,
+  public Pair<BigInteger, List<Object>> sscan(RedisKey key, GlobPattern matchPattern, int count,
       BigInteger cursor) {
     return stripedExecute(key, () -> getRedisSet(key, true).sscan(matchPattern, count, cursor));
   }

diff --git a/...patible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java b/...patible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
@@ -20,7 +20,6 @@
 import static org.apache.geode.internal.JvmSizeUtils.memoryOverhead;
 import static org.apache.geode.redis.internal.data.NullRedisDataStructures.NULL_REDIS_SORTED_SET;
 import static org.apache.geode.redis.internal.data.RedisDataType.REDIS_SORTED_SET;
-import static org.apache.geode.redis.internal.netty.Coder.bytesToString;
 import static org.apache.geode.redis.internal.netty.Coder.doubleToBytes;
 import static org.apache.geode.redis.internal.netty.StringBytesGlossary.bGREATEST_MEMBER_NAME;
 import static org.apache.geode.redis.internal.netty.StringBytesGlossary.bLEAST_MEMBER_NAME;
@@ -34,7 +33,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.logging.log4j.Logger;
@@ -54,6 +52,7 @@
 import org.apache.geode.redis.internal.delta.DeltaInfo;
 import org.apache.geode.redis.internal.delta.RemsDeltaInfo;
 import org.apache.geode.redis.internal.delta.ZAddsDeltaInfo;
+import org.apache.geode.redis.internal.executor.GlobPattern;
 import org.apache.geode.redis.internal.executor.sortedset.AbstractSortedSetRangeOptions;
 import org.apache.geode.redis.internal.executor.sortedset.SortedSetLexRangeOptions;
 import org.apache.geode.redis.internal.executor.sortedset.SortedSetRankRangeOptions;
@@ -388,7 +387,7 @@ long zrevrank(byte[] member) {
     return scoreSet.size() - scoreSet.indexOf(orderedSetEntry) - 1;
   }
 
-  ImmutablePair<Integer, List<byte[]>> zscan(Pattern matchPattern, int count, int cursor) {
+  ImmutablePair<Integer, List<byte[]>> zscan(GlobPattern matchPattern, int count, int cursor) {
     // No need to allocate more space than it's possible to use given the size of the sorted set. We
     // need to add 1 to zcard() to ensure that if count > members.size(), we return a cursor of 0
     long maximumCapacity = 2L * Math.min(count, zcard() + 1);
@@ -595,10 +594,10 @@ private List<byte[]> getElementsFromSet(AbstractSortedSetRangeOptions<?> rangeOp
     return result;
   }
 
-  private void addIfMatching(Pattern matchPattern, List<byte[]> resultList, byte[] key,
+  private void addIfMatching(GlobPattern matchPattern, List<byte[]> resultList, byte[] key,
       double score) {
     if (matchPattern != null) {
-      if (matchPattern.matcher(bytesToString(key)).matches()) {
+      if (matchPattern.matches(key)) {
         resultList.add(key);
         resultList.add(doubleToBytes(score));
       }

diff --git a/...ain/java/org/apache/geode/redis/internal/data/RedisSortedSetCommandsFunctionExecutor.java b/...ain/java/org/apache/geode/redis/internal/data/RedisSortedSetCommandsFunctionExecutor.java
@@ -20,11 +20,11 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.tuple.Pair;
 
 import org.apache.geode.redis.internal.RegionProvider;
+import org.apache.geode.redis.internal.executor.GlobPattern;
 import org.apache.geode.redis.internal.executor.sortedset.RedisSortedSetCommands;
 import org.apache.geode.redis.internal.executor.sortedset.SortedSetLexRangeOptions;
 import org.apache.geode.redis.internal.executor.sortedset.SortedSetRankRangeOptions;
@@ -150,7 +150,7 @@ public long zrevrank(RedisKey key, byte[] member) {
   }
 
   @Override
-  public Pair<Integer, List<byte[]>> zscan(RedisKey key, Pattern matchPattern, int count,
+  public Pair<Integer, List<byte[]>> zscan(RedisKey key, GlobPattern matchPattern, int count,
       int cursor) {
     return stripedExecute(key,
         () -> getRedisSortedSet(key, true).zscan(matchPattern, count, cursor));