Re: PrefetchHelper bug

From: Andrus Adamchik (andru..bjectstyle.org)
Date: Tue Aug 31 2004 - 09:33:02 EDT

  • Next message: Andrus Adamchik: "Re: bug or code mistage?"

    I just opened a bug report, so that we don't forget about this issue:

    http://objectstyle.org/jira/secure/ViewIssue.jspa?key=CAY-188

    Thanks
    Andrus

    On Aug 31, 2004, at 8:21 AM, Twan Kogels wrote:
    > Hello Arndt,
    >
    > Thanks for the reply. When i readed the comments above the
    > "resolveToManyRelations" function i already thought that this class
    > wasn't tested fully in all cases. But i'm still happy you did commit
    > it, i'm only using resolveToOneRelations but it's a real life/time
    > saver for me.
    >
    > Cheers,
    > Twan
    >
    > At 10:56 31-8-2004, you wrote:
    >> Hi Twan,
    >>
    >> sorry, my fault, PrefetchHelper
    >> is a class I once contributed,
    >> but cause I'm not a committer,
    >> I used a local copy for corrections
    >> (I think I found the same bug)
    >> and forgot to distribute the fixes.
    >>
    >> I attach my working copy, maybe someone
    >> can care about committing to CVS?
    >>
    >> regards,
    >>
    >> Arndt
    >>
    >> PS: there are still some issues with this code,
    >> e.g. filtering duplicates from the result-lists
    >> (to increase performance)
    >>
    >> I am using it heavily to speed-up batch processing,
    >> so if more people are using it it could be worth
    >> investing some more engineering?
    >>
    >> PS2: we had a very nasty problem with ORACLE in some
    >> cases refusing to use indices with this kind of
    >> "where xy in (?,?,?...)" prefeteches, which we
    >> finally solved by changing some optimizer parameters
    >> in the ORACLE configuration.
    >> Ask me for details if you encouter this type of problem
    >>
    >> Twan Kogels wrote:
    >>
    >>> Hello,
    >>> I think i have found a bug in PrefetchHelper.java, i discovered it
    >>> when testing my application which uses resolveToOneRelations()
    >>> defined in PrefetchHelper.
    >>> In "PrefetchHelper.java" there is a function called
    >>> resolveToOneRelations(). Here's a the interesting code:
    >>> ===============
    >>> List oids = new ArrayList(nobjects);
    >>> for (int i = 0; i < nobjects; i++) {
    >>> DataObject sourceObject = (DataObject) objects.get(i);
    >>> DataObject targetObject = (DataObject)
    >>> sourceObject.readProperty(relName);
    >>> if (targetObject.getPersistenceState() ==
    >>> PersistenceState.HOLLOW) {
    >>> ObjectId oid = targetObject.getObjectId();
    >>> oids.add(oid);
    >>> }
    >>> }
    >>> // this maybe suboptimal, cause it uses an OR .. OR .. OR ..
    >>> expression
    >>> // instead of IN (..) - to be compatble with compound keys -
    >>> // however, it seems to be quite fast as well
    >>> SelectQuery sel = QueryUtils.selectQueryForIds(oids);
    >>> context.performQuery(sel);
    >>> ===============
    >>> It is possible that "List oids" will be a empty list when it reaches
    >>> the line:
    >>> ===============
    >>> SelectQuery sel = QueryUtils.selectQueryForIds(oids);
    >>> ===============
    >>> because when a object has already been retrieved once the following
    >>> line will return false and no object is added to oids list.
    >>> ===============
    >>> if (targetObject.getPersistenceState() == PersistenceState.HOLLOW) {
    >>> ===============
    >>> "QueryUtils.selectQueryForIds(oids);" will raise a
    >>> IllegalArgumentException when the oids list is empty.
    >>> ===============
    >>> IllegalArgumentException("List must contain at least one ObjectId")
    >>> ===============
    >>> I now just catch this "IllegalArgumentException" in my code and
    >>> ignore it. But a check in PrefetchHelper would be nice, something
    >>> like:
    >>> ===============
    >>> if(oids.size()!=0){
    >>> SelectQuery sel = QueryUtils.selectQueryForIds(oids);
    >>> context.performQuery(sel);
    >>> }
    >>> ===============
    >>> Cheers,
    >>> Twan Kogels
    >>
    >>
    >>
    >> /*
    >> * $Log: PrefetchHelper.java,v $
    >> * Revision 1.8 2003/09/15 08:45:11 bl
    >> * checkstyle fixes
    >> *
    >> * Revision 1.7 2003/09/12 14:28:32 sb2
    >> * (ab)
    >> * added a filter logic to prevent
    >> * re-feteching of already resolved
    >> * toMany relations
    >> *
    >> * (still 2 TODOs:
    >> * - detect multiple occurences of the same object in the given list
    >> * - look at the return value of resolveToOne: does
    >> * it already return the targets already resolved?)
    >> *
    >> */
    >>
    >> package my.local.working_package;
    >>
    >> import org.objectstyle.cayenne.CayenneRuntimeException;
    >> import org.objectstyle.cayenne.DataObject;
    >> import org.objectstyle.cayenne.ObjectId;
    >> import org.objectstyle.cayenne.access.DataContext;
    >> import org.objectstyle.cayenne.access.ToManyList;
    >> import org.objectstyle.cayenne.access.util.QueryUtils;
    >> import org.objectstyle.cayenne.exp.Expression;
    >> import org.objectstyle.cayenne.exp.ExpressionFactory;
    >> import org.objectstyle.cayenne.map.DbAttributePair;
    >> import org.objectstyle.cayenne.map.DbRelationship;
    >> import org.objectstyle.cayenne.map.ObjEntity;
    >> import org.objectstyle.cayenne.map.ObjRelationship;
    >> import org.objectstyle.cayenne.query.SelectQuery;
    >>
    >> import java.util.ArrayList;
    >> import java.util.HashMap;
    >> import java.util.List;
    >> import java.util.Map;
    >>
    >>
    >> /**
    >> *..uthor Arndt Brenschede
    >> */
    >> public final class PrefetchHelper
    >> {
    >> private PrefetchHelper()
    >> {
    >> }
    >>
    >> /**
    >> * Resolves a toOne relationship for a list of objects.
    >> * (performance tuning only)
    >> */
    >> public static List resolveToOneRelations( DataContext context, List
    >> objects,
    >> String relName )
    >> {
    >> int nobjects = objects.size();
    >>
    >> if ( nobjects == 0 )
    >> {
    >> return new ArrayList();
    >> }
    >>
    >> List oids = new ArrayList( nobjects );
    >>
    >> for ( int i = 0; i < nobjects; i++ )
    >> {
    >> DataObject sourceObject = (DataObject)objects.get( i );
    >> DataObject targetObject =
    >> (DataObject)sourceObject.readPropertyDirectly( relName );
    >>
    >> if ( targetObject != null )
    >> {
    >> ObjectId oid = targetObject.getObjectId();
    >> oids.add( oid );
    >> }
    >> }
    >>
    >> // this maybe suboptimal, cause it uses an OR .. OR .. OR ..
    >> expression
    >> // instead of IN (..) - to be compatble with compound keys -
    >> // however, it seems to be quite fast as well
    >> List results;
    >>
    >> if ( oids.size() > 0 )
    >> {
    >> SelectQuery sel = QueryUtils.selectQueryForIds( oids );
    >> results = context.performQuery( sel );
    >> }
    >> else
    >> {
    >> results = new ArrayList();
    >> }
    >>
    >> return results;
    >> }
    >>
    >> /**
    >> * Resolves a toMany relation for a list of objects.
    >> * (performance tuning only)
    >> *
    >> * <p>WARNING: this is a bit of a hack - it works for my
    >> * toMany's, but it possibly doesn't work in all cases.</p>
    >> *
    >> *
    >> * <p>*** It definitly does not work for compound keys ***</p>
    >> */
    >> public static List resolveToManyRelations( DataContext context,
    >> List objectList,
    >> String relName )
    >> {
    >> List objectsResolved = new ArrayList();
    >>
    >> // filter the list of objects to exclude those
    >> // with the relation already resolved
    >> List objects = new ArrayList();
    >> for ( int i = 0; i < objectList.size(); i++ )
    >> {
    >> DataObject object = (DataObject)objectList.get( i );
    >> ToManyList tml = (ToManyList)object.readPropertyDirectly(
    >> relName );
    >>
    >> if ( tml.needsFetch() )
    >> {
    >> objects.add( object );
    >> }
    >> else
    >> {
    >> objectsResolved.addAll( tml );
    >> }
    >> }
    >>
    >> int nobjects = objects.size();
    >>
    >> if ( nobjects == 0 )
    >> {
    >> return objectsResolved;
    >> }
    >>
    >> // get the relationship meta object
    >> ObjEntity ent = context.getEntityResolver().lookupObjEntity(
    >> (DataObject)objects.get( 0 ) );
    >> ObjRelationship rel = (ObjRelationship)ent.getRelationship(
    >> relName );
    >>
    >> // sanity check
    >> if ( rel == null )
    >> {
    >> throw new CayenneRuntimeException( "ObjEntity '" + ent.getName()
    >> + "' has no relationship of name '" + relName + "'" );
    >> }
    >>
    >> ObjEntity destEnt = (ObjEntity)rel.getTargetEntity();
    >>
    >> List dbRels = rel.getDbRelationships();
    >>
    >> // sanity check
    >> if ( ( dbRels == null ) || ( dbRels.size() != 1 ) )
    >> {
    >> throw new CayenneRuntimeException( "ObjRelationship '" +
    >> rel.getName()
    >> + "' has no or >1 DbRels" );
    >> }
    >>
    >> DbRelationship dbRel = (DbRelationship)dbRels.get( 0 );
    >>
    >> List joins = dbRel.getJoins();
    >>
    >> if ( joins.size() != 1 )
    >> {
    >> throw new CayenneRuntimeException( "DbRelationship '" +
    >> dbRel.getName()
    >> + "' has no or >1 joins" );
    >> }
    >>
    >> DbAttributePair join = (DbAttributePair)joins.get( 0 );
    >> String dbKey = join.getSource().getName();
    >> String foreignKey = join.getTarget().getName();
    >>
    >> // put the object-ids in a map for later assignment of the
    >> // query results
    >> Map listMap = new HashMap( nobjects );
    >>
    >> for ( int i = 0; i < nobjects; i++ )
    >> {
    >> DataObject object = (DataObject)objects.get( i );
    >> ObjectId oid = object.getObjectId();
    >> listMap.put( oid.getValueForAttribute( dbKey ), new ArrayList()
    >> );
    >> }
    >>
    >> // get the reverse db-rel
    >> DbRelationship reverse = dbRel.getReverseRelationship();
    >>
    >> if ( reverse == null )
    >> {
    >> throw new CayenneRuntimeException( "DbRelatitionship '" +
    >> dbRel.getName()
    >> + "' has no reverse relationship" );
    >> }
    >>
    >> // do the query
    >> SelectQuery sel = new SelectQuery( destEnt,
    >> ExpressionFactory.binaryDbPathExp( Expression.IN,
    >> reverse.getName(),
    >> ExpressionFactory.unaryExp( Expression.LIST, objects ) ) );
    >> sel.setFetchingDataRows( true );
    >>
    >> List results = context.performQuery( sel );
    >>
    >> // sort the resulting objects into individual lists for each
    >> source object
    >> int nrows = results.size();
    >>
    >> for ( int k = 0; k < nrows; k++ )
    >> {
    >> Map row = (Map)results.get( k );
    >> Object obj = context.objectFromDataRow( destEnt, row, false );
    >> objectsResolved.add( obj );
    >> ( (List)listMap.get( row.get( foreignKey ) ) ).add( obj );
    >> }
    >>
    >> // and finally set these lists in the relation targets
    >> for ( int i = 0; i < nobjects; i++ )
    >> {
    >> DataObject object = (DataObject)objects.get( i );
    >> ObjectId oid = object.getObjectId();
    >> List list = (List)listMap.get( oid.getValueForAttribute( dbKey
    >> ) );
    >>
    >> ( (ToManyList)object.readPropertyDirectly( relName )
    >> ).setObjectList( list );
    >> }
    >>
    >> return objectsResolved;
    >> }
    >> }
    >>
    >
    >
    >



    This archive was generated by hypermail 2.0.0 : Tue Aug 31 2004 - 09:33:08 EDT