Skip to content

HHH-14835 : More changes to support SchemaMigrator/SchemaValidator using Hibernate Reactive #4235

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 24, 2021
Merged

HHH-14835 : More changes to support SchemaMigrator/SchemaValidator using Hibernate Reactive #4235

merged 3 commits into from
Sep 24, 2021

Conversation

gbadner
Copy link
Contributor

@gbadner gbadner commented Sep 21, 2021 •
edited
Loading

@gbadner
HHH-14835 : More changes to support SchemaMigrator/SchemaValidator us…
db37250 
…ing Hibernate Reactive

Changes required for sequences to be created with a default catalog
@gbadner
Copy link
Contributor Author

gbadner commented Sep 21, 2021

Can this be changed to a draft?

@gbadner gbadner mentioned this pull request Sep 21, 2021
Closed
@beikov beikov marked this pull request as draft September 21, 2021 06:17
@DavideD
Copy link
Member

DavideD commented Sep 21, 2021

DON'T MERGE YET; MORE CHANGES MAY BE NEEDED FOR DB2.

Couldn't we apply the changes needed for Db2 as a separate pr?

@gbadner
Copy link
Contributor Author

gbadner commented Sep 21, 2021

Couldn't we apply the changes needed for Db2 as a separate pr?

@DavideD, yes that would be fine. DB2 should be very close. I just need to review it. I just thought it might be worth waiting a (hopefully) a day or two, then this issue could cover DB2 as well.

I also had a very small fix related to Sybase that Andrea mentioned got fixed in 6, that doesn't really warrant a separate issue.

I'm fine with creating a separate issue for DB2 though. Whatever works.

@gavinking
Copy link
Member

@gbadner exciting to hear

@DavideD
Copy link
Member

DavideD commented Sep 21, 2021

I'm fine with creating a separate issue for DB2 though. Whatever works.

We should check when the next ORM Final release is planned (@Sanne? or @dreab8?). If the MSSQL part is ready, it would be nice to have it working in the final release of Hibernate Reactive, even if the Db2 part is missing.

But if we are close to have both, a single PR sounds good.

@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Sep 21, 2021 •
edited
Loading

Thanks for your pull request!

This pull request appears to follow the contribution rules.

› This message was automatically generated.

@gbadner
Copy link
Contributor Author

gbadner commented Sep 22, 2021

Force-pushed with amended commit that fixes an issue @dreab8 found on Sybase that turned up after merging with wip/6.0 branch.

@gbadner gbadner requested a review from Sanne September 22, 2021 00:06
@gbadner
Copy link
Contributor Author

gbadner commented Sep 22, 2021

I started looking at DB2 work, but now I'm seeing failures due to the Vert.X bug with both INDIVIDUALLY and GROUPED strategies.

It looks like we're going to have to wait for the Vert.X bug to be fixed before being able to support SchemaMigrator on DB2.

Provided all the checks pass, I think it's fine to merge this PR.

@gbadner gbadner marked this pull request as ready for review September 22, 2021 00:52
@gbadner gbadner mentioned this pull request Sep 22, 2021
Merged
Sanne
Sanne reviewed Sep 23, 2021
View reviewed changes
...a/org/hibernate/test/schemaupdate/SchemaExportSqlServerWithSequenceDefaultSchemaCatalog.java Outdated
/**
* @author Andrea Boriero
*/
@TestForIssue(jiraKey = "HHH-XXX")
Copy link
Member

Choose a reason for hiding this comment

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

Could you fix the jiraKey - and I guess the author, or did Andrea help here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Sanne, there is no jira key. This was cherry-picked from aaba476 .

@Andrea and I agreed that it was not necessary to create a new jira issue for this.

I suppose I could fill in "HHH-14835" for the jira key though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry, my mistake. That was not from cherry-picking. I'll fix it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 823be54 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the test from a test that Andrea wrote, with minor changes.

@gbadner
HHH-14835 : More changes to support SchemaMigrator/SchemaValidator us…
823be54 
…ing Hibernate Reactive

Add jira key to test case
@gbadner gbadner requested a review from Sanne September 23, 2021 18:52
@Sanne Sanne merged commit c6e3bf2 into hibernate:main Sep 24, 2021
@Sanne
Copy link
Member

Sanne commented Sep 24, 2021

Thanks! merged it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sanne Sanne Sanne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gbadner @DavideD @gavinking @Sanne @dreab8