Re: PrefetchHelper bug

From: Arndt Brenschede (a..iamos.de)
Date: Tue Aug 31 2004 - 04:56:22 EDT

  • Next message: Sako!: "show sql"

    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 - 04:57:52 EDT