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

Document that schema.sql and data.sql will still be run when using Flyway or Liquibase but discourage their use #20920

Closed
ghost opened this issue Apr 11, 2020 · 15 comments
Labels
type: documentation A documentation update
Milestone

Comments

@ghost
Copy link

ghost commented Apr 11, 2020

Expected Behavior:

  • The sql file "src/main/resources/data.sql" is run independent from the database during start up
  • The sql file "src/test/resources/data.sql" is run independent from the database on test start

Actual Behavior:

  • The sql file "src/main/resources/data.sql" is only run with h2 and not postgres during start up
  • The sql file "src/test/resources/data.sql" is only run with h2 and not postgres on test start

Failed approaches:

  • Use property "spring.datasource.initialization-mode=always"
  • Use property "spring.liquibase.drop-first=true"
  • Use property "spring.jpa.hibernate.ddl-auto=create-drop"

Initial analyse:

  • With postgres there is no "DataSourceSchemaCreatedEvent" so the DataSourceInitializer.initSchema() is never run.

Example-project: https://github.com/hauer-io/liquibase

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 11, 2020
@snicoll
Copy link
Member

snicoll commented Apr 11, 2020

That's the wrong expectation I am afraid. When you run a database migration with Liquibase, this is taking control of the schema and the basic schema.sql & data.sql does not run at all.

Said differently, you can't combine, Execute Liquibase Database Migrations on Startup
and Initialize a database.

The name of the latter section has been renamed in 2.3.x but we could certainly improve the documentation to make that more obvious.

@snicoll snicoll added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 11, 2020
@snicoll snicoll added this to the 2.2.x milestone Apr 11, 2020
@ghost
Copy link
Author

ghost commented Apr 13, 2020

Okay. But then the bug is with H2. Since there both is executed.

@snicoll
Copy link
Member

snicoll commented Apr 13, 2020

I don't know what you mean by "bug is with H2". This project looks wrong to me as you are using two different initialisation system. Either you use liquibase (or flyway) or either you use the schema/data pair.

@snicoll snicoll changed the title Data.sql is ignored with postgres db Document that Flyway/Liquibase are not supposed to be used with the simple schema/data initialization Apr 13, 2020
@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Apr 13, 2020
@ghost
Copy link
Author

ghost commented Apr 13, 2020

Example-project: https://github.com/hauer-io/liquibase
Default profile h2 and postgres profile with postgres. So easy to compare that they behave differently.

@snicoll
Copy link
Member

snicoll commented Apr 13, 2020

Default profile h2 and postgres profile with postgres. So easy to compare that they behave differently.

They behave differently because one is creating the schema via Hibernate and one does not. According to the documentation, embedded databases are created automatically on startup if you're using JPA. That's a sensible default since the database and its content are destroyed at every run.

I've added the following to the configuration on your app:

spring:
  jpa:
    hibernate:
      ddl-auto: none

To make sure that the database is not created by Hibernate and now the two tests fail the same way as the behaviour is the same. This is what I was trying to say with using two competing schema management feature (i.e. schema.sql & data.sql vs. liquibase).

The initialization-mode set to always in the postgres profile is wrong as well since you're using Liquibase.

@snicoll snicoll removed the for: team-attention An issue we'd like other members of the team to review label Apr 13, 2020
@ghost
Copy link
Author

ghost commented Apr 14, 2020

So in case of embedded database I have to explicit disable hibernate creation to avoid confusion? Thanks for your support. I was confused since I just change the database.

@ghost ghost closed this as completed Apr 14, 2020
@ghost ghost reopened this Apr 14, 2020
@ghost
Copy link
Author

ghost commented Apr 14, 2020

According to the documentation for spring.jpa.hibernate.ddl-auto:

DDL mode. This is actually a shortcut for the "hibernate.hbm2ddl.auto" property. Defaults to "create-drop" when using an embedded database and no schema manager was detected. Otherwise, defaults to "none".

So there should be no need to set it none, right?

I updated the example so that it is clear, that both are run in case of tests.

@snicoll
Copy link
Member

snicoll commented Apr 14, 2020

@hauer-io there is no need to close the issue. I've triaged it already.

So there should be no need to set it none, right?

That's the other part of the problem I meant to investigate indeed. We're supposed to detect liquibase & flyway. I intended to figure out what's going on as part of looking at this issue in more details.

Long story short, don't expect to be able to use data.sql with liquibase (or flyway).

@nikmanzotti
Copy link
Contributor

That's the wrong expectation I am afraid. When you run a database migration with Liquibase, this is taking control of the schema and the basic schema.sql & data.sql does not run at all.

Said differently, you can't combine, Execute Liquibase Database Migrations on Startup and Initialize a database.

The name of the latter section has been renamed in 2.3.x but we could certainly improve the documentation to make that more obvious.

@hauer-io, I had similar questions only 10 days after you, that's curious!

After a bit of research, learning and testing, I agree with the current expected behaviour. In the end, I was able to achieve my goals properly setting up Liquibase (e.g. using "context" attributes) and the final solution is quite clean.

However, a bit more clarity in both docs would have surely made everything more straightforward.
I wrote down a few points for possible improvements to the Spring Boot documentation.

9.10.3. Initialize a Database

Maybe it would be worth clarifying the behaviour in the admonition blocks. I was thinking if it. would be better grouping everything in a single block.
a
If I understood correctly, the expected behaviour is the following:

  • schema.sql and data.sql used regardless of JPA/Hibernate --> two scripts are always run if the DataSource is an embedded DB (unless you change spring.datasource.initialization-mode).
  • schema.sql and data.sql used with JPA/Hibernate --> you should choose if using the two scripts or JPA, acting on spring.jpa.hibernate.ddl-auto
  • schema.sql and data.sql used with Liquibase/Flyway --> they should not be used and you cannot change this behaviour setting a property. If you add Liquibase/Flyway and keep the files, they will be completely ignored.

9.10.5. Use a Higher-level Database Migration Tool

Maybe it would be worth adding an admonition block reminding that the migration scripts will be run both for main and tests unless not disabled through the respective properties. If you want to use these tools only for tests and you forget to disable them in the main/resources/application.properties, you may incur in some troubles.

5. Data properties

Maybe it would be worth adding a brief description to the spring.datasource.initialization-mode property, if a user lands here first.

@snicoll
Copy link
Member

snicoll commented Apr 21, 2020

@nikmanzotti Thanks for the feedback. Can you turn that into a PR that updates the documentation?

@nikmanzotti
Copy link
Contributor

@nikmanzotti Thanks for the feedback. Can you turn that into a PR that updates the documentation?

@snicoll done, see #21077

@izeye
Copy link
Contributor

izeye commented May 9, 2020

This seems to be resolved via #21077.

@snicoll
Copy link
Member

snicoll commented May 9, 2020

Thanks @izeye, not 100%. I'd like to investigate what's going on with #20920 (comment)

@wilkinsona
Copy link
Member

wilkinsona commented Jul 30, 2020

Looking at the sample in its current form, Liquibase is being detected and, as a result, Hibernate is not attempting to create the schema. However, this leads to a failure as data.sql is still being run and the cite table upon which it relies does not exist. It is run as a result of the LocalContainerEntityManagerFactoryBean being post-processed after initialization. This contradicts the documentation which states the following:

If you are using a Higher-level Database Migration Tool, like Flyway or Liquibase, you cannot use basic SQL scripts to create and initialize the schema. In this situation, if schema.sql and data.sql are present, they will be ignored. It is not possible to use a Database Migration Tool to manage schema creation, and a basic SQL script to initialize it.

The code doesn't match the documentation updates made in #21077.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Aug 5, 2020
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Aug 5, 2020
@wilkinsona
Copy link
Member

schema.sql is also being run. We're going to correct the documentation for now. We'll change the behaviour in #22741.

@wilkinsona wilkinsona changed the title Document that Flyway/Liquibase are not supposed to be used with the simple schema/data initialization Document that schema.sql and data.sql will still be run when using Flyway or Liquibase but discourage their use Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

6 participants