hoytech / lmdbxx

C++17 wrapper for the LMDB embedded B+ tree database library
The Unlicense
50 stars 16 forks source link

Add support for MDB_VL32 #12

Open yak1ex opened 1 year ago

yak1ex commented 1 year ago

If MDB_VL32 is defined, 32bit lmdb lib can handle a db made by 64bit lmdb lib. In this case, mdb_size_t is not the same as std::size_t.

One possible modification to handle the situation, add lmdb::size_t like as

 namespace lmdb {
   using mode = mdb_mode_t;
   using size_t = mdb_size_t;
 }

then replace std::size_t with lmdb::size_t in places related with mdb_env_set_mapsize(), mdb_txn_id() and mdb_cursor_count(). This situation may be not so often, however, I think this modification should not be harmful for other situations, mdb_size_t = size_t.

If this modification is acceptable for you, I will make a pull request in that way. What do you think?

hoytech commented 11 months ago

Hi, very sorry I missed this issue, I had some difficult circumstances and wasn't following up on github issues.

Yes, I think this is a good idea to support this, and your proposed change doesn't sound too intrusive.

yak1ex commented 10 months ago

Thanks. I'll make a pull request.

yak1ex commented 10 months ago

I recognized MDB_VL32 feature has not been officially released yet. So, offical releases do not have mdb_size_t.

We can, maybe, employ some metaprogramming technique to detect if the type is defined or not, but using a preprocessor is simple and enough, isn't it?

namespace lmdb {
  using mode = mdb_mode_t;
#ifdef MDB_SIZE_MAX
  using size_t = mdb_size_t;
#else
  using size_t = std::size_t;
#endif
}
hoytech commented 10 months ago

Yes I think that seems reasonable, ideally a comment explaining when we'd expect MDB_SIZE_MAX to be set.

BTW: Will you be testing this with the unreleased VL32 feature? Are you using it for anything now, or planning to?

uaply commented 3 weeks ago

I do use VL32 for enable copying database files created on Win64 PC to 32-bit ARM device. I can participate in testing modifications required, because I do modification of hoytech/lmdbxx myself anyway.

yak1ex commented 2 weeks ago

Sorry for the long hiatus. My use case was solved by another measure, so there is less concern for this issue... I made some work anyway; I created the PR.

@uaply Could you check the https://github.com/hoytech/lmdbxx/pull/15 ?