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

Upgrade to Hibernate 5.1 #2763

Closed
snicoll opened this issue Apr 2, 2015 · 37 comments
Closed

Upgrade to Hibernate 5.1 #2763

snicoll opened this issue Apr 2, 2015 · 37 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@snicoll
Copy link
Member

snicoll commented Apr 2, 2015

Spring Framework 4.2 will add support for Hibernate 5 which will lead to auto-configuration updates on our side.

@snicoll snicoll added the type: enhancement A general enhancement label Apr 2, 2015
@snicoll snicoll added this to the 1.3.0 milestone Apr 2, 2015
@snicoll
Copy link
Member Author

snicoll commented May 27, 2015

With a pure JPA-based implementation, it looks like we don't have to do anything. Let's reopen if we think we need to change something.

@snicoll snicoll closed this as completed May 27, 2015
@snicoll snicoll removed this from the 1.3.0.RC1 milestone May 27, 2015
@snicoll
Copy link
Member Author

snicoll commented Jun 8, 2015

Actually, Hibernate 5 has a breaking change regarding the naming strategy support so we should at least track that.

@snicoll snicoll reopened this Jun 8, 2015
@snicoll snicoll added this to the 1.3.0 milestone Jun 8, 2015
@Polve
Copy link

Polve commented Aug 17, 2015

The new naming strategy seems to create a regression as documented here:
https://hibernate.atlassian.net/browse/HHH-9908

Maybe due to this deprecated call:

2015-08-15 09:28:43.818  WARN 28766 --- [           main] org.hibernate.orm.deprecation            : HHH90000006: Attempted to specify unsupported NamingStrategy via setting [hibernate.ejb.naming_strategy]; NamingStrategy has been removed in favor of the split ImplicitNamingStrategy and PhysicalNamingStrategy; use [hibernate.implicit_naming_strategy] or [hibernate.physical_naming_strategy], respectively, instead.

@wilkinsona wilkinsona modified the milestones: 1.3.0.RC1, 1.3.0 Aug 18, 2015
@snicoll snicoll self-assigned this Aug 19, 2015
@snicoll snicoll modified the milestones: 1.3.0.RC1, 1.3.0.M5 Aug 26, 2015
snicoll added a commit to snicoll/spring-boot that referenced this issue Sep 8, 2015
Hibernate 5 brings a breaking change by setting the
`hibernate.id.new_generator_mappings` property to `true` by default. This
does not use a sequence for a `Long` PK and therefore any SQL not
providing a value for the primary key will break.

Also, Hibernate 5 changed its naming strategy API so we'll have to detect
that in `JpaProperties#Hibernate` and set the appropriate value.

See spring-projectsgh-2763
@philwebb philwebb modified the milestones: 1.4.0, 1.3.0.RC1 Sep 28, 2015
@Polve
Copy link

Polve commented Oct 21, 2015

I've tested with spring boot 1.3.0.RC1 and I still get the same warning.

Also, when autogenerating tables I see each row two times for each table, maybe there is something suboptimal there too?

2015-10-21 16:24:57.195  INFO 31799 --- [           main] rmationExtractorJdbcDatabaseMetaDataImpl : HHH000262: Table not found: ab
2015-10-21 16:24:57.196  INFO 31799 --- [           main] rmationExtractorJdbcDatabaseMetaDataImpl : HHH000262: Table not found: ab
2015-10-21 16:24:57.305  INFO 31799 --- [           main] rmationExtractorJdbcDatabaseMetaDataImpl : HHH000262: Table not found: ablabel
2015-10-21 16:24:57.306  INFO 31799 --- [           main] rmationExtractorJdbcDatabaseMetaDataImpl : HHH000262: Table not found: ablabel
2015-10-21 16:24:57.416  INFO 31799 --- [           main] rmationExtractorJdbcDatabaseMetaDataImpl : HHH000262: Table not found: account
2015-10-21 16:24:57.417  INFO 31799 --- [           main] rmationExtractorJdbcDatabaseMetaDataImpl : HHH000262: Table not found: account

