My personal feeling is that if the change itself is not going to make it into 5.3 then the SPI addition should not either. There are definitely 2 sides to this though.
On the one hand, if you add this SPI method it will *without question* have to change for 6.0. In a way it is creating an incompatibility 5.3->6.0 that is (1) not there currently and (2) unless/until we actually include the fix that uses it, not needed. This will for sure change in 6 because the entire process of generating SQL is different - those values would be pointless[1]. On the other hand, any fix to this in 5.3 would need (it sounds like) this SPI change. So if we don't add it now, we won't be able to fix this in 5.3 at all. However, I think it is worth pointing out that the entire run-time model (here Entityersister, via Queryable) is drastically changing in a completely non-deprecatable way - so this particular change is "a drop in the bucket" of 6.0 changes in this area. So in this particular case, I don't think the 5.3->6.0 compat breakage is that big of a deal. All that said, I would say go for it. Just make as sure as possible that the signature is good for your needs. [1] Yes we could deprecate the method in 6.0 and just have it do nothing (because it would not have enough info to do anything useful in the new model). But personally I very much dislike that - it really just cheats the definition of deprecation. On Fri, Mar 2, 2018 at 12:12 AM Gail Badner <gbad...@redhat.com> wrote: > This is not necessarily needed for 5.3.0, but I would like to get it into > the 5.3 branch. > > So far, there is one change to an SPI, Queryable#lazyGroupSelectFragment is > added: > > public String lazyGroupSelectFragment( > String tableAlias, > String suffix, > String fetchGroupName); > > The method gets implemented in AbstractEntityPersister. > > I don't think it's possible to create a default method in Queryable because > the method needs access to private/protect methods in > AbstractEntityPersister. > > If I don't get HHH-11223 in for 5.3.0, should I at least push the SPI > change for 5.3.0? > > On Thu, Mar 1, 2018 at 9:55 PM, Gail Badner <gbad...@redhat.com> wrote: > > > Hi, > > > > Currently, when a lazy group is loaded that has associations, there are 2 > > SQL statements executed. > > > > For example, suppose the DEFAULT lazy group contains the following: > > > > * Order#customer is a lazy many-to-one association; > > > > * Order#text is a lazy LOB. > > > > > > When one of these properties is referenced, Hibernate loads both of them. > > > > > > Currently, it takes 2 SQL statements to get the association loaded. > > > > > > 1) Loads #text and the FK value for #customer (select o.text, > > o.customer_id from orders o where o.id = ?) > > > > 2) Loads the state for #customer (select ... from customers c where c.id > = > > ?) > > > > > > In other words, FetchMode.SELECT is used to load associations in a lazy > > group. > > > > > > HHH-11223 involves combining the 2 SQL statements into a single statement > > using a join (i.e., using FetchMode.JOIN), but only when at least one of > > the following is true: > > > > > > * caching is disabled for the SessionFactory (e.g., > > hibernate.cache.use_second_level_cache=false; > > > > * caching is not enabled for "get" operations (i.e., > > session.getCacheMode().isGetEnabled() == false); > > > > * the associated entity type (and subclasses?) is not cached > > > > HHH-11223 only mentions to-one associations, but I think collections in a > > lazy group could also be loaded using a join as well. > > > > I think I'm pretty close, but I need to do a lot more testing to ensure > it > > covers things like: > > > > * lazy association in composite ID > > * lazy association in an embeddable > > * lazy association in a secondary table > > * lazy association in a subclass > > * too many bags > > > > This is what I have implemented so far: https://github.com/ > > hibernate/hibernate-orm/pull/2170 > > > > At the moment, I'm not looking for a comprehensive review. I'm just > > looking for a review of the overall design. > > > > I don't want to get into the nitty-gritty here, but, basically, I've > > created a new type of AbstractLoadPlanBasedEntityLoader, LazyGroupLoader, > > which loads the lazy group properties into the existing entity. > > > > If caching is disabled for the SessionFactory, then one loader is > > constructed per lazy group, and associations will be loaded using > > FetchMode.JOIN. > > > > If caching is enabled for the SessionFactory, then 2 loaders are > > constructed per lazy group. One loader is used when caching is enabled > for > > "get" operations (using Fetch.SELECT for cached associated > > entities/collections; Fetch.JOIN for other associations). The other > loader > > is used when caching is disabled for "get" operations. > > > > Here are some of the changes to the default behavior (described above) > > that could be useful: > > > > 1) Change all associations in lazy groups to be loaded using > > FetchMode.SELECT (as is currently done without the fix), or using > > FetchMode.JOIN (regardless of caching). A new configuration property > could > > be added: > > > > hibernate.bytecode.lazyGroup_association_fetchMode=(default | select | > > join ) > > > > 2) Allow the FetchMode to be configured for a particular lazy > association. > > According to the documentation [1], @Fetch can't be used for this > because, > > "... FetchMode.JOIN acts as a FetchType.EAGER strategy. Even if we mark > the > > association as FetchType.LAZY, the FetchMode.JOIN will load the > association > > eagerly." > > > > A new annotation could be introduced: @LazyFetch. > > > > Comments or suggestions? > > > > Thanks, > > Gail > > > > [1] http://docs.jboss.org/hibernate/orm/5.2/userguide/html_ > > single/Hibernate_User_Guide.html#fetching-fetchmode-join > > > _______________________________________________ > 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
