We're updating the issue view to help you get more done. 

Improve consistency of how we call implicitNamingStrategy.determineBasicColumnName with element collections

Description

This is a followup of and a proposal to fix .

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.

Environment

None

Status

Assignee

Guillaume Smet

Reporter

Guillaume Smet

Fix versions

Labels

None

backPortable

None

Suitable for new contributors

None

Requires Release Note

None

Pull Request

None

backportDecision

None

Components

Affects versions

5.2.0
5.1.0
5.0.9

Priority

Major