Skip to content
This repository was archived by the owner on Sep 13, 2023. It is now read-only.

Have iter_from/iter_dup_from return a Result #1

Merged
merged 4 commits into from
Jul 27, 2018

Conversation

mykmelez
Copy link

(This issue is a clone of danburkert#29.)

The iter_from test in danburkert#7 is failing because MDB_SET_RANGE sets the "Position at first key greater than or equal to specified key." And foo is less than key1, the first key in the test store; so cursor.iter_from(b"foo") sets the cursor position to the first key and returns Ok instead of Err(NotFound).

(The iter_dup_from test doesn't fail in the same way because foo is greater than c, the greatest key in that test's store.)

This branch fixes the test by seeking to key6, which is greater than any key in the store, instead of foo. It also adds tests to confirm that seeking to an intermediate key that doesn't exist (key4 in a store with key3 and key5 keys) will move the cursor's position to the next key greater than or equal to that key.

NB: given LMDB's behavior when seeking to a nonexistent but intermediate key (which is actually quite useful for my use case), it isn't clear to me that an Err(NotFound) is the most ergonomic result of seeking to a nonexistent key that is greater than any in the store.

Returning an iterator over an empty set in that case would make the behavior more consistent, and it would be more usable for callers whose behavior doesn't depend on the presence of the given key (nor any greater ones).

Nevertheless, I haven't made that change in this branch. I'll request a pull with it on another branch for your consideration.

@mykmelez mykmelez requested a review from ncloudioj July 24, 2018 22:45
Copy link
Member

@ncloudioj ncloudioj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LTGM!

Aha, the good old MDB_SET_RANGE, which reminds me this.

@mykmelez mykmelez merged commit eb9185e into mozilla:master Jul 27, 2018
@mykmelez mykmelez deleted the iter-from-result branch July 27, 2018 17:25
@mykmelez mykmelez mentioned this pull request Aug 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants