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.Up() errors out if the latest schema is in use #100

Closed
jon-whit opened this issue Sep 7, 2018 · 4 comments
Closed

Migrate.Up() errors out if the latest schema is in use #100

jon-whit opened this issue Sep 7, 2018 · 4 comments

Comments

@jon-whit
Copy link

jon-whit commented Sep 7, 2018

Migrate.Up() errors out if the currently active schema matches the latest schema version.

Steps to Reproduce
My migrations look like this:

1_initialize_tables.down.sql

DROP roles_test;
DROP permissions_test;

1_initialize_tables.up.sql

CREATE TABLE permissions_test (
    definition STRING(MAX) NOT NULL,
) PRIMARY KEY (definition);
CREATE TABLE roles_test (
    `parent` STRING(MAX) NOT NULL,
    role_id STRING(MAX) NOT NULL,
) PRIMARY KEY (`parent`, role_id);

I have code that looks like this:

migrator, err := migrate.New(
 "file://migrations",
  "spanner://projects/my-project/instances/main-instance/databases/my-database")
if err != nil {
  log.Fatalf("Unable to instantiate the database schema migrator - %v", err)
}   

// Migrate up to the latest active version
if err := migrator.Up(); err != nil {
  log.Fatalf("Unable to migrate up to the latest database schema - %v", err)
}

And I get the error:

Unable to migrate up to the latest database schema - no change

Expected Behavior
I would expect Migrate.Up() to not return an error if the latest schema is in use. At minimum it should return an ErrAlreadyUpToDate or something similar. If this is not the intended use of this method, better documentation should be made to demonstrate how this functionality is best achieved with the current migrate API.

Migrate Version
v3.5.1 - Go Client

Loaded Source Drivers
file

Loaded Database Drivers
spanner

Additional context
I've ran Migrate.Up() once which created an entry in the SchemaMigrations table and a row now exists that looks like this:

| Version | Dirty |
|---------|-------|
| 1       | False |

The second time I start my service up Migrate.Up() fails because the currently active version is the latest one.

@dhui
Copy link
Member

dhui commented Sep 7, 2018

Have you asserted that err != migrate.ErrNoChange?

@jon-whit
Copy link
Author

jon-whit commented Sep 7, 2018

@dhui Oh man, I didn't even see that! I'm sorry. That fixes it!

@jon-whit jon-whit closed this as completed Sep 7, 2018
@hefekranz
Copy link

I'm sorry, but I still don't see how "no change" should be an error. And checking the error for a certain type does not seem like the right solution to me here. Is there any specific reason it is done like that? From my understanding it could just be logged away.

@dhui
Copy link
Member

dhui commented May 16, 2019

I'm sorry, but I still don't see how "no change" should be an error.
...
Is there any specific reason it is done like that?

This decision predates me.

And checking the error for a certain type does not seem like the right solution to me here.

This pattern is also used in the io.Reader interface e.g. io.EOF

From my understanding it could just be logged away.

Unfortunately, logging in Go leaves a lot to be desired. e.g. every package has their own way of handling logging... Until there's a standard configurable way to log messages at different levels, migrate will avoid logging in the library and only use errors.

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

No branches or pull requests

3 participants