vhbit / lmdb-rs

Rust bindings for LMDB
MIT License
114 stars 45 forks source link

consider merging with lmdb #32

Open iqualfragile opened 8 years ago

iqualfragile commented 8 years ago

https://github.com/danburkert/lmdb-rs

vks commented 7 years ago

What are the differences?

cmbrandenburg commented 7 years ago

Here are some differences I observed between this crate and the danburkert crate while I wrote a simple LMDB-driven application during the past few weeks. Not only did I end up using danburkert crate because of its overall superior API design, but I caution everyone about using this crate and recommend they do not use this crate. I apologize for the harshness of my criticism, but this crate suffers from at least one unacceptable design flaw as well as several additional flaws whose seriousness is in the eye of the beholder.

Memory-safety

This is the unacceptable design flaw and the biggest difference between this and the danburkert crate. This crate is memory-unsafe. I explained part of the problem in issue #48. The gist is that this crate fails to mark several memory-unsafe functions as unsafe, meaning an idiomatic, safe Rust application that uses this crate can have buffer overruns, segfaults, etc. The design flaw is that this crate provides no safe alternative—and doing so will require breaking changes to the API. In my opinion, people should not use this crate until the memory-safety issues are resolved.

Simpler database abstraction

Another difference is how the two crates represent database handles. This crate has two distinct types: a non-lifetime-tracked DbHandle and a lifetime-tracked Database instance that lives only within a transaction. Whereas, the danburkert crate has a single type, Database, that is roughly analogous to this crate's DbHandle. Database operations such as get and put are then methods of the transaction, not the lifetime-tracked database.

This difference matters as a point of convenience and ease-of-use. In the danburkert crate, to put an item into a database, the application need only create a transaction. Whereas, in this crate, it's necessary to create a transaction and a Database instance, then scope the Database such that it drops before the transaction—often this requires an explicit {} block. There's simply no benefit to having this be a two-step process, and it requires more boilerplate.

Generic code regarding read-only vs read-write

Yet another design difference is that the danburkert crate provides the Cursor and Transaction traits, which have the read-only methods (e.g., get), while the read-write methods (e.g., put) are provided in the RwCursor and RwTransaction types. This allows applications to have generic code for reading from a cursor and transaction—regardless of whether the actual cursor or transaction is read-only or read-write. This is not currently possible in this crate. However, I partially resolved this in pull request #49, but I used a different technique and didn't solve the problem for cursors. With the benefit of hindsight, I prefer the danburkert design.

Minor points of taste and style

There are other, minor nits. For example, the danburkert crate provides an open_db with an optional database name for opening either the default database or a named database, whereas this crate provides separate methods, get_db vs get_default_db. Also, this crate stuffs definitions in a core submodule, with a couple of traits (FromMdbValue and ToMdbValue) residing in a traits submodule. I don't understand the purpose of these submodules, being as how those two traits are as “core” to the use of this crate as the definitions within the core module—nor can I think of what “non-core” would mean to make the existence of a core module necessary or meaningful.