Skip to content

Commit

Permalink
Complete support for empty keys and add tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
bhartnett committed Jul 8, 2024
1 parent a854cd0 commit e99428d
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 38 deletions.
6 changes: 6 additions & 0 deletions rocksdb/internal/utils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,9 @@ template bailOnErrors*(errors: cstring): auto =
let res = err($(errors))
rocksdb_free(errors)
return res

template unsafeAddrOrNil*(s: openArray[byte]): auto =
if s.len > 0:
unsafeAddr s[0]
else:
nil
14 changes: 5 additions & 9 deletions rocksdb/rocksdb.nim
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ proc get*(
db.cPtr,
db.readOpts.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
len.addr,
cast[cstringArray](errors.addr),
Expand Down Expand Up @@ -285,13 +285,9 @@ proc put*(
db.cPtr,
db.writeOpts.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstring](if val.len > 0:
unsafeAddr val[0]
else:
nil
),
cast[cstring](val.unsafeAddrOrNil()),
csize_t(val.len),
cast[cstringArray](errors.addr),
)
Expand All @@ -312,7 +308,7 @@ proc keyMayExist*(
db.cPtr,
db.readOpts.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
nil,
nil,
Expand Down Expand Up @@ -350,7 +346,7 @@ proc delete*(
db.cPtr,
db.writeOpts.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstringArray](errors.addr),
)
Expand Down
2 changes: 1 addition & 1 deletion rocksdb/rocksiterator.nim
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ proc seekToKey*(iter: RocksIteratorRef, key: openArray[byte]) =
## invalid.
##
doAssert not iter.isClosed()
rocksdb_iter_seek(iter.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len))
rocksdb_iter_seek(iter.cPtr, cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len))

proc seekToFirst*(iter: RocksIteratorRef) =
## Seeks to the first entry in the column family.
Expand Down
6 changes: 3 additions & 3 deletions rocksdb/sstfilewriter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ proc put*(
var errors: cstring
rocksdb_sstfilewriter_put(
writer.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstring](unsafeAddr val[0]),
cast[cstring](val.unsafeAddrOrNil()),
csize_t(val.len),
cast[cstringArray](errors.addr),
)
Expand All @@ -77,7 +77,7 @@ proc delete*(writer: SstFileWriterRef, key: openArray[byte]): RocksDBResult[void
var errors: cstring
rocksdb_sstfilewriter_delete(
writer.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstringArray](errors.addr),
)
Expand Down
12 changes: 4 additions & 8 deletions rocksdb/transactions/transaction.nim
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ proc get*(
tx.cPtr,
tx.readOpts.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
len.addr,
cast[cstringArray](errors.addr),
Expand Down Expand Up @@ -115,13 +115,9 @@ proc put*(
rocksdb_transaction_put_cf(
tx.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstring](if val.len > 0:
unsafeAddr val[0]
else:
nil
),
cast[cstring](val.unsafeAddrOrNil()),
csize_t(val.len),
cast[cstringArray](errors.addr),
)
Expand All @@ -138,7 +134,7 @@ proc delete*(
rocksdb_transaction_delete_cf(
tx.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstringArray](errors.addr),
)
Expand Down
12 changes: 4 additions & 8 deletions rocksdb/writebatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

{.push raises: [].}

import ./lib/librocksdb, ./internal/cftable, ./rocksresult
import ./lib/librocksdb, ./internal/[cftable, utils], ./rocksresult

export rocksresult

Expand Down Expand Up @@ -54,13 +54,9 @@ proc put*(
rocksdb_writebatch_put_cf(
batch.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstring](if val.len > 0:
unsafeAddr val[0]
else:
nil
),
cast[cstring](val.unsafeAddrOrNil()),
csize_t(val.len),
)

Expand All @@ -72,7 +68,7 @@ proc delete*(
## Add a delete operation to the write batch.

rocksdb_writebatch_delete_cf(
batch.cPtr, cfHandle.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len)
batch.cPtr, cfHandle.cPtr, cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len)
)

ok()
Expand Down
12 changes: 4 additions & 8 deletions rocksdb/writebatchwi.nim
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,9 @@ proc put*(
rocksdb_writebatch_wi_put_cf(
batch.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
cast[cstring](if val.len > 0:
unsafeAddr val[0]
else:
nil
),
cast[cstring](val.unsafeAddrOrNil()),
csize_t(val.len),
)

Expand All @@ -86,7 +82,7 @@ proc delete*(
## Add a delete operation to the write batch.

rocksdb_writebatch_wi_delete_cf(
batch.cPtr, cfHandle.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len)
batch.cPtr, cfHandle.cPtr, cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len)
)

ok()
Expand All @@ -107,7 +103,7 @@ proc get*(
batch.cPtr,
batch.dbOpts.cPtr,
cfHandle.cPtr,
cast[cstring](unsafeAddr key[0]),
cast[cstring](key.unsafeAddrOrNil()),
csize_t(key.len),
len.addr,
cast[cstringArray](errors.addr),
Expand Down
9 changes: 9 additions & 0 deletions tests/test_rocksdb.nim
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,15 @@ suite "RocksDbRef Tests":
db.keyMayExist(key4).get() == false
db.keyMayExist(key5).get() == false

test "Put, get and delete empty key":
let empty: seq[byte] = @[]

check:
db.put(empty, val).isOk()
db.get(empty).get() == val
db.delete(empty).isOk()
db.get(empty).isErr()

test "List column familes":
let cfRes1 = listColumnFamilies(dbPath)
check:
Expand Down
14 changes: 14 additions & 0 deletions tests/test_rocksiterator.nim
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,20 @@ suite "RocksIteratorRef Tests":
iter.key() == key2
iter.value() == val2

test "Seek to empty key":
let empty: seq[byte] = @[]
check db.put(empty, val1).isOk()

let iter = db.openIterator().get()
defer:
iter.close()

iter.seekToKey(empty)
check:
iter.isValid()
iter.key() == empty
iter.value() == val1

test "Empty column family":
let res = db.openIterator(emptyCfHandle)
check res.isOk()
Expand Down
13 changes: 13 additions & 0 deletions tests/test_sstfilewriter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,19 @@ suite "SstFileWriterRef Tests":
db.get(key2, otherCfHandle).get() == val2
db.get(key3, otherCfHandle).get() == val3

test "Put, get and delete empty key":
let writer = openSstFileWriter(sstFilePath).get()
defer:
writer.close()

let empty: seq[byte] = @[]
check:
writer.put(empty, val1).isOk()
writer.finish().isOk()
db.ingestExternalFile(sstFilePath).isOk()
db.keyExists(empty).get() == true
db.get(empty).get() == val1

test "Test close":
let res = openSstFileWriter(sstFilePath)
check res.isOk()
Expand Down
14 changes: 13 additions & 1 deletion tests/test_transactiondb.nim
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,18 @@ suite "TransactionDbRef Tests":
tx2.get(key2, otherCfHandle).error() == ""
tx2.get(key3, otherCfHandle).get() == val3

test "Put, get and delete empty key":
let tx = db.beginTransaction()
defer:
tx.close()

let empty: seq[byte] = @[]
check:
tx.put(empty, val1).isOk()
tx.get(empty).get() == val1
tx.delete(empty).isOk()
tx.get(empty).isErr()

test "Test close":
var tx = db.beginTransaction()

Expand Down Expand Up @@ -227,7 +239,7 @@ suite "TransactionDbRef Tests":
columnFamilies[0].isClosed() == true
db.isClosed() == true

test "Test auto close enabled":
test "Test auto close disabled":
let
dbPath = mkdtemp() / "autoclose-disabled"
dbOpts = defaultDbOptions(autoClose = false)
Expand Down
14 changes: 14 additions & 0 deletions tests/test_writebatch.nim
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,20 @@ suite "WriteBatchRef Tests":
not batch1.isClosed()
not batch2.isClosed()

test "Put, get and delete empty key":
let batch = db.openWriteBatch()
defer:
batch.close()

let empty: seq[byte] = @[]
check:
batch.put(empty, val1).isOk()
db.write(batch).isOk()
db.get(empty).get() == val1
batch.delete(empty).isOk()
db.write(batch).isOk()
db.get(empty).isErr()

test "Test write empty batch":
let batch = db.openWriteBatch()
defer:
Expand Down
12 changes: 12 additions & 0 deletions tests/test_writebatchwi.nim
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,18 @@ suite "WriteBatchWIRef Tests":
batch2.count() == 3
batch2.get(key1).get() == val1

test "Put, get and delete empty key":
let batch = db.openWriteBatchWithIndex()
defer:
batch.close()

let empty: seq[byte] = @[]
check:
batch.put(empty, val1).isOk()
batch.get(empty).get() == val1
batch.delete(empty).isOk()
batch.get(empty).isErr()

test "Test close":
let batch = db.openWriteBatchWithIndex()

Expand Down

0 comments on commit e99428d

Please sign in to comment.