@Polve
Copy link

Polve commented Oct 21, 2015

What worries me is using a naming strategy that will change when that problem will be fixed, and hence render uncompatible the autogenerated schema

@snicoll
Copy link
Member Author

snicoll commented Oct 21, 2015

I think the main problem is that the Hibernate team has no concrete answer for that either. If you look at the naming strategy we have for 4.3, it's basically org.hibernate.cfg.ImprovedNamingStrategy and there is no counter part in 5.0. As far as I understand, the one that is closer is org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyHbmImpl and there is legacy in the name.

If you are relying on that naming strategy (with the little fix that we have added in SpringNamingStrategy) I am afraid it's more an hibernate migration problem than a Spring Boot problem.

@holgerstolzenberg
Copy link

I tried to use Hibernate5 with spring-boot-1.3.RC1 for leveraging Oracle12cDialect and native java.time support. It took me quite some time (and pain) to figure it out. The main problem was related to the naming strategy problematic. The things that worked really good was just setting spring.version and hibernate.version in my gradle.propertiesfile and add dependency org.hibernate:hibernate-java8. Libraries were pulled in correctly, no classpath issues.

Then I removed all the Jadira stuff and tried to start to app. Bang. The problem is that the NamingStrategy from configuration key spring.jpa.hibernate.naming-strategy from the HibernateJpaAutoConfiguration class has no effect, as the according Hibernate property has been deprecated. You may also see:

https://jira.spring.io/browse/DATAJPA-777

Hibernate5 now uses two types of naming strategies (org.hibernate.boot.model.naming.ImplicitNamingStrategy & org.hibernate.boot.model.naming.PhysicalNamingStrategy). In order to still leverage Hibernate autoconfiguration, I had to add the following bean definition to my application context:

  @Bean
  @Primary
  public static JpaProperties jpaProperties() {
    final Map<String, String> hibernateConfig = newHashMap();
    hibernateConfig.put("hibernate.implicit_naming_strategy",
        "org.hibernate.boot.model.naming.ImplicitNamingStrategyLegacyHbmImpl");
    hibernateConfig.put("hibernate.physical_naming_strategy",
        "com.anyapp.CustomNamingStrategy");

    final JpaProperties jpaProperties = new JpaProperties();
    jpaProperties.setProperties(hibernateConfig);
    jpaProperties.setDatabase(ORACLE);
    jpaProperties.setDatabasePlatform(ORACLE12C_DIALECT);
    jpaProperties.setGenerateDdl(false);
    jpaProperties.setShowSql(false);
    return jpaProperties;
  }

Therefore I am loosing spring.jpa.* configuration from the application.yml which is no big point here, as I do not really need it to be configurable here. If so I could inject the values into the bean factory method.

The most annoying part was to find out which naming strategies to apply in what way and the fact that I had to implement CustomNamingStrategy on my own, as Hibernate does not provide a PhysicalNamingStrategy that matches the naming of legacy ImprovedNamingStrategy.

The custom naming strategy is nearly the same as here...
http://stackoverflow.com/questions/32165694/spring-hibernate-5-naming-strategy-configuration
... just replaced the regex stuff with Guava's CaseFormat.

The CustomNamingStrategy is just a simple implementation. A verified solution by Spring(-Boot) team is highly appreciated.

Hope this helps getting Hibernate5 compat into 1.3.

Another helper may be:
http://stackoverflow.com/questions/31358356/how-to-migrate-a-hibernate-namingstrategy-to-implicitphysicalnamingstrategy

@snicoll
Copy link
Member Author

snicoll commented Nov 6, 2015

Thanks for the detailed analysis. We know about the naming strategy (see the comments before yours).

I am not sure I understand why you have tot hack JpaProperties like that but if you want to keep support for spring.jpa.* you simply need to add @ConfigurationProperties to your custom bean definition. That is:

@Bean
@Primary
@ConfigurationProperties(prefix = "spring.jpa")
public static JpaProperties jpaProperties() {
...
}

@holgerstolzenberg
Copy link

I overwrote the bean in order to be able to specify the new naming strategy key names:

  • hibernate.implicit_naming_strategy
  • hibernate.physical_naming_strategy

As I understand, they would not be applied by HibernateJpaAutoConfiguration or is there some kind of lax binding I am overlooking.

If I can achieve that from application.yml, it would be my preferred way.

@snicoll
Copy link
Member Author

snicoll commented Nov 6, 2015

Well either do what I wrote above (have you seen it?) or ditch your override and add this to your config

spring:
  jpa:
    properties:
       hibernate.implicit_naming_strategy: xyz
       hibernate.physical_naming_strategy: abc

@holgerstolzenberg
Copy link

That works. Thats a lot.

@philwebb philwebb added this to the 1.4.0.M1 milestone Jan 7, 2016
snicoll added a commit to snicoll/spring-boot that referenced this issue Apr 4, 2016
This commit updates our test suite to be able to run on both Hibernate 4
and Hibernate 5. Two important changes made this change necessary:

* Spring Boot no longer provides a naming strategy for Hibernate 5 so it
defaults to the one provided by Hibernate. The main difference is the
physical name that is provided for camel case properties
* If the `@GeneratedValue` uses the `AUTO` strategy, Hibernate 5 uses
sequence by default. This commit forces the strategy to `IDENTITY` in
order to be compliant with what Hibernate 4 does by default

See spring-projectsgh-2763
@snicoll
Copy link
Member Author

snicoll commented Apr 4, 2016

I've added a proposal in ca023b5 (which is mainly dealing with the naming strategy) and I've also extended the support by "fixing" our test suite to work also with Hibernate 5 (see f3c3119). If you take the state of that branch you can build with -Dhibernate.version=5.0.9.Final. We're not promoting Hibernate 5 by default just yet.

Most of these changes are due to the naming strategy change. Maybe we shouldn't rely on default column names after all?

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Apr 4, 2016
philwebb added a commit that referenced this issue Apr 6, 2016
Upgrade to Hibernate 5.1, whilst still retaining compatibility with
Hibernate 4.3. This commit introduces the following changes:

* Add SpringPhysicalNamingStrategy to provides lowercase/underscore
  table names support. This should be equivalent to the previous
  SpringNamingStrategy that was used with Hibernate 4. No
  ImplicitNamingStrategy is provided since the Hibernate 5 defaults
  appear to be roughly equivalent to the conventions used in Spring
  Boot 1.3
  spring.jpa.hibernate.naming.

* Migrate  `spring.jpa.hibernate.naming-strategy` to
  `spring.jpa.hibernate.naming.strategy` and provide additional
  properties for physical and implicit.

* Add `spring.jpa.hibernate.use-new-id-generator-mappings` property and
  default to `false` when on Hibernate 5 to retain back compatibility.

See gh-2763
philwebb added a commit that referenced this issue Apr 6, 2016
Add a sample showing how Hibernate 4 can be used if necessary.

See gh-2763
philwebb added a commit that referenced this issue Apr 6, 2016
@philwebb
Copy link
Member

philwebb commented Apr 6, 2016

I think this is basically fixed but I'm leaving it open for team discussion and because we'll need some release notes. The plan is:

  • Spring Boot 1.4 will use Hibernate 5.1 by default
  • The SpringNamingStrategy is deprecated because it's no longer used.
  • We're not going to provide out own ImplicitNamingStrategy because Hibernate's defaults are very close to (if not identical) to Spring Boot 1.3 defaults.
  • We are going to provide a SpringPhysicalNamingStrategy to restore lowercase/underscore sanity to names.
  • We're going to default hibernate.id.new_generator_mappings to false on Hibernate 5 so that people can upgrade easily. We might want to change this default in Spring Boot 2.0 but I don't think we should for 1.4
  • If the upgrade breaks things, we have a downgrade to Hibernate 4.3 option. The old code remains the same and there's a Hibernate 4 sample to test things.

