Skip to content

Commit f07e05c

Browse files
devinrsmithdevinrsmith
authored
Merge pull request #2 from lbooker42/nightly/DH-18265-lab-kohash-bugfix
fix: DH-18265 - correct bugs in `replace()` function
2 parents 52dcb48 + 7a2fbaa commit f07e05c

10 files changed

Lines changed: 709 additions & 11 deletions

‎src/main/java/io/deephaven/hash/KeyedDoubleObjectHash.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,19 +251,25 @@ public synchronized V replace(double key, V value) {
251251
}
252252

253253
public synchronized boolean replace(Double key, V oldValue, V newValue) {
254+
if (oldValue == null) {
255+
throw new NullPointerException("oldValue is null, but this map cannot hold null values");
256+
}
254257
if (!doubleKeyDef.equalDoubleKey(key, newValue)) {
255258
throw new IllegalArgumentException(
256259
"key and value are inconsistent:" + key + " and " + doubleKeyDef.getDoubleKey(newValue));
257260
}
258-
return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue) != null;
261+
return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue).equals(oldValue);
259262
}
260263

261264
public synchronized boolean replace(double key, V oldValue, V newValue) {
265+
if (oldValue == null) {
266+
throw new NullPointerException("oldValue is null, but this map cannot hold null values");
267+
}
262268
if (!doubleKeyDef.equalDoubleKey(key, newValue)) {
263269
throw new IllegalArgumentException(
264270
"key and value are inconsistent:" + key + " and " + doubleKeyDef.getDoubleKey(newValue));
265271
}
266-
return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue) != null;
272+
return internalPut(newValue, KeyedDoubleObjectHash.REPLACE, oldValue).equals(oldValue);
267273
}
268274

269275
private static final int NORMAL = 0;
@@ -307,7 +313,8 @@ protected V internalPut(V value, int mode, V oldValue) {
307313
}
308314
return null;
309315
} else if (candidate != DELETED && doubleKeyDef.equalDoubleKey(key, candidate)) {
310-
if (mode != KeyedDoubleObjectHash.IF_ABSENT) {
316+
if (mode != KeyedDoubleObjectHash.IF_ABSENT
317+
&& (oldValue == null || candidate.equals(oldValue))) {
311318
state[index] = value;
312319
_indexableList = null;
313320
}

‎src/main/java/io/deephaven/hash/KeyedIntObjectHash.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,19 +248,25 @@ public synchronized V replace(int key, V value) {
248248
}
249249

250250
public synchronized boolean replace(Integer key, V oldValue, V newValue) {
251+
if (oldValue == null) {
252+
throw new NullPointerException("oldValue is null, but this map cannot hold null values");
253+
}
251254
if (!intKeyDef.equalIntKey(key, newValue)) {
252255
throw new IllegalArgumentException(
253256
"key and value are inconsistent:" + key + " and " + intKeyDef.getIntKey(newValue));
254257
}
255-
return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue) != null;
258+
return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue).equals(oldValue);
256259
}
257260

258261
public synchronized boolean replace(int key, V oldValue, V newValue) {
262+
if (oldValue == null) {
263+
throw new NullPointerException("oldValue is null, but this map cannot hold null values");
264+
}
259265
if (!intKeyDef.equalIntKey(key, newValue)) {
260266
throw new IllegalArgumentException(
261267
"key and value are inconsistent:" + key + " and " + intKeyDef.getIntKey(newValue));
262268
}
263-
return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue) != null;
269+
return internalPut(newValue, KeyedIntObjectHash.REPLACE, oldValue).equals(oldValue);
264270
}
265271

