-
Notifications
You must be signed in to change notification settings - Fork 491
refactor: write bitmap index statistics in file instead #5251
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
base: main
Are you sure you want to change the base?
Changes from all commits
8fc6f89
ea6d9b7
feaa02e
89e2360
b4e2ea2
bf49f79
6f5c835
d80c5fd
22206e7
5426f1f
5bfbf4d
417971e
c1ef0bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ use crate::registry::IndexPluginRegistry; | |
| use crate::{ | ||
| frag_reuse::FragReuseIndex, | ||
| scalar::{expression::ScalarQueryParser, CreatedIndex, IndexStore, ScalarIndex}, | ||
| IndexType, | ||
| }; | ||
|
|
||
| pub const VALUE_COLUMN_NAME: &str = "value"; | ||
|
|
@@ -129,6 +130,9 @@ pub trait ScalarIndexPlugin: Send + Sync + std::fmt::Debug { | |
| /// Returns true if the index returns an exact answer (e.g. not AtMost) | ||
| fn provides_exact_answer(&self) -> bool; | ||
|
|
||
| /// Returns the index type for this plugin | ||
| fn index_type(&self) -> IndexType; | ||
|
Comment on lines
+133
to
+134
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Say someone creates a new custom index plugin, that doesn't fit any of the enum variant we have built into our library. What do they return here?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of this, could we have something like: impl IndexType {
fn try_from_pb_name(pb_name: &str) -> Option<Self> { ... }
}Seems like we should be able to derive from the name, right?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @westonpace in case you have any thoughts on this
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. We should be trying to move away from With the introduction of The index statistics currently has a
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added, PTAL. |
||
|
|
||
| /// The version of the index plugin | ||
| /// | ||
| /// We assume that indexes are not forwards compatible. If an index was written with a | ||
|
|
@@ -156,6 +160,15 @@ pub trait ScalarIndexPlugin: Send + Sync + std::fmt::Debug { | |
| cache: &LanceCache, | ||
| ) -> Result<Arc<dyn ScalarIndex>>; | ||
|
|
||
| /// Optional hook allowing a plugin to provide statistics without loading the index. | ||
| async fn load_statistics( | ||
| &self, | ||
| _index_store: Arc<dyn IndexStore>, | ||
| _index_details: &prost_types::Any, | ||
| ) -> Result<Option<serde_json::Value>> { | ||
| Ok(None) | ||
| } | ||
|
|
||
| /// Optional hook that plugins can use if they need to be aware of the registry | ||
| fn attach_registry(&self, _registry: Arc<IndexPluginRegistry>) {} | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.