ulysses-you commented on code in PR #58466:
URL: https://github.com/apache/spark/pull/58466#discussion_r3920339980
##########
common/kvstore/src/main/java/org/apache/spark/util/kvstore/LevelDBIterator.java:
##########
@@ -147,6 +147,9 @@ public T next() {
} else {
byte[] key = ti.buildKey(false, ti.naturalIndex().keyPrefix(null),
next);
ret = db.get(key, type);
+ if (ret == null) {
+ throw new NoSuchElementException();
Review Comment:
Superseded in b5ac9723bd7: per @dongjoon-hyun's review the iterators are
back to the base shape, and the missing-key exception is built only in `get()`
again.
##########
common/kvstore/src/main/java/org/apache/spark/util/kvstore/RocksDBIterator.java:
##########
@@ -133,6 +133,9 @@ public T next() {
} else {
byte[] key = ti.buildKey(false, ti.naturalIndex().keyPrefix(null),
next);
ret = db.get(key, type);
+ if (ret == null) {
+ throw new NoSuchElementException();
Review Comment:
Superseded in b5ac9723bd7: per @dongjoon-hyun's review the iterators are
back to the base shape, and the missing-key exception is built only in `get()`
again.
##########
common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBSuite.java:
##########
@@ -98,6 +98,20 @@ public void testObjectWriteReadDelete() throws Exception {
assertEquals(0, countKeys(t.getClass()));
}
+ @Test
+ public void testGetMissingKeyReturnsNull() throws Exception {
Review Comment:
Superseded in b5ac9723bd7: the iterator null branch is gone with the
iterators back to the base shape; coverage is now `testGetOrNullMissingKey` and
`testDeleteEdgeCases`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
