HHH-14270 GroupedSchemaValidatorImpl / NameSpaceTablesInformation does not find different-case table information in case insensitive environment #10429
Conversation
There was a problem hiding this comment.
Hello @mehmetali2003, thank you for the contribution, I left a comment on a possibly better approach. I would also ask you to include a test within your PR that asserts that the behavior explained in the linked Jira issue doesn't occur and protects against regressions in the future.
Hello @mbellade, thank you for your feedback. I looked through the issue. Can you tell some details about a better approach, please? |
There was a problem hiding this comment.
@mehmetali2003 sorry I though I left an additional comment in the code, you can find it now. Also please try creating a test as well, thanks.
...ate-core/src/main/java/org/hibernate/tool/schema/extract/spi/NameSpaceTablesInformation.java
Outdated
Show resolved
Hide resolved
...ate-core/src/main/java/org/hibernate/tool/schema/extract/spi/NameSpaceTablesInformation.java
Outdated
Show resolved
Hide resolved
| final Table table = new Table( "orm", tableName.getTableName().getText() ); | ||
| nameSpaceTablesInformation.getTableInformation( table ); | ||
| boolean tableMatched = tableInformation.getName().getTableName().getText().equals( tableName.getTableName().getText() ); | ||
| Assert.assertTrue("Table matched: ", tableMatched); |
There was a problem hiding this comment.
This test is very "synthetic", and instantiating Hibernate internal classes is not portable as they might change in future versions. A good test reproduces a "real-world" scenario, the one where you experienced this particular bug, ensuring that no regressions are introduced. Could you try to isolate that in a simple reproducer? Thanks.
There was a problem hiding this comment.
Hello @mbellade,
I tried to create instances of Hibernate internal class which were not portable. But, tableInformation is null for table PERSON_TABLE. I have supposed that is existing in database. Could I send codes for your review to understand the problem?
Thanks.
There was a problem hiding this comment.
Hey @mehmetali2003, I'm not really sure what you mean by your message and what I should look at in this code screenshot. Feel free to send any code for our review and we'll try to help.
There was a problem hiding this comment.
Hello @mbellade,
Thanks for your feedback. I have send a new commit. Can you check, please?
There was a problem hiding this comment.
The test still looks very complex, and not really how we verify regressions in the ORM code base.
You should start from your real-world application where you observed the bug, and try to isolate a simple reproducer which shows the problem within ORM itself. Please try to do that, instead of invoking a bunch of Hibernate internal constructs.
|
Thanks for your pull request! This pull request appears to follow the contribution rules. › This message was automatically generated. |
…s not find different-case table information in case insensitive environment
|
Rebased, applied suggestion and squashed. @mehmetali2003 could you please pull, then address @mbellade's suggestions regarding complex tests? |
I'm going to address it @yrodiere , thanks. However, I have asked how could I address it at zulip. And, there is no any comment. I'm not sure how to address it. |
In a case-insensitive database, the names in the entity mapping and the database schema should only be compared after converting them to the same case, either upper- or lowercase. For that,
getTableInformationneeds to be rewritten to find the name with either upper or lowercase in the map of tables.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-14270