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

get_range with heed does not work. #348

Open
Boog900 opened this issue Nov 25, 2024 · 3 comments
Open

get_range with heed does not work. #348

Boog900 opened this issue Nov 25, 2024 · 3 comments
Assignees
Labels
A-storage Related to storage. C-bug This is a bug-report. Bug-fix PRs use C-fix instead. P-high High priority.

Comments

@Boog900
Copy link
Member

Boog900 commented Nov 25, 2024

Bug

heed's get_range function returns incorrect results when a different ordering is used with the DB/table.

Steps to reproduce

Add this:

        assert_eq!(table.keys().unwrap().count(), 257);
        assert_eq!(table.values().unwrap().count(), 257);
        assert_eq!(table.get_range(0..257).unwrap().count(), 257);

After this:

let tx_ro = env_inner.tx_ro().unwrap();
let table = env_inner.open_db_ro::<TestTable>(&tx_ro).unwrap();
let iter = table.iter().unwrap();
let keys = table.keys().unwrap();
for ((i, iter), key) in RANGE.zip(iter).zip(keys) {
let (iter, _) = iter.unwrap();
let key = key.unwrap();
assert_eq!(i, iter);
assert_eq!(iter, key);
}


I think this is because heed's range cursor compares raw bytes here: https://docs.rs/heed/latest/src/heed/iterator/range.rs.html#416

@Boog900 Boog900 added A-storage Related to storage. C-bug This is a bug-report. Bug-fix PRs use C-fix instead. P-high High priority. labels Nov 25, 2024
@gzalz
Copy link

gzalz commented Nov 27, 2024

@Boog900 I think things might be working and that the test may need a slight refactor. Let me know your thoughts.

#350

@Boog900
Copy link
Member Author

Boog900 commented Nov 27, 2024

Yeah slightly embarrassing but I went too aggressive on trying to simplify the test case, updated the issue with the new test case that actually fails due to this issue.

@Boog900 Boog900 self-assigned this Nov 27, 2024
@Boog900
Copy link
Member Author

Boog900 commented Nov 27, 2024

meilisearch/heed#289

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-storage Related to storage. C-bug This is a bug-report. Bug-fix PRs use C-fix instead. P-high High priority.
Projects
None yet
Development

No branches or pull requests

2 participants