Migrations on SQL Server by pimeys · Pull Request #1372 · prisma/prisma-engines

pimeys · 2020-11-16T19:04:23Z

Implements migrations on Microsoft SQL Server 2017 and 2019. Probably works with earlier versions from at least 2012, but we test only against 2017 and 2019. Removes the gatekeeper for SQL Server from the Migration Engine, and all hacky test macros, unifying the Microsoft databases to the test_all_connectors.

Closes: #813

I'm 99% sure this one fixes issues with raw queries containing stored procedures and transactions (reported in our Slack), and some issues we might've fixed with this PR include:

Closes: prisma/prisma#4333
Closes: prisma/prisma#4250

migration-engine/connectors/sql-migration-connector/src/sql_schema_differ.rs

query-engine/connector-test-kit/src/test/scala/writes/relations/RelationDesignSpec.scala

...ector-test-kit/src/test/scala/writes/topLevelMutations/DeleteManyMutationRelationsSpec.scala

...connector-test-kit/src/test/scala/writes/topLevelMutations/DeleteMutationRelationsSpec.scala

...connector-test-kit/src/test/scala/writes/topLevelMutations/NonEmbeddedUpsertDesignSpec.scala

...nector-test-kit/src/test/scala/writes/nestedMutations/NestedAtomicNumberOperationsSpec.scala

libs/sql-schema-describer/src/mssql.rs

tomhoule

Just starting to review

tomhoule · 2020-12-01T14:26:55Z

libs/test-macros/src/lib.rs

-#[proc_macro_attribute]
-pub fn test_each_connector_mssql(attr: TokenStream, input: TokenStream) -> TokenStream {
-    test_each_connector::test_each_connector_impl(attr, input, true)
+    test_each_connector::test_each_connector_impl(attr, input)


migration-engine/connectors/sql-migration-connector/src/flavour/mssql.rs

libs/sql-schema-describer/src/lib.rs

libs/sql-schema-describer/src/walkers.rs

tomhoule

Leaving another batch of comments before I move to reviewing the diffing logic. Looking good so far!

tomhoule · 2020-12-02T14:30:02Z

libs/sql-schema-describer/src/walkers.rs

+            .table_walkers()
+            .filter(move |t| t.table_index() != table_index)
+            .flat_map(|t| t.foreign_keys())
+            .filter(move |fk| fk.referenced_table().table_index() == table_index)


migration-engine/connectors/sql-migration-connector/src/flavour/mssql.rs

tomhoule · 2020-12-02T14:57:10Z

migration-engine/connectors/sql-migration-connector/src/sql_database_step_applier.rs

+                .previous()
+                .table_walker_at(drop_index.table_index)
+                .index_at(drop_index.index_index),
+        )],


tomhoule · 2020-12-02T15:02:17Z

migration-engine/connectors/sql-migration-connector/src/sql_migration.rs

-    pub name: String,
+    pub table_index: usize,
+    pub index_index: usize,
 }


migration-engine/connectors/sql-migration-connector/src/sql_imperative_migration_persistence.rs

tomhoule · 2020-12-02T15:04:22Z

migration-engine/connectors/sql-migration-connector/src/sql_renderer.rs


    /// Render a `DropIndex` step.
-    fn render_drop_index(&self, drop_index: &DropIndex) -> String;
+    fn render_drop_index(&self, index: &IndexWalker<'_>) -> String;


.../migration-engine-tests/tests/diagnose_migration_history/diagnose_migration_history_tests.rs

tomhoule · 2020-12-03T11:14:55Z

libs/sql-schema-describer/src/walkers.rs

+            .field("table_index", &self.table_index)
+            .finish()
+    }
+}


💯

Thanks for adding these

tomhoule

a p p r o v e d

migration-engine/connectors/sql-migration-connector/src/sql_schema_differ.rs

migration-engine/migration-engine-tests/tests/migration_tests.rs

For SQL Server, we want to recreate the primary key before recreating a column that's part of it. We do not want this behavior on other databases, thus adding a new flag to the flavour.

Much nicer and following the crate standards this way, without needing to add a completely unnecessary constructor creation...

Also fixes one test to not re-create the whole table (to spawn warnings on SQL Server). The barrel `types::primary` will make the column autocomplete and the next migration removes the identity, spawning a recreation of the whole table.

