bmatsuo / lmdb-go

Bindings for the LMDB C library
BSD 3-Clause "New" or "Revised" License
157 stars 59 forks source link

lmdb: Txn.OpenDBI return value is not specified and not implemented clearly #70

Closed bmatsuo closed 8 years ago

bmatsuo commented 8 years ago

I took a look at the import graph generated by godoc.org. It pointed out some interesting things. Most significantly it seems like math.NaN() is used to generate an "invalid" DBI in case of failure in mdb_dbi_open.

    if ret != success {
        return DBI(math.NaN()), operrno("mdb_dbi_open", ret)
    }

This is weird but it also turns out to be pointless. Converting math.NaN() to DBI (a uint type) results in 0. This turns out to be questionable behavior because 0 is not actually an invalid DBI.

It is probably more appropriate to return ^DBI(0), -1. This should not be a valid DBI (it may even be guaranteed due to a limit from some constant #define).

bmatsuo commented 8 years ago

After more thought I think it's better to just use whatever DBI is set (if any) by mdb_dbi_open. This happens to be zero (never set) at the moment. This is backwards compatible. And applications are supposed to check all their potential errors.

Because the package will use whatever value is "returned" by LMDB then the return value in the error case should remain unspecified because it is deferred to LMDB.