ulysses-you commented on code in PR #58466:
URL: https://github.com/apache/spark/pull/58466#discussion_r3920901895
##########
common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBSuite.java:
##########
@@ -98,6 +98,37 @@ public void testObjectWriteReadDelete() throws Exception {
assertEquals(0, countKeys(t.getClass()));
}
+ @Test
+ public void testGetOrNullMissingKey() throws Exception {
Review Comment:
Done in 1330280aa08: only the `assertNull` keeps the hand-built key; the
throws and present-key assertions go through `db.read`.
##########
common/kvstore/src/test/java/org/apache/spark/util/kvstore/RocksDBSuite.java:
##########
@@ -95,6 +95,37 @@ public void testObjectWriteReadDelete() throws Exception {
assertEquals(0, countKeys(t.getClass()));
}
+ @Test
+ public void testGetOrNullMissingKey() throws Exception {
Review Comment:
Same, done in 1330280aa08.
##########
common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBSuite.java:
##########
@@ -98,6 +98,37 @@ public void testObjectWriteReadDelete() throws Exception {
assertEquals(0, countKeys(t.getClass()));
}
+ @Test
+ public void testGetOrNullMissingKey() throws Exception {
+ // getOrNull() returns null for a missing key so expected misses (e.g. the
write path
+ // looking up an existing entry) skip the cost of building an exception,
while get()
+ // still surfaces a missing key as NoSuchElementException.
+ byte[] missingKey =
db.getTypeInfo(CustomType1.class).naturalIndex().start(null, "missing");
+ assertNull(db.getOrNull(missingKey, CustomType1.class));
+ assertThrows(NoSuchElementException.class, () -> db.get(missingKey,
CustomType1.class));
+
+ CustomType1 t = createCustomType1(1);
+ db.write(t);
+ byte[] presentKey =
db.getTypeInfo(CustomType1.class).naturalIndex().start(null, t.key);
+ assertEquals(t, db.getOrNull(presentKey, CustomType1.class));
+ }
+
+ @Test
+ public void testDeleteEdgeCases() throws Exception {
Review Comment:
Kept as-is; it now documents the same no-op contract the corrected
`KVStore.delete` javadoc states.
##########
common/kvstore/src/test/java/org/apache/spark/util/kvstore/LevelDBSuite.java:
##########
@@ -98,6 +98,37 @@ public void testObjectWriteReadDelete() throws Exception {
assertEquals(0, countKeys(t.getClass()));
}
+ @Test
+ public void testGetOrNullMissingKey() throws Exception {
+ // getOrNull() returns null for a missing key so expected misses (e.g. the
write path
+ // looking up an existing entry) skip the cost of building an exception,
while get()
+ // still surfaces a missing key as NoSuchElementException.
+ byte[] missingKey =
db.getTypeInfo(CustomType1.class).naturalIndex().start(null, "missing");
+ assertNull(db.getOrNull(missingKey, CustomType1.class));
+ assertThrows(NoSuchElementException.class, () -> db.get(missingKey,
CustomType1.class));
+
+ CustomType1 t = createCustomType1(1);
+ db.write(t);
+ byte[] presentKey =
db.getTypeInfo(CustomType1.class).naturalIndex().start(null, t.key);
+ assertEquals(t, db.getOrNull(presentKey, CustomType1.class));
+ }
+
+ @Test
+ public void testDeleteEdgeCases() throws Exception {
+ // Never-written type: type info is created on the fly, lookup misses,
nothing happens.
+ db.delete(CustomType1.class, "missing");
+ assertEquals(0L, db.count(CustomType1.class));
+
+ // Never-written key of a written type.
+ db.write(createCustomType1(1));
+ db.delete(CustomType1.class, "missing");
+ assertEquals(1L, db.count(CustomType1.class));
+
+ // Mismatched key type: the encoded lookup key misses, nothing is removed.
Review Comment:
Dropped the case in 1330280aa08.
##########
common/kvstore/src/test/java/org/apache/spark/util/kvstore/RocksDBSuite.java:
##########
@@ -95,6 +95,37 @@ public void testObjectWriteReadDelete() throws Exception {
assertEquals(0, countKeys(t.getClass()));
}
+ @Test
+ public void testGetOrNullMissingKey() throws Exception {
+ // getOrNull() returns null for a missing key so expected misses (e.g. the
write path
+ // looking up an existing entry) skip the cost of building an exception,
while get()
+ // still surfaces a missing key as NoSuchElementException.
+ byte[] missingKey =
db.getTypeInfo(CustomType1.class).naturalIndex().start(null, "missing");
+ assertNull(db.getOrNull(missingKey, CustomType1.class));
+ assertThrows(NoSuchElementException.class, () -> db.get(missingKey,
CustomType1.class));
+
+ CustomType1 t = createCustomType1(1);
+ db.write(t);
+ byte[] presentKey =
db.getTypeInfo(CustomType1.class).naturalIndex().start(null, t.key);
+ assertEquals(t, db.getOrNull(presentKey, CustomType1.class));
+ }
+
+ @Test
+ public void testDeleteEdgeCases() throws Exception {
+ // Never-written type: type info is created on the fly, lookup misses,
nothing happens.
+ db.delete(CustomType1.class, "missing");
+ assertEquals(0L, db.count(CustomType1.class));
+
+ // Never-written key of a written type.
+ db.write(createCustomType1(1));
+ db.delete(CustomType1.class, "missing");
+ assertEquals(1L, db.count(CustomType1.class));
+
+ // Mismatched key type: the encoded lookup key misses, nothing is removed.
Review Comment:
Same, dropped in 1330280aa08.
--
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]
