DonalEvans commented on a change in pull request #7227:
URL: https://github.com/apache/geode/pull/7227#discussion_r778443201
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SMoveExecutor.java
##########
@@ -14,47 +14,41 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_TYPE;
-import static org.apache.geode.redis.internal.data.RedisDataType.REDIS_SET;
+import static org.apache.geode.redis.internal.data.RedisSet.smove;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.List;
-import org.apache.geode.cache.Region;
import org.apache.geode.redis.internal.commands.Command;
import org.apache.geode.redis.internal.commands.executor.CommandExecutor;
import org.apache.geode.redis.internal.commands.executor.RedisResponse;
-import org.apache.geode.redis.internal.data.RedisData;
+import org.apache.geode.redis.internal.data.RedisDataMovedException;
import org.apache.geode.redis.internal.data.RedisKey;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+import org.apache.geode.redis.internal.services.RegionProvider;
public class SMoveExecutor implements CommandExecutor {
@Override
public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
List<byte[]> commandElems = command.getProcessedCommand();
- Region<RedisKey, RedisData> region = context.getRegion();
- RedisKey source = command.getKey();
- RedisKey destination = new RedisKey(commandElems.get(2));
- byte[] member = commandElems.get(3);
-
- // TODO: this command should lock both source and destination before
changing them
+ List<RedisKey> commandKeys = command.getProcessedCommandKeys();
+ List<RedisKey> setKeys = commandKeys.subList(1, 3);
- String destinationType = context.dataLockedExecute(destination, false,
RedisData::type);
- if (!destinationType.equals(REDIS_SET.toString()) &&
!destinationType.equals("none")) {
- return RedisResponse.wrongType(ERROR_WRONG_TYPE);
+ byte[] member = commandElems.get(3);
+ RegionProvider regionProvider = context.getRegionProvider();
+ try {
+ for (RedisKey k : setKeys) {
+ regionProvider.ensureKeyIsLocal(k);
Review comment:
The `AbstractRenameExecutor` and `ZStoreExecutor` both explicitly check
that the slots of the two keys are equal, so we should probably be consistent
about that here too. Are there places where we have multi-key commands that
only check the bucketId?
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SMoveExecutor.java
##########
@@ -14,47 +14,41 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_TYPE;
-import static org.apache.geode.redis.internal.data.RedisDataType.REDIS_SET;
+import static org.apache.geode.redis.internal.data.RedisSet.smove;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.List;
-import org.apache.geode.cache.Region;
import org.apache.geode.redis.internal.commands.Command;
import org.apache.geode.redis.internal.commands.executor.CommandExecutor;
import org.apache.geode.redis.internal.commands.executor.RedisResponse;
-import org.apache.geode.redis.internal.data.RedisData;
+import org.apache.geode.redis.internal.data.RedisDataMovedException;
import org.apache.geode.redis.internal.data.RedisKey;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+import org.apache.geode.redis.internal.services.RegionProvider;
public class SMoveExecutor implements CommandExecutor {
@Override
public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
List<byte[]> commandElems = command.getProcessedCommand();
- Region<RedisKey, RedisData> region = context.getRegion();
- RedisKey source = command.getKey();
- RedisKey destination = new RedisKey(commandElems.get(2));
- byte[] member = commandElems.get(3);
-
- // TODO: this command should lock both source and destination before
changing them
+ List<RedisKey> commandKeys = command.getProcessedCommandKeys();
+ List<RedisKey> setKeys = commandKeys.subList(1, 3);
- String destinationType = context.dataLockedExecute(destination, false,
RedisData::type);
- if (!destinationType.equals(REDIS_SET.toString()) &&
!destinationType.equals("none")) {
- return RedisResponse.wrongType(ERROR_WRONG_TYPE);
+ byte[] member = commandElems.get(3);
+ RegionProvider regionProvider = context.getRegionProvider();
+ try {
+ for (RedisKey k : setKeys) {
+ regionProvider.ensureKeyIsLocal(k);
Review comment:
The `AbstractRenameExecutor` and `ZStoreExecutor` both explicitly check
that the slots of the two (or more) keys are equal, so we should probably be
consistent about that here too. Are there places where we have multi-key
commands that only check the bucketId?
--
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]
