Skip to content
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

Schema Manager #26

Merged
merged 15 commits into from
Feb 2, 2022
Merged

Schema Manager #26

merged 15 commits into from
Feb 2, 2022

Conversation

billy1624
Copy link
Member

@billy1624 billy1624 commented Nov 25, 2021

@billy1624 billy1624 self-assigned this Nov 25, 2021
@billy1624 billy1624 mentioned this pull request Nov 26, 2021
@billy1624
Copy link
Member Author

I ignored the schema iden in TableRef::SchemaTable for MySQL, so the FROM below will now generate tables instead of information_schema.tables. It cause this error.

pub fn query_tables(&self, schema: SeaRc<dyn Iden>) -> SelectStatement {
type Schema = InformationSchema;
Query::select()
.columns(vec![
TablesFields::TableName,
TablesFields::Engine,
TablesFields::AutoIncrement,
TablesFields::TableCollation,
TablesFields::TableComment,
TablesFields::CreateOptions,
])
.column((
Schema::CollationCharacterSet,
CharacterSetFields::CharacterSetName,
))
.from((Schema::Schema, Schema::Tables))
.left_join(
(Schema::Schema, Schema::CollationCharacterSet),
Expr::tbl(
Schema::CollationCharacterSet,
CharacterSetFields::CollationName,
)
.equals(Schema::Tables, TablesFields::TableCollation),
)
.and_where(Expr::col(TablesFields::TableSchema).eq(schema.to_string()))
.and_where(Expr::col(TablesFields::TableType).eq(TableType::BaseTable.to_string()))
.order_by(TablesFields::TableName, Order::Asc)
.take()
}
}

src/migration/migrator.rs Outdated Show resolved Hide resolved
@tyt2y3
Copy link
Member

tyt2y3 commented Feb 1, 2022

ooops. it does not seem to work :(

@billy1624
Copy link
Member Author

billy1624 commented Feb 1, 2022

ooops. it does not seem to work :(

PgInterval get renamed

@billy1624
Copy link
Member Author

Try cherrypick 0929661

Cargo.toml Show resolved Hide resolved
@tyt2y3 tyt2y3 force-pushed the schema-manager branch 2 times, most recently from 9696ec3 to c7ad70e Compare February 1, 2022 16:48
@tyt2y3
Copy link
Member

tyt2y3 commented Feb 1, 2022

okay so it's completely broken now

@tyt2y3
Copy link
Member

tyt2y3 commented Feb 1, 2022

I think we got a very problematic dependency tree here. As a rule-of-thumb, sea-schema should not directly depends on sea-orm. We might want to split out migration as a crate.

@tyt2y3
Copy link
Member

tyt2y3 commented Feb 2, 2022

Okay so the culprit is that sea-orm and sea-schema each has a dependency on sea-query, currently stuck in a deadlock, and there is no way to upgrade one side without breaking the other side.

We definitely have to re-architect this before releasing. The problem is the API exposed sea-query symbols and that tied things together. Ideally, sea-schema should be able to use a different sea-query version internally from sea-orm without causing trouble.

The migrator should use the sea-query pubed by sea-orm, or ideally, not using the high level sea-query interface at all.

@tyt2y3 tyt2y3 merged commit 764f2aa into master Feb 2, 2022
@tyt2y3 tyt2y3 deleted the schema-manager branch February 2, 2022 03:54
@tyt2y3
Copy link
Member

tyt2y3 commented Feb 2, 2022

Merge to master for now. Note that sea-query is still stuck at 0.20, and we definitely want to upgrade to 0.21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schema Manager
2 participants