[1/n] move db schema into its own crate #7880
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.6-beta.1
sunshowers
commented
Mar 27, 2025
| }; | ||
| } | ||
|
|
||
| define_enums! { |
There was a problem hiding this comment.
Enum definitions are here.
| @@ -6,6 +6,8 @@ | |||
| //! | |||
| //! NOTE: Should be kept up-to-date with dbinit.sql. | |||
|
|
|||
| use diesel::{allow_tables_to_appear_in_same_query, joinable, table}; | |||
There was a problem hiding this comment.
This is the schema -- it's moved with some minor changes.
| #[derive(SqlType, Debug)] | ||
| #[diesel(postgres_type(name = "dns_group", schema = "public"))] | ||
| pub struct DnsGroupEnum; | ||
| DnsGroupEnum: |
There was a problem hiding this comment.
Example of a conversion.
Comment on lines
300
to
+302
| macro_rules! impl_enum_type { | ||
| ( | ||
| $(#[$enum_meta:meta])* | ||
| pub struct $diesel_type:ident; | ||
| $diesel_type:ident: |
There was a problem hiding this comment.
These macros change slightly.
smklein
reviewed
Mar 27, 2025
Created using spr 1.3.6-beta.1
sunshowers
changed the title
|
(need to fix up tests still) |
smklein
reviewed
Mar 31, 2025
| @@ -86,3 +95,11 @@ define_enums! { | |||
| VpcRouterKindEnum => "vpc_router_kind", | |||
| ZoneTypeEnum => "zone_type", | |||
| } | |||
|
|
|||
| define_enums_non_static! { | |||
There was a problem hiding this comment.
I'm assuming this is just 'moving' the definitions from before, but is there a reason we have these enums as "non-static" and the others as static?
There was a problem hiding this comment.
I'm curious, but I haven't investigated in depth -- wanted to keep this PR without any functional changes.
There was a problem hiding this comment.
#7897 is a followup which makes that change.
sunshowers
changed the title
Created using spr 1.3.6-beta.1
smklein
approved these changes
Mar 31, 2025
sunshowers
deleted the
sunshowers/spr/rfc-move-db-schema-into-its-own-crate
branch
sunshowers
added a commit
that referenced
this pull request
Apr 1, 2025
…7897) This is meant to be false if and only if types don't match 1:1 to queries, e.g. with CTEs. These types do match 1:1 to queries, so using `HAS_STATIC_QUERY_ID` should be fine. There's a small possibility some code was inadvertently depending on `HAS_STATIC_QUERY_ID` being false, but tests should hopefully catch that. Depends on #7880.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on Diesel-related code, I noticed that schema generation is both
very large (~450k lines of code currently), and also can be separated out
into its own crate. Spent a couple of hours doing so and I think I quite like
the result! The most important thing here is that the schema code is completely
independent of the rest of Omicron, so it can be compiled in parallel with
crates like omicron-common.
For me, a full
cargo check --all-targetsin Omicron goes from 2m04s to 1m51s,which is a really nice speedup. Incremental builds should also be faster due to
the parallelization and reduction of crate size. (But this doesn't really touch
nexus-db-queries, which is the worst offender at the moment.)