266272
private static final int NORMAL = 0;
@@ -304,7 +310,8 @@ protected V internalPut(V value, int mode, V oldValue) {
304310
}
305311
return null;
306312
} else if (candidate != DELETED && intKeyDef.equalIntKey(key, candidate)) {
307-
if (mode != KeyedIntObjectHash.IF_ABSENT) {
313+
if (mode != KeyedIntObjectHash.IF_ABSENT
314+
&& (oldValue == null || candidate.equals(oldValue))) {
308315
state[index] = value;
309316
_indexableList = null;
310317
}

‎src/main/java/io/deephaven/hash/KeyedLongObjectHash.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,19 +249,25 @@ public synchronized V replace(long key, V value) {
249249
}
250250

251251
public synchronized boolean replace(Long key, V oldValue, V newValue) {
252+
if (oldValue == null) {
253+
throw new NullPointerException("oldValue is null, but this map cannot hold null values");
254+
}
252255
if (!longKeyDef.equalLongKey(key, newValue)) {
253256
throw new IllegalArgumentException(
254257
"key and value are inconsistent:" + key + " and " + longKeyDef.getLongKey(newValue));
255258
}
256-
return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue) != null;
259+
return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue).equals(oldValue);
257260
}
258261

259262
public synchronized boolean replace(long key, V oldValue, V newValue) {
263+
if (oldValue == null) {
264+
throw new NullPointerException("oldValue is null, but this map cannot hold null values");
265+
}
260266
if (!longKeyDef.equalLongKey(key, newValue)) {
261267
throw new IllegalArgumentException(
262268
"key and value are inconsistent:" + key + " and " + longKeyDef.getLongKey(newValue));
263269
}
264-
return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue) != null;
270+
return internalPut(newValue, KeyedLongObjectHash.REPLACE, oldValue).equals(oldValue);
265271
}
266272

267273
private static final int NORMAL = 0;
@@ -305,7 +311,8 @@ protected V internalPut(V value, int mode, V oldValue) {
305311
}
306312
return null;
307313
} else if (candidate != DELETED && longKeyDef.equalLongKey(key, candidate)) {
308-
if (mode != KeyedLongObjectHash.IF_ABSENT) {
314+
if (mode != KeyedLongObjectHash.IF_ABSENT
315+
&& (oldValue == null || candidate.equals(oldValue))) {
309316
state[index] = value;
310317
_indexableList = null;
311318
}

‎src/main/java/io/deephaven/hash/KeyedObjectHash.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -700,11 +700,14 @@ public synchronized V replace(K key, V value) {
700700
}
701701

702702
public synchronized boolean replace(K key, V oldValue, V newValue) {
703+
if (oldValue == null) {
704+
throw new NullPointerException("oldValue is null, but this map cannot hold null values");
705+
}
703706
if (!keyDef.equalKey(key, newValue)) {
704707
throw new IllegalArgumentException(
705708
"key and value are inconsistent:" + key + " and " + keyDef.getKey(newValue));
706709
}
707-
return internalPut(newValue, REPLACE, oldValue) != null;
710+
return internalPut(newValue, REPLACE, oldValue).equals(oldValue);
708711
}
709712

710713
public synchronized boolean add(V value) {
@@ -761,7 +764,7 @@ protected V internalPut(V value, int mode, V oldValue) {
761764
}
762765
return null;
763766
} else if (candidate != DELETED && keyDef.equalKey(key, candidate)) {
764-
if (mode != IF_ABSENT) {
767+
if (mode != IF_ABSENT && (oldValue == null || candidate.equals(oldValue))) {
765768
state[index] = value;
766769
_indexableList = null;
767770
}

‎src/test/java/io/deephaven/hash/KeyedDoubleTestObject.java‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
Copyright (C) 2021 Deephaven Data Labs (https://deephaven.io).
3+
4+
This program is free software: you can redistribute it and/or modify it under the terms of the
5+
GNU Lesser General Public License as published by the Free Software Foundation, either version 3
6+
of the License, or (at your option) any later version.
7+
8+
This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
9+
without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10+
GNU Lesser General Public License for more details.
11+
*/
12+
package io.deephaven.hash;
13+
14+
/** Test class. */
15+
class KeyedDoubleTestObject {
16+
private final double id;
17+
18+
public KeyedDoubleTestObject(double id) {
19+
this.id = id;
20+
}
21+
22+
public double getId() {
23+
return id;
24+
}
25+
26+
public boolean equals(Object other) {
27+
return other instanceof KeyedDoubleTestObject && id == ((KeyedDoubleTestObject) other).id;
28+
}
29+
30+
public int hashCode() {
31+
return ~Double.hashCode(id); // do something different that gnu.trove.HashFunctions.hash(double)
32+
}
33+
34+
public String toString() {
35+
return "[KeyedDoubleTestObject:" + id + "]";
36+
}
37+
38+
public static final KeyedDoubleObjectKey<KeyedDoubleTestObject> keyDef =
39+
new KeyedDoubleObjectKey<KeyedDoubleTestObject>() {
40+
public Double getKey(KeyedDoubleTestObject v) {
41+
return v.getId();
42+
}
43+
44+
public double getDoubleKey(KeyedDoubleTestObject v) {
45+
return v.getId();
46+
}
47+
48+
public int hashKey(Double k) {
49+
return k.hashCode();
50+
}
51+
52+
@Override
53+
public boolean equalKey(Double k, KeyedDoubleTestObject v) {
54+
return v.getId() == k;
55+
}
56+
57+
public int hashDoubleKey(double k) {
58+
return Double.hashCode(k);
59+
}
60+
61+
public boolean equalDoubleKey(double k, KeyedDoubleTestObject v) {
62+
return v.getId() == k;
63+
}
64+
};
65+
66+
public static final KeyedIntObjectHash.ValueFactory<KeyedDoubleTestObject> factory =
67+
new KeyedIntObjectHash.ValueFactory<KeyedDoubleTestObject>() {
68+
public KeyedDoubleTestObject newValue(Integer key) {
69+
return new KeyedDoubleTestObject(key);
70+
}
71+
72+
public KeyedDoubleTestObject newValue(int key) {
73+
return new KeyedDoubleTestObject(key);
74+
}
75+
};
76+
77+
// for intrusive chained maps
78+
79+
private KeyedDoubleTestObject next;
80+
81+
public static final IntrusiveChainedHashAdapter<KeyedDoubleTestObject> adapter =
82+
new IntrusiveChainedHashAdapter<KeyedDoubleTestObject>() {
83+
@Override
84+
public KeyedDoubleTestObject getNext(KeyedDoubleTestObject self) {
85+
return self.next;
86+
}
87+
88+
@Override
89+
public void setNext(KeyedDoubleTestObject self, KeyedDoubleTestObject next) {
90+
self.next = next;
91+
}
92+
};
93+
}

‎src/test/java/io/deephaven/hash/KeyedDoubleTestObjectMap.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
Copyright (C) 2021 Deephaven Data Labs (https://deephaven.io).
3+
4+
This program is free software: you can redistribute it and/or modify it under the terms of the
5+
GNU Lesser General Public License as published by the Free Software Foundation, either version 3
6+
of the License, or (at your option) any later version.
7+
8+
This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
9+
without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10+
GNU Lesser General Public License for more details.
11+
*/
12+
package io.deephaven.hash;
13+
14+
/** Instantiation of KeyedDoubleObjectHashMap on the test class. */
15+
class KeyedDoubleTestObjectMap extends KeyedDoubleObjectHashMap<KeyedDoubleTestObject> {
16+
public KeyedDoubleTestObjectMap() {
17+
super(KeyedDoubleTestObject.keyDef);
18+
}
19+
20+
public KeyedDoubleTestObjectMap(int initialCapacity) {
21+
super(initialCapacity, KeyedDoubleTestObject.keyDef);
22+
}
23+
24+
public KeyedDoubleTestObjectMap(int initialCapacity, float loadFactor) {
25+
super(initialCapacity, loadFactor, KeyedDoubleTestObject.keyDef);
26+
}
27+
28+
public final double getId(KeyedDoubleTestObject obj) {
29+
return obj.getId();
30+
}
31+
}

0 commit comments

Comments
 (0)