Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MDB_VL32 #12

Open
yak1ex opened this issue Apr 14, 2023 · 6 comments
Open

Add support for MDB_VL32 #12

yak1ex opened this issue Apr 14, 2023 · 6 comments

Comments

@yak1ex
Copy link

yak1ex commented Apr 14, 2023

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
Copy link
Owner

hoytech commented Aug 7, 2023

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
Copy link
Author

yak1ex commented Aug 12, 2023

Thanks. I'll make a pull request.

@yak1ex
Copy link
Author

yak1ex commented Aug 12, 2023

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
Copy link
Owner

hoytech commented Aug 14, 2023

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
Copy link

uaply commented Jun 11, 2024

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
Copy link
Author

yak1ex commented Jun 17, 2024

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 #15 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants