Author: epbernard Date: 2008-06-28 06:51:13 -0400 (Sat, 28 Jun 2008) New Revision: 14818 Added: search/trunk/src/java/org/hibernate/search/engine/MultiClassesQueryLoader.java search/trunk/src/java/org/hibernate/search/engine/ObjectLoaderHelper.java search/trunk/src/test/org/hibernate/search/test/query/MultiClassesQueryLoaderTest.java Removed: search/trunk/src/test/org/hibernate/search/test/query/ObjectLoaderTest.java Modified: search/trunk/src/java/org/hibernate/search/engine/DocumentBuilder.java search/trunk/src/java/org/hibernate/search/engine/ObjectLoader.java search/trunk/src/java/org/hibernate/search/engine/ProjectionLoader.java search/trunk/src/java/org/hibernate/search/engine/QueryLoader.java search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java Log: HSEARCH-223 use multiple queries rathen than ObjectLoader when possible (can't do that if id is composite) Modified: search/trunk/src/java/org/hibernate/search/engine/DocumentBuilder.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/DocumentBuilder.java 2008-06-27 00:25:45 UTC (rev 14817) +++ search/trunk/src/java/org/hibernate/search/engine/DocumentBuilder.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -47,6 +47,7 @@ import org.hibernate.search.bridge.BridgeFactory; import org.hibernate.search.bridge.FieldBridge; import org.hibernate.search.bridge.TwoWayFieldBridge; +import org.hibernate.search.bridge.TwoWayString2FieldBridgeAdaptor; import org.hibernate.search.store.DirectoryProvider; import org.hibernate.search.store.IndexShardingStrategy; import org.hibernate.search.util.BinderHelper; @@ -81,7 +82,13 @@ private int maxLevel = Integer.MAX_VALUE; private final ScopedAnalyzer analyzer = new ScopedAnalyzer(); private Similarity similarity; + private boolean isRoot; + //if composite id, use of (a, b) in ((1,2), (3,4)) fails on most database + private boolean safeFromTupleId; + public boolean isRoot() { + return isRoot; + } public DocumentBuilder(XClass clazz, InitContext context, DirectoryProvider[] directoryProviders, IndexShardingStrategy shardingStrategy, ReflectionManager reflectionManager) { @@ -103,6 +110,9 @@ if ( idKeywordName == null ) { throw new SearchException( "No document id in: " + clazz.getName() ); } + //if composite id, use of (a, b) in ((1,2)TwoWayString2FieldBridgeAdaptor, (3,4)) fails on most database + //a TwoWayString2FieldBridgeAdaptor is never a composite id + safeFromTupleId = TwoWayString2FieldBridgeAdaptor.class.isAssignableFrom( idBridge.getClass() ); } private Analyzer getAnalyzer(XAnnotatedElement annotatedElement, InitContext context) { @@ -788,7 +798,16 @@ for (Class currentClass : indexedClasses) { if ( plainClass.isAssignableFrom( currentClass ) ) tempMappedSubclasses.add( currentClass ); } - mappedSubclasses = Collections.unmodifiableSet( tempMappedSubclasses ); + this.mappedSubclasses = Collections.unmodifiableSet( tempMappedSubclasses ); + Class superClass = plainClass.getSuperclass(); + this.isRoot = true; + while ( superClass != null) { + if ( indexedClasses.contains( superClass ) ) { + this.isRoot = false; + break; + } + superClass = superClass.getSuperclass(); + } } @@ -796,6 +815,14 @@ return mappedSubclasses; } + /** + * Make sure to return false if there is a risk of composite id + * if composite id, use of (a, b) in ((1,2), (3,4)) fails on most database + */ + public boolean isSafeFromTupleId() { + return safeFromTupleId; + } + private static class PropertiesMetadata { public Float boost; public Analyzer analyzer; Added: search/trunk/src/java/org/hibernate/search/engine/MultiClassesQueryLoader.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/MultiClassesQueryLoader.java (rev 0) +++ search/trunk/src/java/org/hibernate/search/engine/MultiClassesQueryLoader.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -0,0 +1,112 @@ +package org.hibernate.search.engine; + +import java.util.List; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import java.util.HashMap; +import java.util.Arrays; + +import org.hibernate.Session; +import org.hibernate.Criteria; +import org.hibernate.annotations.common.AssertionFailure; + +/** + * @author Emmanuel Bernard + */ +public class MultiClassesQueryLoader implements Loader { + private Session session; + private SearchFactoryImplementor searchFactoryImplementor; + private List<RootEntityMetadata> entityMatadata; + //useful if loading with a query is unsafe + private ObjectLoader objectLoader; + + public void init(Session session, SearchFactoryImplementor searchFactoryImplementor) { + this.session = session; + this.searchFactoryImplementor = searchFactoryImplementor; + this.objectLoader = new ObjectLoader(); + this.objectLoader.init( session, searchFactoryImplementor ); + } + + public void setEntityTypes(Class[] entityTypes) { + List<Class> safeEntityTypes; + //TODO should we go find the root entity for a given class rather than just checking for it's root status? + // root entity could lead to quite inefficient queries in Hibnernate when using table per class + if ( entityTypes.length == 0 ) { + //support all classes + safeEntityTypes = new ArrayList<Class>(); + for( Map.Entry<Class, DocumentBuilder<Object>> entry : searchFactoryImplementor.getDocumentBuilders().entrySet() ) { + //get only root entities to limit queries + if ( entry.getValue().isRoot() ) { + safeEntityTypes.add( entry.getKey() ); + } + } + } + else { + safeEntityTypes = Arrays.asList(entityTypes); + } + entityMatadata = new ArrayList<RootEntityMetadata>( safeEntityTypes.size() ); + for (Class clazz : safeEntityTypes) { + entityMatadata.add( new RootEntityMetadata( clazz, searchFactoryImplementor, session ) ); + } + } + + public Object load(EntityInfo entityInfo) { + return ObjectLoaderHelper.load( entityInfo, session ); + } + + public List load(EntityInfo... entityInfos) { + if ( entityInfos.length == 0 ) return Collections.EMPTY_LIST; + //split EntityInfo per root entity + Map<RootEntityMetadata, List<EntityInfo>> entityinfoBuckets = + new HashMap<RootEntityMetadata, List<EntityInfo>>( entityMatadata.size()); + for (EntityInfo entityInfo : entityInfos) { + boolean found = false; + for (RootEntityMetadata rootEntityInfo : entityMatadata) { + if ( rootEntityInfo.mappedSubclasses.contains( entityInfo.clazz ) ) { + List<EntityInfo> bucket = entityinfoBuckets.get( rootEntityInfo ); + if ( bucket == null ) { + bucket = new ArrayList<EntityInfo>(); + entityinfoBuckets.put( rootEntityInfo, bucket ); + } + bucket.add( entityInfo ); + found = true; + break; //we stop looping for the right bucket + } + } + if (!found) throw new AssertionFailure( "Could not find root entity for " + entityInfo.clazz ); + } + + //initialize objects by bucket + for ( Map.Entry<RootEntityMetadata, List<EntityInfo>> entry : entityinfoBuckets.entrySet() ) { + final RootEntityMetadata key = entry.getKey(); + final List<EntityInfo> value = entry.getValue(); + final EntityInfo[] bucketEntityInfos = value.toArray( new EntityInfo[value.size()] ); + if ( key.useObjectLoader ) { + objectLoader.load( bucketEntityInfos ); + } + else { + ObjectLoaderHelper.initializeObjects( bucketEntityInfos, + key.criteria, key.rootEntity, searchFactoryImplementor); + } + } + return ObjectLoaderHelper.returnAlreadyLoadedObjectsInCorrectOrder( entityInfos, session ); + } + + private static class RootEntityMetadata { + public final Class rootEntity; + public final Set<Class> mappedSubclasses; + private final Criteria criteria; + public final boolean useObjectLoader; + + RootEntityMetadata(Class rootEntity, SearchFactoryImplementor searchFactoryImplementor, Session session) { + this.rootEntity = rootEntity; + DocumentBuilder provider = searchFactoryImplementor.getDocumentBuilders().get( rootEntity ); + if ( provider == null) throw new AssertionFailure("Provider not found for class: " + rootEntity); + this.mappedSubclasses = provider.getMappedSubclasses(); + this.criteria = session.createCriteria( rootEntity ); + this.useObjectLoader = !provider.isSafeFromTupleId(); + } + } +} Modified: search/trunk/src/java/org/hibernate/search/engine/ObjectLoader.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/ObjectLoader.java 2008-06-27 00:25:45 UTC (rev 14817) +++ search/trunk/src/java/org/hibernate/search/engine/ObjectLoader.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -21,22 +21,7 @@ } public Object load(EntityInfo entityInfo) { - //be sure to get an initialized object but save from ONFE and ENFE - Object maybeProxy = session.load( entityInfo.clazz, entityInfo.id ); - try { - Hibernate.initialize( maybeProxy ); - } - catch (RuntimeException e) { - if ( LoaderHelper.isObjectNotFoundException( e ) ) { - log.debug( "Object found in Search index but not in database: {} with id {}", - entityInfo.clazz, entityInfo.id ); - maybeProxy = null; - } - else { - throw e; - } - } - return maybeProxy; + return ObjectLoaderHelper.load( entityInfo, session ); } public List load(EntityInfo... entityInfos) { Added: search/trunk/src/java/org/hibernate/search/engine/ObjectLoaderHelper.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/ObjectLoaderHelper.java (rev 0) +++ search/trunk/src/java/org/hibernate/search/engine/ObjectLoaderHelper.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -0,0 +1,90 @@ +package org.hibernate.search.engine; + +import java.util.Collections; +import java.util.List; +import java.util.ArrayList; +import java.io.Serializable; + +import org.hibernate.Hibernate; +import org.hibernate.Session; +import org.hibernate.Criteria; +import org.hibernate.type.EntityType; +import org.hibernate.criterion.Disjunction; +import org.hibernate.criterion.Restrictions; +import org.hibernate.annotations.common.AssertionFailure; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * @author Emmanuel Bernard + */ +public class ObjectLoaderHelper { + + private static final int MAX_IN_CLAUSE = 500; + private static final Logger log = LoggerFactory.getLogger( ObjectLoader.class ); + + public static Object load(EntityInfo entityInfo, Session session) { + //be sure to get an initialized object but save from ONFE and ENFE + Object maybeProxy = session.load( entityInfo.clazz, entityInfo.id ); + try { + Hibernate.initialize( maybeProxy ); + } + catch (RuntimeException e) { + if ( LoaderHelper.isObjectNotFoundException( e ) ) { + log.debug( "Object found in Search index but not in database: {} with id {}", + entityInfo.clazz, entityInfo.id ); + maybeProxy = null; + } + else { + throw e; + } + } + return maybeProxy; + } + + public static void initializeObjects(EntityInfo[] entityInfos, Criteria criteria, Class entityType, + SearchFactoryImplementor searchFactoryImplementor) { + final int maxResults = entityInfos.length; + if ( maxResults == 0 ) return; + + DocumentBuilder builder = searchFactoryImplementor.getDocumentBuilders().get( entityType ); + String idName = builder.getIdentifierName(); + int loop = maxResults / MAX_IN_CLAUSE; + boolean exact = maxResults % MAX_IN_CLAUSE == 0; + if ( !exact ) loop++; + Disjunction disjunction = Restrictions.disjunction(); + for (int index = 0; index < loop; index++) { + int max = index * MAX_IN_CLAUSE + MAX_IN_CLAUSE <= maxResults ? + index * MAX_IN_CLAUSE + MAX_IN_CLAUSE : + maxResults; + List<Serializable> ids = new ArrayList<Serializable>( max - index * MAX_IN_CLAUSE ); + for (int entityInfoIndex = index * MAX_IN_CLAUSE; entityInfoIndex < max; entityInfoIndex++) { + ids.add( entityInfos[entityInfoIndex].id ); + } + disjunction.add( Restrictions.in( idName, ids ) ); + } + criteria.add( disjunction ); + criteria.list(); //load all objects + } + + + public static List returnAlreadyLoadedObjectsInCorrectOrder(EntityInfo[] entityInfos, Session session) { + //mandatory to keep the same ordering + List result = new ArrayList( entityInfos.length ); + for (EntityInfo entityInfo : entityInfos) { + Object element = session.load( entityInfo.clazz, entityInfo.id ); + if ( Hibernate.isInitialized( element ) ) { + //all existing elements should have been loaded by the query, + //the other ones are missing ones + result.add( element ); + } + else { + if ( log.isDebugEnabled() ) { + log.debug( "Object found in Search index but not in database: {} with {}", + entityInfo.clazz, entityInfo.id ); + } + } + } + return result; + } +} Modified: search/trunk/src/java/org/hibernate/search/engine/ProjectionLoader.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/ProjectionLoader.java 2008-06-27 00:25:45 UTC (rev 14817) +++ search/trunk/src/java/org/hibernate/search/engine/ProjectionLoader.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -10,6 +10,7 @@ /** * @author Emmanuel Bernard */ +//TODO change the underlying ObjectLoader to a MutliClassesQueryLoader public class ProjectionLoader implements Loader { private SearchFactoryImplementor searchFactoryImplementor; private Session session; Modified: search/trunk/src/java/org/hibernate/search/engine/QueryLoader.java =================================================================== --- search/trunk/src/java/org/hibernate/search/engine/QueryLoader.java 2008-06-27 00:25:45 UTC (rev 14817) +++ search/trunk/src/java/org/hibernate/search/engine/QueryLoader.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -2,15 +2,13 @@ package org.hibernate.search.engine; import java.util.ArrayList; -import java.util.Collections; import java.util.List; +import java.util.Collections; import org.hibernate.Criteria; import org.hibernate.Hibernate; import org.hibernate.Session; import org.hibernate.annotations.common.AssertionFailure; -import org.hibernate.criterion.Disjunction; -import org.hibernate.criterion.Restrictions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,7 +16,6 @@ * @author Emmanuel Bernard */ public class QueryLoader implements Loader { - private static final int MAX_IN_CLAUSE = 500; private final Logger log = LoggerFactory.getLogger( QueryLoader.class ); private Session session; @@ -36,66 +33,16 @@ } public Object load(EntityInfo entityInfo) { - //be sure to get an initialized object - Object maybeProxy = session.get( entityInfo.clazz, entityInfo.id ); - try { - Hibernate.initialize( maybeProxy ); - } - catch (RuntimeException e) { - if ( LoaderHelper.isObjectNotFoundException( e ) ) { - log.debug( "Object found in Search index but not in database: {} with id {}", - entityInfo.clazz, entityInfo.id ); - maybeProxy = null; - } - else { - throw e; - } - } - return maybeProxy; + return ObjectLoaderHelper.load( entityInfo, session ); } public List load(EntityInfo... entityInfos) { - final int maxResults = entityInfos.length; - if ( maxResults == 0 ) return Collections.EMPTY_LIST; + if ( entityInfos.length == 0 ) return Collections.EMPTY_LIST; if ( entityType == null ) throw new AssertionFailure( "EntityType not defined" ); if ( criteria == null ) criteria = session.createCriteria( entityType ); - DocumentBuilder builder = searchFactoryImplementor.getDocumentBuilders().get( entityType ); - String idName = builder.getIdentifierName(); - int loop = maxResults / MAX_IN_CLAUSE; - boolean exact = maxResults % MAX_IN_CLAUSE == 0; - if ( !exact ) loop++; - Disjunction disjunction = Restrictions.disjunction(); - for (int index = 0; index < loop; index++) { - int max = index * MAX_IN_CLAUSE + MAX_IN_CLAUSE <= maxResults ? - index * MAX_IN_CLAUSE + MAX_IN_CLAUSE : - maxResults; - List ids = new ArrayList( max - index * MAX_IN_CLAUSE ); - for (int entityInfoIndex = index * MAX_IN_CLAUSE; entityInfoIndex < max; entityInfoIndex++) { - ids.add( entityInfos[entityInfoIndex].id ); - } - disjunction.add( Restrictions.in( idName, ids ) ); - } - criteria.add( disjunction ); - criteria.list(); //load all objects - - //mandatory to keep the same ordering - List result = new ArrayList( entityInfos.length ); - for (EntityInfo entityInfo : entityInfos) { - Object element = session.load( entityInfo.clazz, entityInfo.id ); - if ( Hibernate.isInitialized( element ) ) { - //all existing elements should have been loaded by the query, - //the other ones are missing ones - result.add( element ); - } - else { - if ( log.isDebugEnabled() ) { - log.debug( "Object found in Search index but not in database: {} with {}", - entityInfo.clazz, entityInfo.id ); - } - } - } - return result; + ObjectLoaderHelper.initializeObjects( entityInfos, criteria, entityType, searchFactoryImplementor ); + return ObjectLoaderHelper.returnAlreadyLoadedObjectsInCorrectOrder( entityInfos, session ); } public void setCriteria(Criteria criteria) { Modified: search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java =================================================================== --- search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java 2008-06-27 00:25:45 UTC (rev 14817) +++ search/trunk/src/java/org/hibernate/search/query/FullTextQueryImpl.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -43,10 +43,10 @@ import org.hibernate.search.engine.EntityInfo; import org.hibernate.search.engine.FilterDef; import org.hibernate.search.engine.Loader; -import org.hibernate.search.engine.ObjectLoader; import org.hibernate.search.engine.ProjectionLoader; import org.hibernate.search.engine.QueryLoader; import org.hibernate.search.engine.SearchFactoryImplementor; +import org.hibernate.search.engine.MultiClassesQueryLoader; import org.hibernate.search.filter.ChainedFilter; import org.hibernate.search.filter.FilterKey; import org.hibernate.search.reader.ReaderProvider; @@ -185,15 +185,16 @@ return loader; } else if ( classes.length == 1 ) { - QueryLoader loader = new QueryLoader(); + final QueryLoader loader = new QueryLoader(); loader.init( session, searchFactoryImplementor ); loader.setEntityType( classes[0] ); return loader; } else { - final ObjectLoader objectLoader = new ObjectLoader(); - objectLoader.init( session, searchFactoryImplementor ); - return objectLoader; + final MultiClassesQueryLoader loader = new MultiClassesQueryLoader(); + loader.init( session, searchFactoryImplementor ); + loader.setEntityTypes( classes ); + return loader; } } Copied: search/trunk/src/test/org/hibernate/search/test/query/MultiClassesQueryLoaderTest.java (from rev 14770, search/trunk/src/test/org/hibernate/search/test/query/ObjectLoaderTest.java) =================================================================== --- search/trunk/src/test/org/hibernate/search/test/query/MultiClassesQueryLoaderTest.java (rev 0) +++ search/trunk/src/test/org/hibernate/search/test/query/MultiClassesQueryLoaderTest.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -0,0 +1,56 @@ +//$ +package org.hibernate.search.test.query; + +import java.sql.Statement; +import java.util.List; + +import org.hibernate.search.test.SearchTestCase; +import org.hibernate.search.FullTextSession; +import org.hibernate.search.Search; +import org.hibernate.search.FullTextQuery; +import org.hibernate.Session; +import org.hibernate.Transaction; +import org.apache.lucene.queryParser.QueryParser; +import org.apache.lucene.analysis.KeywordAnalyzer; +import org.apache.lucene.search.Query; + +/** + * @author Emmanuel Bernard + */ +public class MultiClassesQueryLoaderTest extends SearchTestCase { + + public void testObjectNotFound() throws Exception { + Session sess = openSession(); + Transaction tx = sess.beginTransaction(); + Author author = new Author(); + author.setName( "Moo Cow" ); + sess.persist( author ); + + tx.commit(); + sess.clear(); + Statement statement = sess.connection().createStatement(); + statement.executeUpdate( "DELETE FROM Author" ); + statement.close(); + FullTextSession s = Search.createFullTextSession( sess ); + tx = s.beginTransaction(); + QueryParser parser = new QueryParser( "title", new KeywordAnalyzer() ); + Query query = parser.parse( "name:moo" ); + FullTextQuery hibQuery = s.createFullTextQuery( query, Author.class, Music.class ); + List result = hibQuery.list(); + assertEquals( "Should have returned no author", 0, result.size() ); + + for (Object o : s.createCriteria( Object.class ).list()) { + s.delete( o ); + } + + tx.commit(); + s.close(); + } + + protected Class[] getMappings() { + return new Class[] { + Author.class, + Music.class + }; + } +} Deleted: search/trunk/src/test/org/hibernate/search/test/query/ObjectLoaderTest.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/query/ObjectLoaderTest.java 2008-06-27 00:25:45 UTC (rev 14817) +++ search/trunk/src/test/org/hibernate/search/test/query/ObjectLoaderTest.java 2008-06-28 10:51:13 UTC (rev 14818) @@ -1,56 +0,0 @@ -//$ -package org.hibernate.search.test.query; - -import java.sql.Statement; -import java.util.List; - -import org.hibernate.search.test.SearchTestCase; -import org.hibernate.search.FullTextSession; -import org.hibernate.search.Search; -import org.hibernate.search.FullTextQuery; -import org.hibernate.Session; -import org.hibernate.Transaction; -import org.apache.lucene.queryParser.QueryParser; -import org.apache.lucene.analysis.KeywordAnalyzer; -import org.apache.lucene.search.Query; - -/** - * @author Emmanuel Bernard - */ -public class ObjectLoaderTest extends SearchTestCase { - - public void testObjectNotFound() throws Exception { - Session sess = openSession(); - Transaction tx = sess.beginTransaction(); - Author author = new Author(); - author.setName( "Moo Cow" ); - sess.persist( author ); - - tx.commit(); - sess.clear(); - Statement statement = sess.connection().createStatement(); - statement.executeUpdate( "DELETE FROM Author" ); - statement.close(); - FullTextSession s = Search.createFullTextSession( sess ); - tx = s.beginTransaction(); - QueryParser parser = new QueryParser( "title", new KeywordAnalyzer() ); - Query query = parser.parse( "name:moo" ); - FullTextQuery hibQuery = s.createFullTextQuery( query, Author.class, Music.class ); - List result = hibQuery.list(); - assertEquals( "Should have returned no author", 0, result.size() ); - - for (Object o : s.createCriteria( Object.class ).list()) { - s.delete( o ); - } - - tx.commit(); - s.close(); - } - - protected Class[] getMappings() { - return new Class[] { - Author.class, - Music.class - }; - } -}