This is an automated email from the ASF dual-hosted git repository.
jensdeppe 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 4ac6c47 GEODE-8624: Support Redis HINCRBYFLOAT command (#5986)
4ac6c47 is described below
commit 4ac6c47555d93263e94d8d85431bbe37dcaa32e1
Author: Jens Deppe <jde...@pivotal.io>
AuthorDate: Thu Feb 4 05:50:04 2021 -0800
GEODE-8624: Support Redis HINCRBYFLOAT command (#5986)
- Switch to using BigDecimal instead of double for increment value.
- Improve HINCRBYFLOAT output accuracy for very large values.
- Add concurrency tests.
- Mark HINCRBYFLOAT as supported.
---
.../hash/HincrByFloatNativeRedisAccetanceTest.java | 32 ++++
.../hash/AbstractHashesIntegrationTest.java | 57 ------
.../hash/AbstractHincrByFloatIntegrationTest.java | 202 +++++++++++++++++++++
.../executor/hash/HincrByFloatIntegrationTest.java | 32 ++++
.../geode/redis/internal/RedisCommandType.java | 2 +-
.../geode/redis/internal/data/NullRedisHash.java | 7 +-
.../geode/redis/internal/data/RedisHash.java | 20 +-
.../data/RedisHashCommandsFunctionExecutor.java | 4 +-
.../redis/internal/executor/CommandFunction.java | 2 +-
.../executor/hash/HIncrByFloatExecutor.java | 28 +--
.../internal/executor/hash/RedisHashCommands.java | 3 +-
.../hash/RedisHashCommandsFunctionInvoker.java | 4 +-
.../executor/string/IncrByFloatExecutor.java | 23 ++-
.../redis/internal/SupportedCommandsJUnitTest.java | 8 +-
14 files changed, 326 insertions(+), 98 deletions(-)
diff --git
a/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/HincrByFloatNativeRedisAccetanceTest.java
b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/HincrByFloatNativeRedisAccetanceTest.java
new file mode 100644
index 0000000..6ca6f0a
--- /dev/null
+++
b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/hash/HincrByFloatNativeRedisAccetanceTest.java
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis.internal.executor.hash;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.NativeRedisTestRule;
+
+public class HincrByFloatNativeRedisAccetanceTest extends
AbstractHincrByFloatIntegrationTest {
+
+ @ClassRule
+ public static NativeRedisTestRule redis = new NativeRedisTestRule();
+
+ @Override
+ public int getPort() {
+ return redis.getPort();
+ }
+
+}
diff --git
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
index facd18b..7bd6623 100755
---
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
+++
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHashesIntegrationTest.java
@@ -16,9 +16,7 @@ package org.apache.geode.redis.internal.executor.hash;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.assertj.core.api.Assertions.offset;
-import java.text.DecimalFormat;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -296,46 +294,6 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
}
@Test
- public void testHIncrFloatBy() {
- String key = "key";
- String field = "field";
-
- DecimalFormat decimalFormat = new DecimalFormat("#.#####");
- double incr = rand.nextDouble();
- String incrAsString = decimalFormat.format(incr);
- incr = Double.valueOf(incrAsString);
- if (incr == 0) {
- incr = incr + 1;
- }
-
- Double response1 = jedis.hincrByFloat(key, field, incr);
- assertThat(response1).isEqualTo(incr, offset(.00001));
-
- assertThat(response1).isEqualTo(Double.valueOf(jedis.hget(key, field)),
offset(.00001));
-
- double response2 = jedis.hincrByFloat("new", "newField", incr);
-
- assertThat(response2).isEqualTo(incr, offset(.00001));
-
- Double response3 = jedis.hincrByFloat(key, field, incr);
- assertThat(response3).isEqualTo(2 * incr, offset(.00001));
-
- assertThat(response3).isEqualTo(Double.valueOf(jedis.hget(key, field)),
offset(.00001));
-
- }
-
- @Test
- public void incrByFloatFailsWithNonFloatFieldValue() {
- String key = "key";
- String field = "field";
- jedis.hset(key, field, "foobar");
- assertThatThrownBy(() -> {
- jedis.hincrByFloat(key, field, 1.5);
- }).isInstanceOf(JedisDataException.class)
- .hasMessageContaining("ERR hash value is not a float");
- }
-
- @Test
public void testHExists_isFalseForNonexistentKeyOrField() {
jedis.hset("key", "field", "value");
@@ -776,21 +734,6 @@ public abstract class AbstractHashesIntegrationTest
implements RedisPortSupplier
}
@Test
- public void testConcurrentHIncrByFloat_sameKeyPerClient() {
- String key = "HSET_KEY";
- String field = "HSET_FIELD";
-
- jedis.hset(key, field, "0");
-
- new ConcurrentLoopingThreads(ITERATION_COUNT,
- (i) -> jedis.hincrByFloat(key, field, 0.5),
- (i) -> jedis2.hincrByFloat(key, field, 1.0)).run();
-
- String value = jedis.hget(key, field);
- assertThat(value).isEqualTo(String.format("%.0f", ITERATION_COUNT * 1.5));
- }
-
- @Test
public void testHSet_keyExistsWithDifferentDataType() {
jedis.set("key", "value");
diff --git
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHincrByFloatIntegrationTest.java
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHincrByFloatIntegrationTest.java
new file mode 100644
index 0000000..d8f0ce6
--- /dev/null
+++
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHincrByFloatIntegrationTest.java
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis.internal.executor.hash;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.offset;
+
+import java.math.BigDecimal;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.Protocol;
+import redis.clients.jedis.exceptions.JedisDataException;
+
+import org.apache.geode.redis.ConcurrentLoopingThreads;
+import org.apache.geode.test.dunit.rules.RedisPortSupplier;
+
+public abstract class AbstractHincrByFloatIntegrationTest implements
RedisPortSupplier {
+
+ private Jedis jedis;
+ private Jedis jedis2;
+ private static int ITERATION_COUNT = 4000;
+
+ @Before
+ public void setUp() {
+ jedis = new Jedis("localhost", getPort(), 10000000);
+ jedis2 = new Jedis("localhost", getPort(), 10000000);
+ }
+
+ @After
+ public void flushAll() {
+ jedis.flushAll();
+ }
+
+ @After
+ public void tearDown() {
+ jedis.close();
+ jedis2.close();
+ }
+
+ @Test
+ public void givenKeyNotProvided_returnsWrongNumberOfArgumentsError() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT))
+ .hasMessageContaining("ERR wrong number of arguments for
'hincrbyfloat' command");
+ }
+
+ @Test
+ public void givenHashNotProvided_returnsWrongNumberOfArgumentsError() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT,
"key"))
+ .hasMessageContaining("ERR wrong number of arguments for
'hincrbyfloat' command");
+ }
+
+ @Test
+ public void givenIncrementNotProvided_returnsWrongNumberOfArgumentsError() {
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT,
"key", "hash"))
+ .hasMessageContaining("ERR wrong number of arguments for
'hincrbyfloat' command");
+ }
+
+ @Test
+ public void
givenMoreThanFourArgumentsProvided_returnsWrongNumberOfArgumentsError() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key", "hash",
"5", "extraArg"))
+ .hasMessageContaining("ERR wrong number of arguments for
'hincrbyfloat' command");
+ }
+
+ @Test
+ public void testHincrByFloat_withInfinityAndVariants() {
+ jedis.hset("key", "number", "1.4");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "+inf"))
+ .hasMessage("ERR increment would produce NaN or Infinity");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "-inf"))
+ .hasMessage("ERR increment would produce NaN or Infinity");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "inf"))
+ .hasMessage("ERR increment would produce NaN or Infinity");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "+infinity"))
+ .hasMessage("ERR increment would produce NaN or Infinity");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "-infinity"))
+ .hasMessage("ERR increment would produce NaN or Infinity");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "infinity"))
+ .hasMessage("ERR increment would produce NaN or Infinity");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "nan"))
+ .hasMessage("ERR value is not a valid float");
+
+ assertThatThrownBy(
+ () -> jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "infant"))
+ .hasMessage("ERR value is not a valid float");
+ }
+
+ @Test
+ public void testHincrByFloat() {
+ double incr = 0.37373;
+ String key = "key";
+ String field = "field";
+ jedis.hset(key, field, "1.0");
+
+ double response = jedis.hincrByFloat(key, field, incr);
+ assertThat(response).isEqualTo(incr + 1, offset(.00001));
+ }
+
+ @Test
+ public void testHincrByFloat_likeRedisDoesIt() {
+ String key = "key";
+ String field = "field";
+
+ Object response = jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, key,
field, "1.23");
+ assertThat(new String((byte[]) response)).isEqualTo("1.23");
+
+ response = jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, key, field,
"0.77");
+ assertThat(new String((byte[]) response)).isEqualTo("2");
+
+ response = jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, key, field,
"-0.1");
+ assertThat(new String((byte[]) response)).isEqualTo("1.9");
+ }
+
+ @Test
+ public void testHincrByFloat_whenFieldDoesNotExist() {
+ double incr = 0.37373;
+ String key = "key";
+ String field = "field";
+ jedis.hset(key, "other-field", "a");
+
+ double response = jedis.hincrByFloat(key, field, incr);
+ assertThat(response).isEqualTo(incr);
+ }
+
+ @Test
+ public void testHincrByFloat_whenKeyDoesNotExist() {
+ double incr = 0.37373;
+
+ double response = jedis.hincrByFloat("new", "newField", incr);
+ assertThat(response).isEqualTo(incr);
+ }
+
+ @Test
+ public void testIncrByFloat_withReallyBigNumbers() {
+ // max unsigned long long - 1
+ BigDecimal biggy = new BigDecimal("18446744073709551614");
+ jedis.hset("key", "number", biggy.toPlainString());
+
+ // Beyond this, native redis produces inconsistent results.
+ Object rawResult = jedis.sendCommand(Protocol.Command.HINCRBYFLOAT, "key",
"number", "1");
+ BigDecimal result = new BigDecimal(new String((byte[]) rawResult));
+
+
assertThat(result.toPlainString()).isEqualTo(biggy.add(BigDecimal.ONE).toPlainString());
+ }
+
+ @Test
+ public void hincrByFloatFails_whenFieldIsNotANumber() {
+ String key = "key";
+ String field = "field";
+ jedis.hset(key, field, "foobar");
+ assertThatThrownBy(() -> jedis.hincrByFloat(key, field, 1.5))
+ .isInstanceOf(JedisDataException.class)
+ .hasMessageContaining("ERR hash value is not a float");
+ }
+
+ @Test
+ public void testConcurrentHincrByFloat_sameKeyPerClient() {
+ String key = "HSET_KEY";
+ String field = "HSET_FIELD";
+
+ jedis.hset(key, field, "0");
+
+ new ConcurrentLoopingThreads(ITERATION_COUNT,
+ (i) -> jedis.hincrByFloat(key, field, 0.5),
+ (i) -> jedis2.hincrByFloat(key, field, 1.0)).run();
+
+ String value = jedis.hget(key, field);
+ assertThat(Float.valueOf(value)).isEqualTo(ITERATION_COUNT * 1.5f);
+ }
+
+}
diff --git
a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/HincrByFloatIntegrationTest.java
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/HincrByFloatIntegrationTest.java
new file mode 100644
index 0000000..fffd4d3
--- /dev/null
+++
b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/HincrByFloatIntegrationTest.java
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis.internal.executor.hash;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.GeodeRedisServerRule;
+
+public class HincrByFloatIntegrationTest extends
AbstractHincrByFloatIntegrationTest {
+
+ @ClassRule
+ public static GeodeRedisServerRule server = new GeodeRedisServerRule();
+
+ @Override
+ public int getPort() {
+ return server.getPort();
+ }
+
+}
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
index c3b5568..a884873 100755
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/RedisCommandType.java
@@ -158,6 +158,7 @@ public enum RedisCommandType {
/************* Hashes *****************/
HGETALL(new HGetAllExecutor(), SUPPORTED, new ExactParameterRequirements(2)),
+ HINCRBYFLOAT(new HIncrByFloatExecutor(), SUPPORTED, new
ExactParameterRequirements(4)),
HMSET(new HMSetExecutor(), SUPPORTED,
new MinimumParameterRequirements(4).and(new
EvenParameterRequirements())),
HGET(new HGetExecutor(), SUPPORTED, new ExactParameterRequirements(3)),
@@ -240,7 +241,6 @@ public enum RedisCommandType {
HDEL(new HDelExecutor(), UNSUPPORTED, new MinimumParameterRequirements(3)),
HINCRBY(new HIncrByExecutor(), UNSUPPORTED, new
ExactParameterRequirements(4)),
- HINCRBYFLOAT(new HIncrByFloatExecutor(), UNSUPPORTED, new
ExactParameterRequirements(4)),
HKEYS(new HKeysExecutor(), UNSUPPORTED, new ExactParameterRequirements(2)),
HSCAN(new HScanExecutor(), UNSUPPORTED, new MinimumParameterRequirements(3),
new OddParameterRequirements(ERROR_SYNTAX)),
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java
index a960f4c..4e20ad9 100644
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java
@@ -18,6 +18,7 @@ package org.apache.geode.redis.internal.data;
import static java.util.Collections.emptyList;
+import java.math.BigDecimal;
import java.util.Arrays;
import java.util.List;
@@ -51,11 +52,11 @@ public class NullRedisHash extends RedisHash {
}
@Override
- public double hincrbyfloat(Region<ByteArrayWrapper, RedisData> region,
ByteArrayWrapper key,
- ByteArrayWrapper field, double increment) throws NumberFormatException {
+ public BigDecimal hincrbyfloat(Region<ByteArrayWrapper, RedisData> region,
ByteArrayWrapper key,
+ ByteArrayWrapper field, BigDecimal increment) throws
NumberFormatException {
region.put(key,
new RedisHash(
- Arrays.asList(field, new
ByteArrayWrapper(Coder.doubleToBytes(increment)))));
+ Arrays.asList(field, new
ByteArrayWrapper(Coder.bigDecimalToBytes(increment)))));
return increment;
}
}
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java
index bb11b34..66a3b68 100644
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java
@@ -22,6 +22,7 @@ import static
org.apache.geode.redis.internal.RedisConstants.ERROR_OVERFLOW;
import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
+import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collection;
@@ -278,39 +279,40 @@ public class RedisHash extends AbstractRedisData {
return value;
}
- public double hincrbyfloat(Region<ByteArrayWrapper, RedisData> region,
ByteArrayWrapper key,
- ByteArrayWrapper field, double increment) throws NumberFormatException {
+ public BigDecimal hincrbyfloat(Region<ByteArrayWrapper, RedisData> region,
ByteArrayWrapper key,
+ ByteArrayWrapper field, BigDecimal increment) throws
NumberFormatException {
ByteArrayWrapper oldValue = hash.get(field);
if (oldValue == null) {
- ByteArrayWrapper newValue = new
ByteArrayWrapper(Coder.doubleToBytes(increment));
+ ByteArrayWrapper newValue = new
ByteArrayWrapper(Coder.bigDecimalToBytes(increment));
hashPut(field, newValue);
AddsDeltaInfo deltaInfo = new AddsDeltaInfo();
deltaInfo.add(field);
deltaInfo.add(newValue);
storeChanges(region, key, deltaInfo);
- return increment;
+ return increment.stripTrailingZeros();
}
String valueS = oldValue.toString();
if (valueS.contains(" ")) {
throw new NumberFormatException("hash value is not a float");
}
- double value;
+
+ BigDecimal value;
try {
- value = Coder.stringToDouble(valueS);
+ value = new BigDecimal(valueS);
} catch (NumberFormatException ex) {
throw new NumberFormatException("hash value is not a float");
}
- value += increment;
+ value = value.add(increment);
- ByteArrayWrapper modifiedValue = new
ByteArrayWrapper(Coder.doubleToBytes(value));
+ ByteArrayWrapper modifiedValue = new
ByteArrayWrapper(Coder.bigDecimalToBytes(value));
hashPut(field, modifiedValue);
AddsDeltaInfo deltaInfo = new AddsDeltaInfo();
deltaInfo.add(field);
deltaInfo.add(modifiedValue);
storeChanges(region, key, deltaInfo);
- return value;
+ return value.stripTrailingZeros();
}
@Override
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java
index c621034..b8e8682 100644
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java
@@ -16,6 +16,7 @@
package org.apache.geode.redis.internal.data;
+import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Collection;
import java.util.List;
@@ -107,7 +108,8 @@ public class RedisHashCommandsFunctionExecutor extends
RedisDataCommandsFunction
}
@Override
- public double hincrbyfloat(ByteArrayWrapper key, ByteArrayWrapper field,
double increment) {
+ public BigDecimal hincrbyfloat(ByteArrayWrapper key, ByteArrayWrapper field,
+ BigDecimal increment) {
return stripedExecute(key,
() -> getRedisHash(key, false)
.hincrbyfloat(getRegion(), key, field, increment));
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java
index 81b5ef1..5d4bacf 100644
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java
@@ -267,7 +267,7 @@ public class CommandFunction extends
SingleResultRedisFunction {
}
case HINCRBYFLOAT: {
ByteArrayWrapper field = (ByteArrayWrapper) args[1];
- double increment = (double) args[2];
+ BigDecimal increment = (BigDecimal) args[2];
return hashCommands.hincrbyfloat(key, field, increment);
}
default:
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java
index bb933dd..455683f 100755
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java
@@ -14,11 +14,15 @@
*/
package org.apache.geode.redis.internal.executor.hash;
+
+import java.math.BigDecimal;
import java.util.List;
+import org.apache.commons.lang3.tuple.Pair;
+
import org.apache.geode.redis.internal.data.ByteArrayWrapper;
import org.apache.geode.redis.internal.executor.RedisResponse;
-import org.apache.geode.redis.internal.netty.Coder;
+import org.apache.geode.redis.internal.executor.string.IncrByFloatExecutor;
import org.apache.geode.redis.internal.netty.Command;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
@@ -46,31 +50,27 @@ import
org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
*/
public class HIncrByFloatExecutor extends HashExecutor {
- private static final String ERROR_INCREMENT_NOT_USABLE =
- "The increment on this key must be floating point numeric";
-
private static final int INCREMENT_INDEX = FIELD_INDEX + 1;
@Override
public RedisResponse executeCommand(Command command,
ExecutionHandlerContext context) {
List<byte[]> commandElems = command.getProcessedCommand();
- byte[] byteField = commandElems.get(FIELD_INDEX);
- ByteArrayWrapper field = new ByteArrayWrapper(byteField);
- byte[] incrArray = commandElems.get(INCREMENT_INDEX);
- double increment;
- try {
- increment = Coder.bytesToDouble(incrArray);
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_INCREMENT_NOT_USABLE);
+ Pair<BigDecimal, RedisResponse> validated =
+
IncrByFloatExecutor.validateIncrByFloatArgument(commandElems.get(INCREMENT_INDEX));
+ if (validated.getRight() != null) {
+ return validated.getRight();
}
ByteArrayWrapper key = command.getKey();
RedisHashCommands redisHashCommands = createRedisHashCommands(context);
+ byte[] byteField = commandElems.get(FIELD_INDEX);
+ ByteArrayWrapper field = new ByteArrayWrapper(byteField);
+
+ BigDecimal value = redisHashCommands.hincrbyfloat(key, field,
validated.getLeft());
- double value = redisHashCommands.hincrbyfloat(key, field, increment);
- return RedisResponse.bulkString(value);
+ return RedisResponse.bigDecimal(value);
}
}
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java
index db99caa..b51cb8b 100644
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java
@@ -15,6 +15,7 @@
package org.apache.geode.redis.internal.executor.hash;
+import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Collection;
import java.util.List;
@@ -50,5 +51,5 @@ public interface RedisHashCommands {
long hincrby(ByteArrayWrapper key, ByteArrayWrapper field, long increment);
- double hincrbyfloat(ByteArrayWrapper key, ByteArrayWrapper field, double
increment);
+ BigDecimal hincrbyfloat(ByteArrayWrapper key, ByteArrayWrapper field,
BigDecimal increment);
}
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java
index 571dcad..a97b7e7 100644
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java
@@ -29,6 +29,7 @@ import static
org.apache.geode.redis.internal.RedisCommandType.HSET;
import static org.apache.geode.redis.internal.RedisCommandType.HSTRLEN;
import static org.apache.geode.redis.internal.RedisCommandType.HVALS;
+import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Collection;
import java.util.List;
@@ -116,7 +117,8 @@ public class RedisHashCommandsFunctionInvoker extends
RedisCommandsFunctionInvok
}
@Override
- public double hincrbyfloat(ByteArrayWrapper key, ByteArrayWrapper field,
double increment) {
+ public BigDecimal hincrbyfloat(ByteArrayWrapper key, ByteArrayWrapper field,
+ BigDecimal increment) {
return invokeCommandFunction(key, HINCRBYFLOAT, field, increment);
}
}
diff --git
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/IncrByFloatExecutor.java
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/IncrByFloatExecutor.java
index 79995f2..470d93f 100755
---
a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/IncrByFloatExecutor.java
+++
b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/IncrByFloatExecutor.java
@@ -18,6 +18,8 @@ import java.math.BigDecimal;
import java.util.List;
import java.util.regex.Pattern;
+import org.apache.commons.lang3.tuple.Pair;
+
import org.apache.geode.redis.internal.RedisConstants;
import org.apache.geode.redis.internal.data.ByteArrayWrapper;
import org.apache.geode.redis.internal.executor.RedisResponse;
@@ -36,23 +38,32 @@ public class IncrByFloatExecutor extends StringExecutor {
public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
List<byte[]> commandElems = command.getProcessedCommand();
ByteArrayWrapper key = command.getKey();
+
+ Pair<BigDecimal, RedisResponse> validated =
+ validateIncrByFloatArgument(commandElems.get(INCREMENT_INDEX));
+ if (validated.getRight() != null) {
+ return validated.getRight();
+ }
+
RedisStringCommands stringCommands = getRedisStringCommands(context);
+ BigDecimal result = stringCommands.incrbyfloat(key, validated.getLeft());
- byte[] incrArray = commandElems.get(INCREMENT_INDEX);
+ return RedisResponse.bigDecimal(result);
+ }
+
+ public static Pair<BigDecimal, RedisResponse>
validateIncrByFloatArgument(byte[] incrArray) {
String doub = Coder.bytesToString(incrArray).toLowerCase();
if (invalidArgs.matcher(doub).matches()) {
- return RedisResponse.error(RedisConstants.ERROR_NAN_OR_INFINITY);
+ return Pair.of(null,
RedisResponse.error(RedisConstants.ERROR_NAN_OR_INFINITY));
}
BigDecimal increment;
try {
increment = Coder.bytesToBigDecimal(incrArray);
} catch (NumberFormatException e) {
- return RedisResponse.error(RedisConstants.ERROR_NOT_A_VALID_FLOAT);
+ return Pair.of(null,
RedisResponse.error(RedisConstants.ERROR_NOT_A_VALID_FLOAT));
}
- BigDecimal result = stringCommands.incrbyfloat(key, increment);
-
- return RedisResponse.bigDecimal(result);
+ return Pair.of(increment, null);
}
}
diff --git
a/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
b/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
index 5e2e3b9..d18d605 100644
---
a/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
+++
b/geode-redis/src/test/java/org/apache/geode/redis/internal/SupportedCommandsJUnitTest.java
@@ -37,13 +37,14 @@ public class SupportedCommandsJUnitTest {
"EXPIRE",
"EXPIREAT",
"GET",
+ "HGET",
"HGETALL",
+ "HINCRBYFLOAT",
+ "HLEN",
+ "HMGET",
"HMSET",
"HSET",
"HSETNX",
- "HLEN",
- "HGET",
- "HMGET",
"HSTRLEN",
"HVALS",
"HEXISTS",
@@ -85,7 +86,6 @@ public class SupportedCommandsJUnitTest {
"GETSET",
"HDEL",
"HINCRBY",
- "HINCRBYFLOAT",
"HKEYS",
"HSCAN",
"INCR",