pimeys force-pushed the mssql-migrations branch 5 times, most recently from c3f3068 to 494472e Compare November 20, 2020 16:46

pimeys force-pushed the mssql-migrations branch 6 times, most recently from f6903d5 to 8996dd9 Compare December 1, 2020 12:31

pimeys changed the title ~~WIP: Migrations on SQL Server~~ Migrations on SQL Server Dec 1, 2020

pimeys added this to the 2.13.0 milestone Dec 1, 2020

pimeys force-pushed the mssql-migrations branch from 8996dd9 to 8126b48 Compare December 1, 2020 12:41

pimeys commented Dec 1, 2020

View reviewed changes

migration-engine/connectors/sql-migration-connector/src/sql_schema_differ.rs Outdated Show resolved Hide resolved

pimeys force-pushed the mssql-migrations branch 3 times, most recently from 7e4eed5 to 44880c6 Compare December 1, 2020 13:06

pimeys commented Dec 1, 2020

View reviewed changes

migration-engine/connectors/sql-migration-connector/src/sql_schema_differ.rs Show resolved Hide resolved

pimeys force-pushed the mssql-migrations branch 2 times, most recently from 2c1857b to 39c0ca0 Compare December 1, 2020 13:46

pimeys commented Dec 1, 2020

View reviewed changes

libs/sql-schema-describer/src/mssql.rs Show resolved Hide resolved

tomhoule reviewed Dec 1, 2020

View reviewed changes

migration-engine/connectors/sql-migration-connector/src/flavour/mssql.rs Show resolved Hide resolved

pimeys force-pushed the mssql-migrations branch 4 times, most recently from 86a2d32 to 8230965 Compare December 2, 2020 09:00

tomhoule reviewed Dec 2, 2020

View reviewed changes

libs/sql-schema-describer/src/lib.rs Show resolved Hide resolved

tomhoule reviewed Dec 2, 2020

View reviewed changes

libs/sql-schema-describer/src/walkers.rs Outdated Show resolved Hide resolved

tomhoule reviewed Dec 2, 2020

View reviewed changes

.../migration-engine-tests/tests/diagnose_migration_history/diagnose_migration_history_tests.rs Outdated Show resolved Hide resolved

tomhoule reviewed Dec 3, 2020

View reviewed changes

libs/sql-schema-describer/src/walkers.rs

.field("table_index", &self.table_index)

.finish()

}

}

Copy link

Contributor

tomhoule Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Thanks for adding these

tomhoule approved these changes Dec 3, 2020

View reviewed changes

Julius de Bruijn and others added 9 commits December 3, 2020 16:28

Migrations on Microsoft SQL Server

1522091

Make the shadow database cleanup more robust

ad627ae

Primary key recreation flag for column recreate

975c307

For SQL Server, we want to recreate the primary key before recreating a column that's part of it. We do not want this behavior on other databases, thus adding a new flag to the flavour.

Make alter table to work as a module

293051a

Much nicer and following the crate standards this way, without needing to add a completely unnecessary constructor creation...

Hide stuff in SqlSchema debug, change few fields

8833685

Flavour out the imperative migrations table

7892a7a

Add bigger timeouts on tests

053a28b

Warn about NotCastable column changes

ae2f91a

Add back assertions

51e95d1

Also fixes one test to not re-create the whole table (to spawn warnings on SQL Server). The barrel `types::primary` will make the column autocomplete and the next migration removes the identity, spawning a recreation of the whole table.

pimeys force-pushed the mssql-migrations branch from cdb4b3d to 51e95d1 Compare December 3, 2020 15:29

Julius de Bruijn added 3 commits December 3, 2020 16:44

Fix non-castable warnings

031391c

Cargo fmt

2d15bec

Update base64 crate

e0328a1

pimeys merged commit 06e7855 into master Dec 3, 2020

pimeys deleted the mssql-migrations branch December 3, 2020 16:34

Conversation

pimeys commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomhoule left a comment

Choose a reason for hiding this comment

Uh oh!

tomhoule Dec 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomhoule left a comment

Choose a reason for hiding this comment

Uh oh!

tomhoule Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomhoule Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

tomhoule Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomhoule Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomhoule Dec 3, 2020

Choose a reason for hiding this comment

Uh oh!

tomhoule left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pimeys commented Nov 16, 2020 •

edited

Loading