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

Have multiLoad not return (unflushed) DELETED entities

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 5.2.1
    • Fix Version/s: 5.2.2, 5.1.10
    • Component/s: hibernate-core
    • Labels:
      None
    • Environment:
      5.2.1

      Description

      The crux of the initial report was to say that locally deleted entities (an entity that has been passed to Session#delete, but not yet flushed) should not be returned.

      It turns out that really this user wants the behavior of Session#get, while multiLoad was initially designed to operate like Session#load. I can see 3 distinct behaviors that we may want to expose:

      1. the plural form of Session#load/IdentifierLoadAccess#getReference - maybe #getReferences?
      2. the plural form of Session#get/IdentifierLoadAccess#load - maybe #loadEntities?
      3. a hybrid form where:

      So we will offer both semantics via distinct methods:

      • MultiIdentifierLoadAccess#multiLoad - will work

      Initially multipleLoad behaved more like Session#load/IdentifierLoadAccess#getReference, meaning:

      1. proxies could be created for return
      2. entities that are in DELETED (deleted, but not yet flushed) would be returned.

      The request here specifically is to make sure that the second point does not happen.


      Original report


      When using session.byMulipleIds(SimpleEntity.class).multipleLoad(ids); I got a few problems.

      1. multiLoad 's sessionCheckingEnabled option default value should be true.

      If one of ids which managed entity by session should not be included in multiLoad query's 'in' condition.

      SimpleEntity third = session.byId( SimpleEntity.class ).load( 3 );
      List<SimpleEntity> list = session.byMultipleIds(SimpleEntity.class)
                                          .multiLoad( Arrays.asList(1 , 2 , 3 ));
      assertEquals( 3, list.size() );
      

      id "3" is already loaded in session that i expect query include only another ids "2", "3" ; like session.byId(SimpleEntity.class).load(1);
      But default value of sessionCheckingEnable is false that query include all ids even "1".
      It is more efficient i think.

      If some of one wants to execute query include all ids like REFRESH it would be better.
      session.byMultipleIds(SimpleEntity.class).enableSessionCheck(false).multiLoad( Arrays.asList(1 , 2 , 3 );

      I fixed it in this Pull-Request commit
      https://github.com/hibernate/hibernate-orm/pull/1489/commits/2bed426a82c6db4551e96b5fe5a23e3284ca6bfd

      2. Fix in multiLoad bugs with non flushed entities both side of sessionCheckingEnable true and false

      When sessionCheckingEnabled is true, DynamicBatchingEntityLoaderBuilder will check if some of ids are managed entities from session then they will exclude for query.
      But, below test will be failed.

      @Test
      public void testBasicMultiLoadWithManagedButRemovedAndChecking() {
      	Session session = openSession();
      	session.getTransaction().begin();
      	SimpleEntity third = session.byId( SimpleEntity.class ).load( 3 );
      	session.remove(third);
      	List<SimpleEntity> list = session.byMultipleIds(SimpleEntity.class).enableSessionCheck(true)
                             .multiLoad( ids(56) );
      	// removed entity excluded in results
      	assertEquals( 55, list.size() );
      	session.getTransaction().commit();
      	session.close();
      }
      

      removed but non flushed entity will be founded by
      DynamicBatchingEntityLoaderBuilder that it will be excluded in query but included in results.
      I think it is a kind of bug. removed and non flushed entity should be excluded query and results.
      So DynamicBatchingEntityLoaderBuilder need to check if managedEntity marked DELETED or GONE.

      When sessionCheckingEnabled is false, DynamicBatchingEntityLoaderBuilder should flush non flushed entity queries for consistence query results like executing HQL. (depends on FlushMode)
      Below test will be failed.

      @Test
      public void testBasicMultiLoadWithNonFlushedAndNoChecking() {
      	Session session = openSession();
      	session.getTransaction().begin();
      	session.save( new SimpleEntity( 100, "Entity #" + 100 ) );
      	Integer[] ids = { 1, 2, 3, 100 };
      	List<SimpleEntity> list = session.byMultipleIds(SimpleEntity.class).enableSessionCheck(false)
                            .multiLoad( ids );
      	// should include saved(non-flushed) entities in results depends on FlushMode
      	assertEquals( 4, list.size() );
      	session.getTransaction().commit();
      	session.close();
      }
      

      ids "1", "2", "3" are persisted but non managed entities, and id "100" saved before executing mulgiLoad(). id "100" of entity is managed in session but non flushed.
      When executing multiLoad() then results size is only 3 exclude "100". It is weird result with AutoFlush mode. If i use HQL query session will flush before executing query then result will correct.
      As a result, I think if sessionCheckingEnabled is false session flushed before executing multiLoad().

      I fixed it in Pull-Request commit
      https://github.com/hibernate/hibernate-orm/pull/1489/commits/3d671af35fc65e0182b498a3520f6eb8aa125beb

      3. Enhance multiLoad test case

      When sessionCheckingEnabled is true that managed entities put in first position of results.
      But current both of test case tested by first entity and assert with first position of results.
      Given entity for right test will be another position of entity

      I fixed it in Pull-Request commit
      https://github.com/hibernate/hibernate-orm/pull/1489/commits/2745fc6017cd03d31377ef8e315087031c5ef23d

      It would be nice, if either these things apply in hibernate-orm
      Thanks.

        Attachments

          Issue links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: