DonalEvans commented on a change in pull request #6701:
URL: https://github.com/apache/geode/pull/6701#discussion_r671483756
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java
##########
@@ -237,35 +237,32 @@ public int hstrlen(byte[] field) {
return new ArrayList<>(hash.keySet());
}
- public ImmutablePair<Integer, List<byte[]>> hscan(Pattern matchPattern,
- int count,
- int cursor) {
-
- ArrayList<byte[]> resultList = new ArrayList<>(count + 2);
+ public ImmutablePair<Integer, List<ImmutablePair<byte[], byte[]>>>
hscan(Pattern matchPattern,
+ int count, int cursor) {
+ // No need to allocate more space than it's possible to use given the size
of the hash
+ int initialCapacity = Math.min(count, hash.size());
+ List<ImmutablePair<byte[], byte[]>> resultList = new
ArrayList<>(initialCapacity);
do {
cursor = hash.scan(cursor, 1,
(list, key, value) -> addIfMatching(matchPattern, list, key, value),
resultList);
- } while (cursor != 0 && resultList.size() < (count * 2));
+ } while (cursor != 0 && resultList.size() < count);
return new ImmutablePair<>(cursor, resultList);
}
- private void addIfMatching(Pattern matchPattern, List<byte[]> resultList,
byte[] key,
- byte[] value) {
+ private void addIfMatching(Pattern matchPattern, List<ImmutablePair<byte[],
byte[]>> resultList,
+ byte[] key, byte[] value) {
if (matchPattern != null) {
if (matchPattern.matcher(bytesToString(key)).matches()) {
- resultList.add(key);
- resultList.add(value);
+ resultList.add(new ImmutablePair<>(key, value));
Review comment:
It's necessary to wrap the key and value in some object here, since the
maximum length of a list returned by `HSCAN` is 2*(number of keys) which could
in theory be greater than `Integer.MAX_VALUE`. If we want to assume that a user
will never have that many keys in their Redish hash, then we can dispense with
this pairing of values to increase efficiency and document that the maximum
number of Redis hash keys we support is `Integer.MAX_VALUE / 2`, but if we want
to be safe, then this is necessary.
--
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]
