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



##########
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java
##########
@@ -41,12 +43,36 @@ public RedisResponse executeCommand(Command command, 
ExecutionHandlerContext con
 
     List<RedisKey> commandElements = command.getProcessedCommandKeys();
     List<RedisKey> setKeys = commandElements.subList(setsStartIndex, 
commandElements.size());
-    if (isStorage()) {
-      RedisKey destination = command.getKey();
-      int storeCount = doStoreSetOp(command.getCommandType(), context, 
destination, setKeys);
-      return RedisResponse.integer(storeCount);
+    RegionProvider regionProvider = context.getRegionProvider();
+    try {
+      for (RedisKey k : setKeys) {
+        regionProvider.ensureKeyIsLocal(k);
+      }
+    } catch (RedisDataMovedException ex) {
+      return RedisResponse.error(ex.getMessage());
+    }
+
+    /*
+     * SDIFFSTORE, SINTER, SINTERSTORE, SUNION, SUNIONSTORE currently use the 
else part of the code
+     * for their
+     * implementation.
+     * TODO: Once the above commands have been implemented remove the if else 
and
+     * refactor so it implements doSetOp
+     */
+
+    if (command.isOfType(RedisCommandType.SDIFF)) {
+      Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new 
ArrayList<>(setKeys),

Review comment:
       The `lockedExecute()` method sorts the list passed to it, which results 
in the order of the keys in `setKeys` changing if it's passed in. Since we also 
pass `setKeys` into the `sdiff()` method, this results in an incorrect result 
for the diff, hence the need to pass a copy to one of the methods.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to