-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(db): initialize db with tables #13130
feat(db): initialize db with tables #13130
Conversation
crates/storage/db/src/tables/mod.rs
Outdated
type TableIter: Iterator<Item = Box<dyn TableMetadata>>; | ||
|
||
/// Returns an iterator over the tables. | ||
fn tables() -> Self::TableIter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can just return Boxed iter of Table instead, no need for the associated type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean directly using Box<dyn Iterator<Item = Box<dyn TableMetadata>>>
as return type for the tables
function and remove the TableIter
associated type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or it could be like this:
pub trait TableSet {
/// Returns an iterator over the tables
fn tables() -> Box<dyn Iterator<Item = Box<dyn TableMetadata>>>;
}
and then the impl inside the tables!
macro:
impl TableSet for Tables {
fn tables() -> Box<dyn Iterator<Item = Box<dyn TableMetadata>>> {
Box::new(Self::ALL.iter().map(|table| Box::new(*table) as Box<dyn TableMetadata>))
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
next step would be repeating the same thing for functions that call create_tables, like
https://github.com/paradigmxyz/reth/blob/main/crates/storage/db/src/mdbx.rs#L31-L39
essentially walking up the call graph
great, im gonna work on it |
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Supersedes #13125 (review)
@mattsse