Add count_duplicates() for O(1) duplicate counting - #380
Conversation
LMDB already provides mdb_cursor_count(), which reads the stored duplicate count from page metadata in constant time. Expose it.
p-lindberg
left a comment
There was a problem hiding this comment.
I think it would be better to just propagate the error when the underlying database is non-dupsort instead of returning 1 by default, since this leads to an inconsistent result when the cursor is not positioned on an entry (as demonstrated by the tests). Besides, returning 1 does not really add much value since this is implicit anyway.
I can't speak for whether or not this change is in line with the design intent of the crate as a whole, but I looked through the changes and I think they look solid, apart from the above comment.
Verified all tests pass too, as long as the omitted import is fixed. Would love to see this PR merged as I too would have use for this method.
| /// let mut wtxn = env.write_txn()?; | ||
| /// let db = env.database_options() | ||
| /// .types::<BEI64, BEI64>() | ||
| /// .flags(DatabaseFlags::DUP_SORT) |
There was a problem hiding this comment.
This line causes an error when running doc tests due to DatabaseFlags not being imported. It can be fixed by updating line 162 to
/// # use heed::{EnvOpenOptions, DatabaseFlags};
|
I have reconsidered and won't push for this anymore. The reason is simple. I have determined that using Sorry for the noise. |
Hi.
I am using
heedto store a custom trigram index using DUP_SORT. To weight the search results I need to know the size of the key (number of values). I have considered storing it in another table, but apparently that is not necessary as LMDB already stores number of duplicate values and provides access to the count via cursor.Please consider adding support for this functionality. I have drafted this PR as a starting point, but since I am not knowledgeable about the intended direction of the project I cannot judge if this is the way to go.
I do not feel strongly about returning
Ok(1),Err(_)or panicking for incompatible (non-DUP_SORT) tables. I just felt thatOk(1)is trivially correct for such tables.Initial research and patch were assisted by DeepSeek V4 Flash. I have revised heavily.
I did not read contributing guidelines, because I failed to find them.