search

danburkert / lmdb-rs

Safe Rust bindings for LMDB
Apache License 2.0
172 stars 87 forks source link

Have iter_from/iter_dup_from return a Result (update of #7) #29

Closed mykmelez closed 6 years ago

mykmelez commented 6 years ago

The _iterfrom test in #7 is failing because MDB_SET_RANGE sets the "Position at first key greater than or equal to specified key." And foo is less than key1, the first key in the test store; so cursor.iter_from(b"foo") sets the cursor position to the first key and returns Ok instead of Err(NotFound).

(The _iter_dupfrom test doesn't fail in the same way because foo is greater than c, the greatest key in that test's store.)

This branch fixes the test by seeking to key6, which is greater than any key in the store, instead of foo. It also adds tests to confirm that seeking to an intermediate key that doesn't exist (key4 in a store with key3 and key5 keys) will move the cursor's position to the next key greater than or equal to that key.

NB: given LMDB's behavior when seeking to a nonexistent but intermediate key (which is actually quite useful for my use case), it isn't clear to me that an Err(NotFound) is the most ergonomic result of seeking to a nonexistent key that is greater than any in the store.

Returning an iterator over an empty set in that case would make the behavior more consistent, and it would be more usable for callers whose behavior doesn't depend on the presence of the given key (nor any greater ones).

Nevertheless, I haven't made that change in this branch. I'll request a pull with it on another branch for your consideration.

mykmelez commented 6 years ago

The Linux, Mac, and Windows MSVC builds are succeeding, but the Windows GNU builds are failing with:

  = note: C:\projects\lmdb-rs\target\x86_64-pc-windows-gnu\debug\build\lmdb-sys-9ae76c8c27193545\out\liblmdb.a(mdb.o): In function `mdb_assert_fail':
          C:/projects/lmdb-rs/lmdb-sys/lmdb/libraries/liblmdb/mdb.c:1535: undefined reference to `__imp___acrt_iob_func'

Which seems like unrelated bustage.

mykmelez commented 6 years ago

Which seems like unrelated bustage.

Ah, it's indeed bustage in Rust and/or MinGW. https://github.com/rust-lang/rust/issues/47048 is tracking this on the Rust side, while https://github.com/Alexpux/MINGW-packages/issues/3237 is tracking it for MinGW.

danburkert commented 6 years ago

Merged as part of #37