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

migrate [initOnMigrate, initVersion] should use initVersion when the schema does not exist #842

Closed
marksievers opened this issue Sep 7, 2014 · 6 comments

Comments

@marksievers
Copy link

@marksievers marksievers commented Sep 7, 2014

My setup currently has flyway managing one schema, the one which schema_version is held in eg migration.schema_version. No other tables will exist in this schema, flyway is completely isolated.

It seems like migrate -initOnMigrate=true -initVersion=10 schemas=migration should also honour initVersion when the configured schema does not exist. My experiments seem to show that executing this command on an existing database (devoid of migration of course) will create the schema but then try to migrate forwards over all migrations, ignoring the initVersion.

The effect of this is of course failures when attempting to run migrations against an already 'bootstrapped' but non-flyway-integrated database.

@marksievers
Copy link
Author

@marksievers marksievers commented Sep 26, 2014

Thanks for adding to 3.1, Axel.

This is going to be very slick for our implementation, all that will be required is one command (migrate) to both init and migrate. Makes automation over deployment, developer machines and test environments that much simpler and true to Flyways motto.

Exciting times.

@marksievers
Copy link
Author

@marksievers marksievers commented Sep 30, 2014

@@ -820,7 +829,7 @@ public class Flyway {

                 new DbSchemas(connectionMetaDataTable, schemas, metaDataTable).create();

-                if (!metaDataTable.hasSchemasMarker() && !metaDataTable.hasInitMarker() && !metaDataTable.hasAppliedMigrations()) {
+                if (!metaDataTable.hasInitMarker() && !metaDataTable.hasAppliedMigrations()) {
                     List<Schema> nonEmptySchemas = new ArrayList<Schema>();
                     for (Schema schema : schemas) {
                         if (!schema.empty()) {

Hi Axel,

So I had some time over the weekend to take a look into this, it looks like this line:

new DbSchemas(connectionMetaDataTable, schemas, metaDataTable).create();

influences what will happen in the following if statement by conditionally setting the schemasMarker of the metaData table.

By relaxing that if condition (allowing situations where metaData table may or may not have the SCHEMA row) we are basically saying we are in an initOnMigrate scenario or some erroneous state.

At the moment I can't think of legitimate scenarios that dropping this condition will adversely impact. So far my testing has had the desired effects and the unit tests are still passing. If anything, seems like the schemasMarker check should be pushed down further into the block where initOnMigrate validation is done, but even then it may be superfluous.

I'd like to hear any of your thoughts before I start pushing on with additional unit tests to validate this approach.

@marksievers
Copy link
Author

@marksievers marksievers commented Oct 21, 2014

@axelfontaine Care to comment? I'd like to assist the 3.1 release with a PR addressing this, I have an vested interest :)

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Nov 25, 2014

HI Mark,

sorry for the late reply. This somehow fell off my radar.

I'm looking into this today.

Cheers
Axel

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Nov 25, 2014

This is now fixed. Under normal circumstances I would have gladly taken a PR. However, since I had the init -> baseline change coming, this would have caused you more trouble than it would have been worth.

Cheers
Axel

P.S.: Feel free to give the new release a spin and check if it all works for you.

@marksievers
Copy link
Author

@marksievers marksievers commented Nov 26, 2014

Hey Axel,

Tested and looks to work as expected, thanks a lot!

Mark

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.