-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat: add dup_sort_comparator
#283
base: main
Are you sure you want to change the base?
Conversation
Hey @oXtxNt9U 👋 They're already is a way to use custom comparison functions in a safe way in heed. You can see that in the database open options, you just have to use the Compare trait. However, it was hard, even for me, to find that in the documentation. Would you mind adding a small guide (in the top three) to the cookbook to show people that we support it and how to use it? There already are some examples in this PR (whoops, nope). Have a nice day and please close this PR if I was right 😊 |
@Kerollmops Thank you for pointing out the Basically, what I need is something like this inside unsafe {
mdb_result(ffi::mdb_dbi_open(raw_txn, name_ptr, flags, &mut dbi))?;
if TypeId::of::<C>() != TypeId::of::<DefaultComparator>() {
if AllDatabaseFlags::from_bits(flags)
.is_some_and(|f| f.contains(AllDatabaseFlags::DUP_SORT))
{
mdb_result(ffi::mdb_set_dupsort(
raw_txn,
dbi,
Some(custom_key_cmp_wrapper::<C>),
))?;
} else {
mdb_result(ffi::mdb_set_compare(
raw_txn,
dbi,
Some(custom_key_cmp_wrapper::<C>),
))?;
}
}
}; However this makes it impossible to use a different compare for both keys and values since it reuses let db = env
.database_options()
.types::<StorageKey, StorageValue>()
.dup_sort_comparator::<CustomComparator>() // sets CDUP
.flags(DatabaseFlags::DUP_SORT)
.create(&mut wtxn)
.expect("db"); What do you think? |
Yeah! That could be much better. Also, not breaking APIs could be great too but may be impossible. |
3e324d0
to
9713e32
Compare
@Kerollmops Hey, I updated the PR and included examples for the cookbook too. Please let me know if anything is still missing. |
@Kerollmops Friendly ping! 😄 |
Hey @oXtxNt9U 👋 Thank you for the changes. Everything looks good to me. However, it is breaking, so I would prefer to wait for some other PRs to be released first. Also, we merged a couple of PRs on the main. Would you mind rebasing, please? Have a nice end of the week 🌹 |
9713e32
to
fd33042
Compare
@Kerollmops I rebased the PR. And of course I don't mind waiting for a new release. Thank you! |
Pull Request
What does this PR do?
Expose http://www.lmdb.tech/doc/group__internal.html#gacef4ec3dab0bbd9bc978b73c19c879ae via new
dup_sort_comparator
function to set a customComparator
.It basically works like
key_comparator
, except that for callingmdb_set_compare
it callsmdb_set_dupsort
on the database.It can be used like this:
Also added an example and updated the cookbook (resolves #284).