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]