jdeppe-pivotal commented on a change in pull request #6783:
URL: https://github.com/apache/geode/pull/6783#discussion_r705609775
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/AbstractRedisData.java
##########
@@ -200,6 +202,15 @@ public void fromDelta(DataInput in) throws IOException,
InvalidDeltaException {
byte[] byteArray = DataSerializer.readByteArray(in);
applyDelta(new AppendDeltaInfo(byteArray, sequence));
break;
+ case ZADDS:
+ int numMembers = DataSerializer.readPrimitiveInt(in);
+ List<byte[]> members = new ArrayList<>();
+ List<Double> scores = new ArrayList<>();
Review comment:
You should use `numMembers` to initialize the array sizes here.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -160,40 +152,44 @@ public int getDSFID() {
return REDIS_SORTED_SET_ID;
}
- protected synchronized byte[] memberAdd(byte[] memberToAdd, byte[]
scoreToAdd) {
- OrderedSetEntry existingEntry = members.get(memberToAdd);
- if (existingEntry == null) {
+ protected synchronized boolean memberAdd(byte[] memberToAdd, double
scoreToAdd) {
+ OrderedSetEntry entry = members.get(memberToAdd);
+ if (entry == null) {
OrderedSetEntry newEntry = new OrderedSetEntry(memberToAdd, scoreToAdd);
members.put(memberToAdd, newEntry);
scoreSet.add(newEntry);
- return null;
+ return false;
} else {
- scoreSet.remove(existingEntry);
- byte[] oldScore = existingEntry.scoreBytes;
- existingEntry.updateScore(stripTrailingZeroFromDouble(scoreToAdd));
- members.put(memberToAdd, existingEntry);
- scoreSet.add(existingEntry);
- return oldScore;
+ if (entry.score == scoreToAdd) {
+ return false;
+ }
+ updateScore(memberToAdd, scoreToAdd, entry);
}
+ return true;
+ }
+
+ private void updateScore(byte[] memberToAdd, double scoreToAdd,
OrderedSetEntry entry) {
+ scoreSet.remove(entry);
+ entry.updateScore(scoreToAdd);
+ members.put(memberToAdd, entry);
Review comment:
I think this line is not necessary since this is a reference to the
value already in the `members` map. Could you also rather inline the remaining
3 lines since this method isn't used anywhere else.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -818,15 +781,11 @@ protected int sizeValue(OrderedSetEntry value) {
public void toData(DataOutput out) throws IOException {
InternalDataSerializer.writePrimitiveInt(size(), out);
- final int maxIndex = getMaxIndex();
- for (int pos = 0; pos < maxIndex; ++pos) {
- OrderedSetEntry value = getValueAtIndex(pos);
- if (value != null) {
- byte[] member = value.getMember();
- byte[] score = value.getScoreBytes();
- InternalDataSerializer.writeByteArray(member, out);
- InternalDataSerializer.writeByteArray(score, out);
- }
+ for (Map.Entry<byte[], OrderedSetEntry> entry : entrySet()) {
+ byte[] member = entry.getKey();
+ double score = entry.getValue().getScore();
+ InternalDataSerializer.writeByteArray(member, out);
+ InternalDataSerializer.writeDouble(score, out);
Review comment:
Prefer `DataSerializer` instead of `InternalDataSerializer`
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java
##########
@@ -411,9 +411,31 @@ public static double bytesToDouble(byte[] bytes) {
return NEGATIVE_INFINITY;
}
if (isNaN(bytes)) {
- return NaN;
+ throw new NumberFormatException();
+ }
+
+ try {
+ return stringToDouble(bytesToString(bytes));
+ } catch (NumberFormatException e) {
+ throw new NumberFormatException(RedisConstants.ERROR_NOT_A_VALID_FLOAT);
+ }
+ }
+
+ /**
+ * Redis specific manner to parse floats
+ *
+ * @param d String holding double
+ * @return Value of string
+ * @throws NumberFormatException if the double cannot be parsed
+ */
+ public static double stringToDouble(String d) {
+ if (d.equalsIgnoreCase(P_INF)) {
+ return Double.POSITIVE_INFINITY;
+ } else if (d.equalsIgnoreCase(N_INF)) {
Review comment:
Should this also handle the longer form strings `[+-]Infinfity`?
--
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]
