Closed Sicheng-Pan closed 5 days ago
Please leverage this checklist to ensure your code review is thorough before approving
This stack of pull requests is managed by Graphite. Learn more about stacking.
Join @Sicheng-Pan and the rest of your teammates on Graphite
Update: Will break this down a bit for easier review
Description of changes
Summarize the changes made by this PR.
SignedRoaringBitmap
which captures the idea of performing conjunction and disjunction on bitmaps without the knowledge of the whole domain. This is useful because in our case the domain is decided by the entries in the database that is not yet deleted, and currently it is costly to scan over the entire collection to determine existing entries. Since we would like to support$ne
and$nin
, whose results depend on the domain, we hope to delay the evaluation of the negation unless it is necessary to do so, and do it only once.MetadataProvider
enum, which represents an universal interface for metadata access. Currently it can be constructed either from materialized logs or aMetadataSegmentReader
.WhereComparison
so that it could be handled with less duplicated code.WhereDocument
as a variant ofWhere
enum, which makes it easier to handle in a consistent way.RoaringMetadataFilter
trait, which helps to evaluate aWhere
clause given aMetadataProvider
.MetadataFilteringOperator
, which is now responsible to produce the exact set ofoffset_id
s that should be returned (withoffset
andlimit
considered).MergeMetadataResultsOperator
, which is now namedHydrateMetadataResultsOperator
and only hydrate theoffset_id
s fromMetadataFilteringOperator
$ne
and$nin
are supported, as part of the refactoring$eq
and$in
)Test plan
How are these changes tested?
Testing is not part of this PR and more tests will be added as follow-up PRs. This PR only aims to pass the existing
cargo test
.[x] Tests pass locally with
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?
$ne
and$nin
has inconsistent behavior with local Chroma. In a follow-up PR we will update the behavior of local Chroma and provide migration guides.