This is an automated email from the ASF dual-hosted git repository.
dschneider pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new dec583b GEODE-8387: fix SET*STORE commands handling of empty results
(#5405)
dec583b is described below
commit dec583b9411664c4db55c4ee4651a80cfe8a51a7
Author: Darrel Schneider <[email protected]>
AuthorDate: Fri Jul 31 08:55:30 2020 -0700
GEODE-8387: fix SET*STORE commands handling of empty results (#5405)
---
.../executor/set/SDiffIntegrationTest.java | 22 ++++++++++++++++++++++
.../executor/set/SInterIntegrationTest.java | 22 ++++++++++++++++++++++
.../executor/set/SUnionIntegrationTest.java | 22 ++++++++++++++++++++++
.../geode/redis/internal/data/NullRedisSet.java | 11 ++++++++---
4 files changed, 74 insertions(+), 3 deletions(-)
diff --git
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SDiffIntegrationTest.java
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SDiffIntegrationTest.java
index f721941..9288210 100755
---
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SDiffIntegrationTest.java
+++
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SDiffIntegrationTest.java
@@ -15,6 +15,7 @@
package org.apache.geode.redis.internal.executor.set;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.ArrayList;
import java.util.HashSet;
@@ -124,6 +125,27 @@ public class SDiffIntegrationTest {
}
@Test
+ public void testSDiffStore_withNonExistentKeys() {
+ String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
+ jedis.sadd("set1", firstSet);
+
+ Long resultSize = jedis.sdiffstore("set1", "nonExistent1", "nonExistent2");
+ assertThat(resultSize).isEqualTo(0);
+ assertThat(jedis.exists("set1")).isFalse();
+ }
+
+ @Test
+ public void testSDiffStore_withNonSetKey() {
+ String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
+ jedis.sadd("set1", firstSet);
+ jedis.set("string1", "value1");
+
+ assertThatThrownBy(() -> jedis.sdiffstore("set1", "string1"))
+ .hasMessage("WRONGTYPE Operation against a key holding the wrong kind
of value");
+ assertThat(jedis.exists("set1")).isTrue();
+ }
+
+ @Test
public void testConcurrentSDiffStore() throws InterruptedException {
int ENTRIES = 100;
int SUBSET_SIZE = 100;
diff --git
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SInterIntegrationTest.java
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SInterIntegrationTest.java
index b0e820b..ce09c32 100755
---
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SInterIntegrationTest.java
+++
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SInterIntegrationTest.java
@@ -15,6 +15,7 @@
package org.apache.geode.redis.internal.executor.set;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.ArrayList;
import java.util.HashSet;
@@ -124,6 +125,27 @@ public class SInterIntegrationTest {
}
@Test
+ public void testSInterStore_withNonExistentKeys() {
+ String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
+ jedis.sadd("set1", firstSet);
+
+ Long resultSize = jedis.sinterstore("set1", "nonExistent1",
"nonExistent2");
+ assertThat(resultSize).isEqualTo(0);
+ assertThat(jedis.exists("set1")).isFalse();
+ }
+
+ @Test
+ public void testSInterStore_withNonSetKey() {
+ String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
+ jedis.sadd("set1", firstSet);
+ jedis.set("string1", "value1");
+
+ assertThatThrownBy(() -> jedis.sinterstore("set1", "string1"))
+ .hasMessage("WRONGTYPE Operation against a key holding the wrong kind
of value");
+ assertThat(jedis.exists("set1")).isTrue();
+ }
+
+ @Test
public void testConcurrentSInterStore() throws InterruptedException {
int ENTRIES = 100;
int SUBSET_SIZE = 100;
diff --git
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SUnionIntegrationTest.java
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SUnionIntegrationTest.java
index 4e7a6ed..e48381d 100755
---
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SUnionIntegrationTest.java
+++
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/set/SUnionIntegrationTest.java
@@ -15,6 +15,7 @@
package org.apache.geode.redis.internal.executor.set;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.ArrayList;
import java.util.HashSet;
@@ -116,6 +117,27 @@ public class SUnionIntegrationTest {
}
@Test
+ public void testSUnionStore_withNonExistentKeys() {
+ String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
+ jedis.sadd("set1", firstSet);
+
+ Long resultSize = jedis.sunionstore("set1", "nonExistent1",
"nonExistent2");
+ assertThat(resultSize).isEqualTo(0);
+ assertThat(jedis.exists("set1")).isFalse();
+ }
+
+ @Test
+ public void testSUnionStore_withNonSetKey() {
+ String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
+ jedis.sadd("set1", firstSet);
+ jedis.set("string1", "value1");
+
+ assertThatThrownBy(() -> jedis.sunionstore("set1", "string1"))
+ .hasMessage("WRONGTYPE Operation against a key holding the wrong kind
of value");
+ assertThat(jedis.exists("set1")).isTrue();
+ }
+
+ @Test
public void testConcurrentSUnionStore() throws InterruptedException {
int ENTRIES = 100;
int SUBSET_SIZE = 100;
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java
index 498ebe4..c27b27d 100644
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java
@@ -120,9 +120,14 @@ class NullRedisSet extends RedisSet {
ByteArrayWrapper destination,
ArrayList<Set<ByteArrayWrapper>> nonDestinationSets) {
RedisSet destinationSet = helper.getRedisSet(destination);
- RedisSet redisSet = new RedisSet(computeSetOp(setOp, nonDestinationSets,
destinationSet));
- helper.getRegion().put(destination, redisSet);
- return redisSet.scard();
+ Set<ByteArrayWrapper> result = computeSetOp(setOp, nonDestinationSets,
destinationSet);
+ if (result.isEmpty()) {
+ helper.getRegion().remove(destination);
+ return 0;
+ } else {
+ helper.getRegion().put(destination, new RedisSet(result));
+ return result.size();
+ }
}
private Set<ByteArrayWrapper> computeSetOp(SetOp setOp,
