This is a followup of https://hibernate.atlassian.net/browse/ HHH-6005 and a proposal to fix https://hibernate.atlassian.net/browse/ OGM-893 .
In OGM, our implicit naming strategy rely on the fact that {{ImplicitNamingStrategy#determineBasicColumnName}} is called with a propertyName containing {{collection&&element.}} for collection elements.
This was the case before ORM 5, but following HHH-6005, the behavior of {{Ejb3Column}} has changed in this regard and {{ImplicitNamingStrategy#determineBasicColumnName}} is sometimes called with a propertyName cleaned up from {{collection&&element.}} and sometimes with the full propertyName containing {{collection&&element.}}.
Let's admit, we have the following propertyName {{items.collection&&element.name}}.
in https://github.com/gsmet/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/cfg/Ejb3Column.java#L272
-> {{determineBasicColumnName}} is called with {{items.name}}
while in https://github.com/gsmet/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/cfg/Ejb3Column.java#L375
-> {{determineBasicColumnName}} is called with the full propertyName {{items.collection&&element.name}}.
While we should not expose {{collection&&element.}} to the physical naming strategy as we don't want it to be included in the column name, I think we should be consistent regarding the parameter we pass to {{ImplicitNamingStrategy#determineBasicColumnName}} and restore the old behavior in this regard.
It doesn't break the expectations of people having implemented their own implicit naming strategy as currently {{determineBasicColumnName}} is already sometimes called with {{collection&&element.}} in the property name.
What I propose is to change the code in {{Ejb3Column}} to always call {{ImplicitNamingStrategy#determineBasicColumnName}} with "collection&&element." included in the propertyName and only remove it before passing it to the physical strategy.
This would allow the calls to {{ImplicitNamingStrategy#determineBasicColumnName}} to be consistent and would fix the aforementioned OGM bug which has been silently introduced when we migrated to ORM 5 (and yes, we will add a unit test in OGM).
It would be nice if it could be backported also to 5.1 as OGM will move to 5.1 soon and we would like to fix this bug sooner rather than later. |
|