@ceefour
Copy link
Contributor

ceefour commented Apr 6, 2016

I like @philwebb's plan. We're already using Hibernate 5.1 due to bug with 4.3's component path naming strategy that is fixed in 5.1.

This will make us upgrading to boot 1.4 easily. And we always use the component-path physical naming strategy anyway.

@philwebb
Copy link
Member

philwebb commented Apr 6, 2016

We need to not deprecate SpringNamingStrategy

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Apr 6, 2016
@snicoll snicoll closed this as completed in fbe53be Apr 8, 2016
@philwebb philwebb changed the title Support for Hibernate 5 Upgrade to Hibernate 5.1 Apr 13, 2016
@Polve
Copy link

Polve commented Apr 13, 2016

I tried to upgrade to spring boot 1.4.0M2 my app that is using 1.3.x where I was already using hibernate5 but a new naming strategy was adopted and all my tables were modified (and of course everything stopped working).
Which configuration options must I set to revert back to the old strategy?

@snicoll
Copy link
Member Author

snicoll commented Apr 13, 2016

@Polve check the release notes

I am not sure I understand what you mean here. Upgrading to Spring Boot will not modify your tables. We didn't support Hibernate 5 previously so if you were specifying an implicit or physical strategy before, you should have done so in your own code. The alternative is that you were using Hibernate 5 default and those breaks compared to what you can experience with Boot and Hibernate 4.3.

The easiest way to know which properties are available is to use the auto-completion in your IDE or check the appendix.

@Polve
Copy link

Polve commented Apr 14, 2016

But the problem I'm having is exactly that by upgrading from Hibernate 5.1+spring boot 1.3 to hibernate 5.1+spring boot 1.4M2 the naming strategy changed substantially and a field named "twoWords" is now mapped to "two_words", breaking havoc on the application.

I understand this is related to the new physical-strategy and implicit-strategy handling, the problem being that I'm unable to find any documentation on what are the correct values for those fields to restore the old behaviour (the default used in boot 1.3).

@snicoll
Copy link
Member Author

snicoll commented Apr 14, 2016

@Polve I think there is a misunderstanding of what the issue is. You were using Hibernate 5 before we actually support it so you got the default Hibernate 5 provides. These breaks compared to Hibernate 4 with Spring Boot. What we did is making sure that those who are upgrading can do so safely (If you take Hibernate 4 and Spring Boot, you'll get two_words not twoWords).

If you want to stick with the default Hibernate 5 provides, just configure the naming strategy accordingly:

spring.jpa.hibernate.naming.physical-strategy=org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl

the old behaviour (the default used in boot 1.3).

Just to make that clear, this is the hibernate 5 default that you got by using it before we actually support it. It's not something that we changed, we didn't support it before. I hope that clarifies.

@ceefour
Copy link
Contributor

ceefour commented Apr 14, 2016

@snicoll https://docs.spring.io/spring-boot/docs/1.4.0.M2/reference/html/common-application-properties.html states:

spring.jpa.hibernate.naming.physical-strategy= # Hibernate 5 physical naming strategy fully qualified name.
spring.jpa.hibernate.naming.strategy= # Hibernate 4 naming strategy fully qualified name. Not supported with Hibernate 5.

which means the default physical strategy is empty, which should mean Hibernate's default, not Spring Boot's.

If Spring Boot 1.4 specifies a physical strategy, the appendix should mention it to be clear.

Same goes for Hibernate4's naming.strategy, since Spring Boot specifies the SpringNamingStrategy this should be mentioned in the appendix.

So even if the behavior is "by design", I think there's a documentation bug here.

@Polve
Copy link

Polve commented Apr 14, 2016

Thanks @snicoll for the details: that was exactly what I was looking for.

I agree with @ceefour : I got confused because I thought I was using hibernate's default, that's why by already using hibernate 5.1 on boot 1.3 I expected to work the same way with boot 1.4.

Thanks to both for the valuable infos.

@snicoll
Copy link
Member Author

snicoll commented Apr 14, 2016

@ceefour we can't document everything in the appendix. There is an algorithm to figure out what to do there and it's documented in the release notes. Granted, we should add that to the doc but simply putting the value in the appendix does not reflect what's happening either.

Please don't use the appendix to attempt to figure out what happens.

which means the default physical strategy is empty, which should mean Hibernate's default, not Spring Boot's.

There are many places where we compute the default based on the environment and we can't write "a default " because it simply does not exist. Don't take the shortcut of no-value in the appendix == no default.

@ceefour
Copy link
Contributor

ceefour commented Apr 14, 2016

@snicoll After reading the release notes I guess the behavior is the following as Spring Boot's defaults:

spring.jpa.hibernate.naming.physical-strategy=org.springframework.boot.orm.jpa.hibernate.SpringPhysicalNamingStrategy # Hibernate 5 physical naming strategy fully qualified name.
spring.jpa.hibernate.naming.strategy=org.springframework.boot.orm.jpa.hibernate.SpringNamingStrategy # Hibernate 4 naming strategy fully qualified name. Not supported with Hibernate 5.

Other properties mention their defaults, e.g.:

server.port=8080 # Server HTTP port.

I don't understand why Spring Boot's naming strategy defaults can't be mentioned in the appendix. Leaving it empty as it is now is even more confusing.

@snicoll
Copy link
Member Author

snicoll commented Apr 15, 2016

No it's not. I think I already explained why above so I am afraid I am out of arguments here. We only set the latter if you're running Hibernate 4 (otherwise it's not set at all). The former is only set if you haven't set it any other way (including via spring.jpa.properties.hibernate.physical-naming-strategy). This is too much information to write in there, sorry. And I still think trying to understand how things work by only looking there is broken.

@snicoll
Copy link
Member Author

snicoll commented Apr 15, 2016

For reference, this commit updates the documentation. I've also updated the release notes based on your feedback.

@ccanning
Copy link

Is there a way for a user to apply the SpringNamingStrategy in SpringBoot 1.3.x with latest Hibernate so as to have it already compatible when upgrading to 1.4.x?

Does adding

spring.jpa.hibernate.naming.strategy=org.springframework.boot.orm.jpa.hibernate.SpringNamingStrategy

work for 1.3.x?

@snicoll
Copy link
Member Author

snicoll commented May 25, 2016

No it doesn't, that file is compiling against the Hibernate 5 API. The whole point of that effort is that migrating from Hibernate 4 (1.3) to Hibernate 5 (1.4) is painless if you're using our naming strategy. Have you actually noticed that's not compatible?

@ccanning
Copy link

We want to use hibernate 5 with spring boot 1.3.x and want to have the same naming as hibernate 4.3 as 1.4.x isn't available and don't want to run into any issues when upgrading to 1.4.x.

@snicoll
Copy link
Member Author

snicoll commented May 25, 2016

We don't officially support H5 in 1.3.x so I am afraid I have nothing to suggest. You could copy the naming strategies but you'll need to setup that manually in 1.3.x.

@ccanning
Copy link

Thanks. That's sufficient :)

@btiernay
Copy link

btiernay commented Aug 7, 2016

On a related note, you may be interested in adding something about this to the docs:

http://stackoverflow.com/questions/34595084/embedded-field-does-not-work-after-upgrading-to-hibernate-5-0-6-final-in-spring/38815345#38815345

Using DefaultComponentSafeNamingStrategy was very common in older versions of Hibernate to avoid having to use excessive AttributeOverrides

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

No branches or pull requests