So can you guys please review my pull request to asses whether this is the best way to implement that change? Any comments are welcome.
On Fri, Nov 5, 2010 at 7:45 PM, Emmanuel Bernard <emman...@hibernate.org>wrote: > Sorry, I had a off-line week, catching up here. > > I think we all agree the expected behavior (according to the spec) is that > EntityType.getName() should return @Entity.name. > I'd say for the future major rev, we should make the change. > I also think that the metamodel use at this stage is small enough that such > change would only impact a few users. And there is a clear and easy API > change for people impacted. > > I personally would do the move as soon as possible but I can see / respect > the decision if people want to wait post 3.6.x (in practice that would most > likely be Hibernate 4). > > On 3 nov. 2010, at 16:55, Steve Ebersole wrote: > > > I completely empathize since the JPA "metamodel" defines no other means > to get > > at this information. I am however nervous especially nervous about > > backporting this fix as it is a change in behavior that is far from > clearly a > > bug. I am kind of new to annotations so it is hard for me to say how/if > users > > use this EntityType.getName() today. Emmanuel? Hardy? > > > > > > On Wednesday, November 03, 2010, at 10:34 am, Tomasz Blachowicz wrote: > >> It's up to you. I put my arguments in the ticket. I agree that jsr317 > spec > >> does not specify the correlation of the @Entity.name and > >> EntityType#getName(), but I do think it if reasonable to think that > entity > >> name returned by the meta model and the entity name specified in the > >> annotation is the same thing. Furthermore the two other implementations > of > >> jsp317, Apache OpenJPA and EclipseLink, went this very route. > >> > >> On Wed, Nov 3, 2010 at 2:24 PM, Steve Ebersole <st...@hibernate.org> > wrote: > >>> We were just discussing this on IRC. Yes I think adding a new > attribute > >>> to PersistentClass to capture the value of @Entity.name specifically > >>> (really we'd > >>> just populate it with EntityBinder.name) is best. > >>> > >>> So we know *how* to do it. But to be honest, I am still uncertain > >>> whether we > >>> *should* do it. > >>> > >>> HHH-2597 is completely irrelevant here. We are not discussing the > >>> Hibernate > >>> Criteria API here. > >>> > >>> HHH-4375 is a bad idea. As stated before, @Entity.name is not really > in > >>> any > >>> way associated with Hibernate's notion of an entity name. @Entity.name > >>> is what Hibernate calls a query import name. > >>> > >>> On Wednesday, November 03, 2010, at 08:54 am, Tomasz Blachowicz wrote: > >>>> I think I understand what you mean. "Node name" might not be indeed > the > >>>> best way to do that. > >>>> > >>>> I think implementation of the EntityBinder with regards to the > handling > >>> > >>> of > >>> > >>>> @Entity.name is correct. My understanding of the jsr-317 is that the > >>> > >>> entity > >>> > >>>> name used in queries should be @Entity.name if specified otherwise the > >>>> default value that is unqualified name of the entity class. This is > how > >>>> EntityBinder works now I presume > >>>> (EntityBinder#bindEjb3Annotation(@Entity)). The think is that this > >>>> value is set to the name field of EntityBinder and then used to set > >>>> PresistentClass#setNodeName(). > >>>> > >>>> I wonder how I could implement the meta model change to make sure the > >>>> EntityType#getName() returns @Entity.name or unqualified class name in > >>> > >>> case > >>> > >>>> the @Entity.name has not been specified. How about adding new property > >>>> to the PersistentClass that would hot the value as computed in > >>>> EntityBinder and the I could use the value of this property to > >>>> populate EntityTypeImpl from MetamodelImpl#buildEntityType? Can you > >>>> comment that approach? > >>>> > >>>> On Wed, Nov 3, 2010 at 1:22 PM, Steve Ebersole <st...@hibernate.org> > >>> > >>> wrote: > >>>>> Ah, I misread. I thought you meant the Hibernate notion of an > >>>>> "entity name". > >>>>> As far as I can tell now looking at it, there is no such annotation > >>> > >>> equiv > >>> > >>>>> for > >>>>> that. > >>>>> > >>>>> @javax.persistence.Entity.name is really more akin to Hibernate's > >>> > >>> notion > >>> > >>>>> of an > >>>>> import name for queries. > >>>>> > >>>>> I am not really understanding the benefit of returning the query > >>>>> import name > >>>>> for EntityType.getName(). Personally I think its just a case of > >>>>> badly matched > >>>>> attribute names and an inference being drawn there. > >>>>> > >>>>> But we do not really use that internally, so i have no qualms with > >>>>> changing it > >>>>> I guess. However, I do not want "node name" used. That is a totally > >>>>> different beast, and if annotations ever support alternate entity > >>>>> modes this > >>>>> will become a problem in this code you suggest; its currently working > >>>>> just by > >>>>> a quirk where the default node name just happens to be the > >>>>> unqualified entity > >>>>> name, and I am assuming the annotation binding code is setting it if > >>>>> @Entity.name is specified as well. To be honest, after looking at > >>>>> org.hibernate.cfg.annotations.EntityBinder#bindEntity my first > >>>>> thought was whether this code even handles @Entity.name properly in > >>>>> terms of making it an > >>>>> import name. It would work as is for the default case because that > >>>>> is Hibernate's internal default behavior (to register the > >>>>> unqualified > >>> > >>> entity > >>> > >>>>> name > >>>>> as an import name). > >>>>> > >>>>> On Wednesday, November 03, 2010, at 07:58 am, Steve Ebersole wrote: > >>>>>> If that is really the case, then that is a problem with the > >>> > >>> annotation > >>> > >>>>>> binding code. > >>>>>> > >>>>>> On Wednesday, November 03, 2010, at 06:50 am, Tomasz Blachowicz > >>> > >>> wrote: > >>>>>>> Hi Steve, > >>>>>>> > >>>>>>> PersistentClass#getEntityName is not good, because it is exactly > >>> > >>> the > >>> > >>>>> same > >>>>> > >>>>>>> value as PersistentClass#getClassName. > >>>>>>> PersistentClass#getClassName is populated in > >>>>>>> org.hibernate.cfg.annotations.EntityBinder#bindEntity with the > >>> > >>> value > >>> > >>>>> from > >>> > >>> org.hibernate.annotations.common.reflection.java.JavaXClass#getName. > >>> > >>>>>>> JavaXClass#getName is implemented as clazz.getName(), so it is > >>> > >>> always > >>> > >>>>>>> class name and the @Entity.name is simply ignored for the purpose > >>> > >>> of > >>> > >>>>> the > >>>>> > >>>>>>> entity name. And this is the reason I used > >>>>>>> PersistentClass#getNodeName that returns @Entity.name or > >>>>>>> "unqualified" class name in case @Entity.name is unspecified. > >>>>>>> > >>>>>>> If I'd stick with PersistentClass#getEntityName the issues is not > >>>>>>> resolved and EntityType.getName() returns stil FQN of the class > >>>>>>> instead of @Entity.name or "unqualified" class name. > >>>>>>> > >>>>>>> Regards, > >>>>>>> Tom > >>>>>>> > >>>>>>> On Wed, Nov 3, 2010 at 11:12 AM, Steve Ebersole < > >>> > >>> st...@hibernate.org> > >>> > >>>>> wrote: > >>>>>>>> Uncertain yet of how these pull request comments work on > >>>>>>>> GitHub, > >>> > >>> so > >>> > >>>>>>>> I thought > >>>>>>>> I'll make sure and respond here. The change should use > >>>>>>>> org.hibernate.mapping.PersistentClass#getEntityName instead of > >>>>>>>> org.hibernate.mapping.PersistentClass#getNodeName. > >>>>>>>> > >>>>>>>> See my other email to the dev list about porting to 3.6 > >>>>>>>> > >>>>>>>> On Wednesday, November 03, 2010, at 05:48 am, Tomasz Blachowicz > >>>>> > >>>>> wrote: > >>>>>>>>> I've submitted the pull request ( > >>>>>>>>> https://github.com/hibernate/hibernate-core/pull/8) for > >>> > >>> master. I > >>> > >>>>>>>>> should have it also ready for 3.6 branch as soon as I manage > >>>>>>>>> to > >>>>> > >>>>> merge > >>>>> > >>>>>>>>> the change between branches in this git thng that is pretty > >>>>>>>>> new to > >>>>> > >>>>> me > >>>>> > >>>>>>>>> ;) > >>>>>>>>> > >>>>>>>>> On Tue, Nov 2, 2010 at 1:18 PM, Emmanuel Bernard > >>>>>>>> > >>>>>>>> <emman...@hibernate.org>wrote: > >>>>>>>>>> Cool :) > >>>>>>>>>> If you use GitHub's infrastructure and use a pull-request, > >>>>>>>>>> we can get that integrated quite fast. > >>>>>>>>>> > >>>>>>>>>> On 2 nov. 2010, at 13:50, Tomasz Blachowicz wrote: > >>>>>>>>>> > >>>>>>>>>> I've already created the ticket ( > >>>>> > >>>>> http://opensource.atlassian.com/projects/hibernate/browse/HHH-5709) > >>>>> > >>>>>>>>>> . And I should be able to create the patch sometime this > >>>>>>>>>> evening. > >>>>>>>>>> > >>>>>>>>>> Cheers, > >>>>>>>>>> Tom > >>>>>>>>>> > >>>>>>>>>> On Tue, Nov 2, 2010 at 12:24 PM, Emmanuel Bernard > >>>>>>>> > >>>>>>>> <emman...@hibernate.org>wrote: > >>>>>>>>>>> I'd say you're correct and the name should match in the > >>>>> > >>>>> metamodel. > >>>>> > >>>>>>>>>>> Can you open a JIRA issue and even better try and work out > >>>>>>>>>>> a patch? > >>>>>>>>>>> > >>>>>>>>>>> PS: I've spilled hot chocolate on my laptop in the past: > >>>>>>>>>>> not > >>>>> > >>>>> good, > >>>>> > >>>>>>>>>>> the machine slowly but inevitably dies as corrosion wins > >>>>>>>>>>> it > >>>>> > >>>>> over. > >>>>> > >>>>>>>>>>> On 2 nov. 2010, at 12:35, Tomasz Blachowicz wrote: > >>>>>>>>>>>> Hi, > >>>>>>>>>>>> > >>>>>>>>>>>> I had been working recently on some stuff that deals > >>>>>>>>>>>> with JPA2 meta > >>>>>>>>>>> > >>>>>>>>>>> model. > >>>>>>>>>>> > >>>>>>>>>>>> Hibernate is my JPA2 provider/engine. Everything works > >>> > >>> fine > >>> > >>>>>>>>>>>> except > >>>>>>>> > >>>>>>>> one > >>>>>>>> > >>>>>>>>>>>> little thing that is name of the Entity. I'd expect that > >>>>>>>>>>>> value returned > >>>>>>>>>>> > >>>>>>>>>>> by > >>>>>>>>>>> > >>>>>>>>>>>> EntityType.getName() would be the same as @Entity.name > >>>>>>>>>>>> but not the > >>>>>>>> > >>>>>>>> FQN > >>>>>>>> > >>>>>>>>>>> of > >>>>>>>>>>> > >>>>>>>>>>>> the entity class. Although this is not explicitly stated > >>> > >>> in > >>> > >>>>> the > >>>>> > >>>>>>>>>>>> JPA2 specification it is reasonable to expect that > >>>>> > >>>>> @Entity.name > >>>>> > >>>>>>>>>>>> or the > >>>>>>>>>>> > >>>>>>>>>>> default > >>>>>>>>>>> > >>>>>>>>>>>> value (shortened class name) is the name of the entity > >>> > >>> used > >>> > >>>>>>>>>>>> in > >>>>>>>> > >>>>>>>> queries > >>>>>>>> > >>>>>>>>>>> ans > >>>>>>>>>>> > >>>>>>>>>>>> well as other places such as meta model. I know the > >>>>>>>>>>>> topic > >>> > >>> of > >>> > >>>>>>>>>>>> entity name > >>>>>>>>>>> > >>>>>>>>>>> was > >>>>>>>>>>> > >>>>>>>>>>>> discussed many times in the past (HHH-2597, HHH-4375, > >>>>> > >>>>> HHH-4465, > >>>>> > >>>>>>>>>>> HHH-5709), > >>>>>>>>>>> > >>>>>>>>>>>> and the rationale has been given for current > >>> > >>> implementation > >>> > >>>>>>>>>>>> of > >>>>>>>>>>> > >>>>>>>>>>> Hibernate, > >>>>>>>>>>> > >>>>>>>>>>>> however the topic hasn't been touched in the context of > >>> > >>> meta > >>> > >>>>>>>>>>>> model that > >>>>>>>>>>> > >>>>>>>>>>> is > >>>>>>>>>>> > >>>>>>>>>>>> new stuff added in JPA2. > >>>>>>>>>>>> > >>>>>>>>>>>> I just wanted to know what is your thinking with regards > >>> > >>> to > >>> > >>>>> the > >>>>> > >>>>>>>> matter > >>>>>>>> > >>>>>>>>>>> at > >>>>>>>>>>> > >>>>>>>>>>>> this stage and how likely is implementation of Hibernate > >>>>>>>>>>>> would > >>>>>>>> > >>>>>>>> change > >>>>>>>> > >>>>>>>>>>>> to match EntityType.getName with @Entity.name. > >>>>>>>>>>>> > >>>>>>>>>>>> P.S. > >>>>>>>>>>>> I just have spilled the entire cup of fresh coffee on my > >>>>> > >>>>> desktop > >>>>> > >>>>>>>> while > >>>>>>>> > >>>>>>>>>>>> writing this message. The mouse seems to be drowned :| > >>>>>>>>>>>> > >>>>>>>>>>>> Cheers, > >>>>>>>>>>>> Tom > >>>>>>>>>>>> _______________________________________________ > >>>>>>>>>>>> hibernate-dev mailing list > >>>>>>>>>>>> hibernate-dev@lists.jboss.org > >>>>>>>>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>>>>>>>> > >>>>>>>>> _______________________________________________ > >>>>>>>>> hibernate-dev mailing list > >>>>>>>>> hibernate-dev@lists.jboss.org > >>>>>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>>>>>>> > >>>>>>>> --- > >>>>>>>> Steve Ebersole <st...@hibernate.org> > >>>>>>>> http://hibernate.org > >>>>>>>> _______________________________________________ > >>>>>>>> hibernate-dev mailing list > >>>>>>>> hibernate-dev@lists.jboss.org > >>>>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>>>>> > >>>>>> --- > >>>>>> Steve Ebersole <st...@hibernate.org> > >>>>>> http://hibernate.org > >>>>> > >>>>> --- > >>>>> Steve Ebersole <st...@hibernate.org> > >>>>> http://hibernate.org > >>>>> _______________________________________________ > >>>>> hibernate-dev mailing list > >>>>> hibernate-dev@lists.jboss.org > >>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >>> > >>> --- > >>> Steve Ebersole <st...@hibernate.org> > >>> http://hibernate.org > >>> _______________________________________________ > >>> hibernate-dev mailing list > >>> hibernate-dev@lists.jboss.org > >>> https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > > --- > > Steve Ebersole <st...@hibernate.org> > > http://hibernate.org > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
