Failiable predicate in retain and extract

[dpc]

Since I'm working on a bincode wrapper, my predicate can (in theory) fail to decode key/value and then there's no way to bubble it up. All other APIs I've wrapped so far allowed me to bubble up internal encoding/decoding errors.

[cberner]

redb will only pass a value to from_bytes() that came from as_bytes(), so it should not be possible for it to fail if you wrote your implementation in a way that can always round trip values

[dpc]

That's something that I'm on a fence with.

Technically the user of my code is free to implement their own de/en-coding for every type, I can't really force them to round-trip. And in principle the predicate might even have a reason to fail other then encoding/decoding. E.g. what if the decoding requires something that is fallible - let's say a look up in a persisted dictionary?

So far I was able to expose the possibility of encoding issues in every API without much of a UX degradation, if few places exposing alternative APIs that just panic. In redb-bincode indeed in a normal case things should round-trip...

[dpc]

In redb-bincode maybe I should just stuff decoding problems into StorageError::Corrupted and call it a day.

[cberner]

And in principle the predicate might even have a reason to fail other then encoding/decoding. E.g. what if the decoding requires something that is fallible - let's say a look up in a persisted dictionary?

I modeled the API after BTreeMap in std. My reasoning was that the authors of std probably thought about this and made a reasonable trade off by not supporting errors. So I'd say that's on the user of your library to ensure their predicate doesn't fail

[dpc]

I modeled the API after BTreeMap in std.

People are not going to do the exact same things with a map, as they do with a database. AFAICT, the way retain works with BTreeMap, one can do just map.into_iter().map(Err(foo))).collect::Result<BTreeMap<(), ()>>()? and have error support without loosing anything (except having to write slightly more code).

[cberner]

Could you collect a few real world use cases to motivate this, and write up the exact semantics that you'd like retain()/extract() to have when the predicate returns an Err?

That'd help me evaluate whether it's makes sense to change it

[dpc]

These two APIs take the control from the caller for indefinite time with no ability to influence it, and experience tells me that there will be uncommon, but real cases where that will be a problem.

Real world use case? I don't know ... Lets say I have 1B (or rather: potentially very large number) keys to go through, the whatever computation on each might multiply the effect, and if the user/system requested a timeout, I'd like to abort immediately/prompty?

I assume that the way extract works is way faster than individually looking up and deleting keys, which would be the only workaround otherwise.

Semantics would be just short-circuiting and bubbling the error out. Then the user can abort or commit, depending if the partial delete is better than none at all.

[cberner]

ya, makes sense. I'll give it some thought, but am reluctant to complicate the API. To make it properly general I think the predicate would need to be FnMut -> Result<bool, E> so that the user can also control the type of error returned.

It may be better to support advanced use cases like that with a mutable cursor API

[dpc]

It may be better to support advanced use cases like that with a mutable cursor API

That sounds really powerful and would address use cases that the current APIs might not, so 👍