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

HHH-13917: Add support for HANA Cloud #3323

Merged
merged 1 commit into from Apr 17, 2020
Merged

Conversation

breglerj
Copy link
Contributor

  • Add new HANA Cloud dialect
  • Adapt Databases enum
  • Adapt failing tests

@breglerj breglerj force-pushed the hanacloud branch 2 times, most recently from 1c2b9be to 9d2398f Compare April 6, 2020 19:37
Copy link
Member

@Sanne Sanne left a comment

Choose a reason for hiding this comment

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

Hi @breglerj , looks good in principle.

One question: while I haven't tested it, it looks like this might expect a schema change for existing users. Is that the case? And if so, would you want to do so in a minor version or which version of Hibernate do you think this should appear?

@breglerj
Copy link
Contributor Author

@Sanne: there is no automatic upgrade from HANA to HANA Cloud, because of some feature incompatibilities. Users will either have to start fresh with HANA Cloud or they will have to migrate their existing persistence manually before switching to HANA Cloud. In that sense the added dialect is more like a new database type instead of a new database version. I'd be fine with including it in a minor version so users can get started quickly.

@Sanne
Copy link
Member

Sanne commented Apr 17, 2020

ok, great!

@Sanne Sanne merged commit 95c9526 into hibernate:master Apr 17, 2020
@Sanne
Copy link
Member

Sanne commented Apr 17, 2020

Hi @breglerj,
I've merged the PR. Could you check if there's any need to update documentation?

Regarding versions: this was merged in master, which implies it will be included in 5.5.0.Beta1 as next release. This version will be JPA 3.0, so I expect some resistance from people: many will stay on older versions for a while as they won't want to upgrade right away.
We can of course backport this to 5.4 and or 5.3 - up to you.

But most importantly, we will need this ported to the 6 branch as well. The Dialect(s) changed quite a bit, so we will need your help for that as cherry-picking this commit on the 6 branch won't be trivial. Could you coordinate with @dreab8 to make that happen?

Thanks again!

@breglerj
Copy link
Contributor Author

Hi @Sanne,

Thanks for merging the PR.

I think it would be worth backporting it to 5.4 to have an alternative for people not willing or able to use JPA 3.0. Do you want me to create another PR for the backport, or will you do it?

I'm happy to help with getting the dialect into the 6 branch. @dreab8: let me know what you need from me.

@Sanne
Copy link
Member

Sanne commented Apr 17, 2020

I think it would be worth backporting it to 5.4 to have an alternative for people not willing or able to use JPA 3.0. Do you want me to create another PR for the backport, or will you do it?

+1
No worries, it's trivial enough. I'll run the tests once and push it already.

@breglerj
Copy link
Contributor Author

@Sanne: thanks.

BTW, I've created a new PR (#3351) to update the documentation.

@Sanne
Copy link
Member

Sanne commented Apr 17, 2020

thanks for the doc changes. Merged the dialect in 5.4 too, I'll do the same with the new PR later.

@dreab8
Copy link
Member

dreab8 commented Apr 17, 2020

Thanks @breglerj, I have already ported the changes to 6 branch, it was not too complex :)

@breglerj
Copy link
Contributor Author

@dreab8: great, thanks.

@dreab8
Copy link
Member

dreab8 commented Apr 17, 2020

@breglerj thanks to you for the PR 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants