Uploaded image for project: 'Hibernate ORM'
  1. HHH-10863

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects versions: 5.1.0, 5.0.9, 5.2.0
    • Fix versions: 5.2.1, 5.1.1
    • Components: hibernate-core
    • Labels:
      None
    • Bug Testcase Reminder (view):

      Bug reports should generally be accompanied by a test case!

    • Last commented by a user?:
      true
    • Sprint:

      Description

      This is a followup of HHH-6005 Closed and a proposal to fix OGM-893 Closed .

      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 Closed , 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.

        Attachments

          Issue links

            Activity

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: