-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: split db abstraction into new crate #8594
Conversation
594e640
to
acd15ac
Compare
Removed dbi caching in 37fca74 since mdbx already does this and it would complicate doing this split if we wanted to keep it around (and complicate adding support for custom tables), see https://erthink.github.io/libmdbx/group__c__dbi.html#ga9bef4a9fdf27655e9343bbbf8b6fc5a1
|
@@ -1,5 +1,4 @@ | |||
//! Storage sharded key | |||
|
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.
why :(
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.
its a conversation starter
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.
👎
impl $crate::table::Table for $name { | ||
const TABLE: Tables = Tables::$name; | ||
impl reth_db_api::table::Table for $name { | ||
const NAME: &'static str = table_names::$name; |
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.
Does this also address @prestwich's limitation of not being able to define custom tables in ExEx?
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.
not entirely, most of the code still assumes a fixed set of tables by using the generated Tables
enum
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 LOC was one of the primary issues and is now resolved, but i haven't identified others yet
0f0fe5c
to
9762234
Compare
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.
one suggestion when doing these kinds of refactors, I try to avoid most dep changes on the initial PR with re-exports and then update them once the pr is merged because doing it like this can result of a lot of conflicts and makes the initial pr harder to review
Splits the db abstraction into a new crate
reth-db-api
.Todo:
TABLE
associated type inTable
(otherwise this PR is not possible)reth-db-api
(e.g.impl_uints!(u8, u16, ...)
)Ref #8512