Mailing-List: contact dev-help@geronimo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geronimo.apache.org
Received-SPF: neutral (nike.apache.org: local policy)
Message-ID: <49A2D494.3080004@apache.org>
Date: Mon, 23 Feb 2009 11:53:40 -0500
From: Donald Woods <dwoods@apache.org>
User-Agent: Thunderbird 2.0.0.19 (Macintosh/20081209)
MIME-Version: 1.0
To: dev@openjpa.apache.org
CC: dev@geronimo.apache.org
Subject: Re: OpenJPA svn commit: r745691 - Reverting OPENJPA-838 and
 OPENJPA-917
 for additional testing.
References: <20090218232726.8FF57238889F@eris.apache.org>
 <499ECDFA.2000803@apache.org> <499ECF7F.5050602@apache.org>
In-Reply-To: <499ECF7F.5050602@apache.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Mike, any ETA on when OPENJPA-838 will be reintegrated for the Geronimo 
2.1.4 release?


-Donald


Donald Woods wrote:
> To be more specific, we need OPENJPA-838 to resolve an earlier reported 
> problem in the EJB TCK by David Blevins in OPENJPA-872.
> 
> -Donald
> 
> 
> Donald Woods wrote:
>> Mike, removing the below fixes is causing known EJB TCK failures in 
>> Geronimo.  We'll need these reapplied before we can cut a OpenJPA 
>> 1.2.1 release and a Geronimo 2.1.4 release....
>>
>>
>> -Donald
>>
>>
>>
>> mikedd@apache.org wrote:
>>> Author: mikedd
>>> Date: Wed Feb 18 23:27:25 2009
>>> New Revision: 745691
>>>
>>> URL: http://svn.apache.org/viewvc?rev=745691&view=rev
>>> Log:
>>> Reverting OPENJPA-838 and OPENJPA-917 for additional testing.
>>>
>>> Removed:
>>>     
>>> openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/Invoice.java 
>>>
>>>     
>>> openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/InvoiceKey.java 
>>>
>>>     
>>> openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/LineItem.java 
>>>
>>> Modified:
>>>     
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java 
>>>
>>>     
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java 
>>>
>>>     
>>> openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java 
>>>
>>>
>>> Modified: 
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java 
>>>
>>> URL: 
>>> http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java?rev=745691&r1=745690&r2=745691&view=diff 
>>>
>>> ============================================================================== 
>>>
>>> --- 
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java 
>>> (original)
>>> +++ 
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java 
>>> Wed Feb 18 23:27:25 2009
>>> @@ -26,8 +26,11 @@
>>>  import java.util.Map;
>>>  
>>>  import org.apache.openjpa.enhance.PersistenceCapable;
>>> +import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
>>>  import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
>>> +import org.apache.openjpa.jdbc.kernel.JDBCFetchConfigurationImpl;
>>>  import org.apache.openjpa.jdbc.kernel.JDBCStore;
>>> +import org.apache.openjpa.jdbc.kernel.JDBCStoreManager;
>>>  import org.apache.openjpa.jdbc.meta.ClassMapping;
>>>  import org.apache.openjpa.jdbc.meta.FieldMapping;
>>>  import org.apache.openjpa.jdbc.meta.FieldStrategy;
>>> @@ -35,11 +38,14 @@
>>>  import org.apache.openjpa.jdbc.schema.Column;
>>>  import org.apache.openjpa.jdbc.schema.ForeignKey;
>>>  import org.apache.openjpa.jdbc.sql.Joins;
>>> +import org.apache.openjpa.jdbc.sql.LogicalUnion;
>>>  import org.apache.openjpa.jdbc.sql.Result;
>>>  import org.apache.openjpa.jdbc.sql.Select;
>>>  import org.apache.openjpa.jdbc.sql.SelectExecutor;
>>> +import org.apache.openjpa.jdbc.sql.SelectImpl;
>>>  import org.apache.openjpa.jdbc.sql.Union;
>>>  import org.apache.openjpa.kernel.OpenJPAStateManager;
>>> +import org.apache.openjpa.lib.log.Log;
>>>  import org.apache.openjpa.lib.util.Localizer;
>>>  import org.apache.openjpa.meta.ClassMetaData;
>>>  import org.apache.openjpa.meta.JavaTypes;
>>> @@ -520,19 +526,86 @@
>>>              return;
>>>          }
>>>  
>>> +        //cache union for field here
>>>          // select data for this sm
>>> +        boolean found = true;
>>>          final ClassMapping[] elems = 
>>> getIndependentElementMappings(true);
>>>          final Joins[] resJoins = new Joins[Math.max(1, elems.length)];
>>> -        Union union = store.getSQLFactory().newUnion
>>> -            (Math.max(1, elems.length));
>>> -        union.select(new Union.Selector() {
>>> -            public void select(Select sel, int idx) {
>>> -                ClassMapping elem = (elems.length == 0) ? null : 
>>> elems[idx];
>>> -                resJoins[idx] = selectAll(sel, elem, sm, store, fetch,
>>> -                    JDBCFetchConfiguration.EAGER_PARALLEL);
>>> +        List parmList = null;
>>> +        Union union = null;
>>> +        SelectImpl sel = null;
>>> +        Map<JDBCStoreManager.SelectKey, Object[]> 
>>> storeCollectionUnionCache = null;
>>> +        JDBCStoreManager.SelectKey selKey = null;
>>> +        if (!((JDBCStoreManager)store).isQuerySQLCacheOn() || 
>>> elems.length > 1)
>>> +            union = newUnion(sm, store, fetch, elems, resJoins);
>>> +        else {
>>> +            parmList = new ArrayList();
>>> +            JDBCFetchConfiguration fetchClone = new 
>>> JDBCFetchConfigurationImpl();
>>> +            fetchClone.copy(fetch);
>>> +           +            // to specify the type so that no cast is 
>>> needed
>>> +            storeCollectionUnionCache = ((JDBCStoreManager)store).
>>> +                
>>> getCacheMapFromQuerySQLCache(StoreCollectionFieldStrategy.class);
>>> +            selKey = +                new 
>>> JDBCStoreManager.SelectKey(null, field, fetchClone);
>>> +            Object[] objs = storeCollectionUnionCache.get(selKey);
>>> +            if (objs != null) {
>>> +                union = (Union) objs[0];
>>> +                resJoins[0] = (Joins) objs[1];
>>>              }
>>> -        });
>>> -
>>> +            else {
>>> +                synchronized(storeCollectionUnionCache) {
>>> +                    objs = storeCollectionUnionCache.get(selKey);
>>> +                    if (objs == null) {
>>> +                        // select data for this sm
>>> +                        union = newUnion(sm, store, fetch, elems, 
>>> resJoins);
>>> +                        found = false;
>>> +                    } else {
>>> +                        union = (Union) objs[0];
>>> +                        resJoins[0] = (Joins) objs[1];
>>> +                    }
>>> +
>>> +                    sel = 
>>> ((LogicalUnion.UnionSelect)union.getSelects()[0]).
>>> +                        getDelegate();
>>> +                    if (sel.getSQL() == null) {
>>> +                        ((SelectImpl)sel).setSQL(store, fetch);
>>> +                        found = false;
>>> +                    }
>>> +
>>> +                    // only cache the union when elems length is 1 
>>> for now
>>> +                    if (!found) { +                        Object[] 
>>> objs1 = new Object[2];
>>> +                        objs1[0] = union;
>>> +                        objs1[1] = resJoins[0];
>>> +                        ((JDBCStoreManager)store).addToSqlCache(
>>> +                            storeCollectionUnionCache, selKey, objs1);
>>> +                     }
>>> +                }
>>> +            }
>>> +            +            Log log = store.getConfiguration().
>>> +                getLog(JDBCConfiguration.LOG_JDBC);
>>> +            if (log.isTraceEnabled()) {
>>> +                if (found)
>>> +                    log.trace(_loc.get("cache-hit", field, 
>>> this.getClass()));
>>> +                else
>>> +                    log.trace(_loc.get("cache-missed", field, 
>>> this.getClass())); +            }
>>> +            +            ClassMapping mapping = 
>>> field.getDefiningMapping();
>>> +            Object oid = sm.getObjectId();
>>> +            Column[] cols = mapping.getPrimaryKeyColumns();
>>> +            if (sel == null)
>>> +                sel = 
>>> ((LogicalUnion.UnionSelect)union.getSelects()[0]).
>>> +                getDelegate();
>>> +
>>> +            sel.wherePrimaryKey(mapping, cols, cols, oid, store, 
>>> +                    null, null, parmList);
>>> +            List nonFKParams = sel.getSQL().getNonFKParameters();
>>> +            if (nonFKParams != null && nonFKParams.size() > 0) 
>>> +                parmList.addAll(nonFKParams);
>>> +        }
>>> +                 // create proxy
>>>          Object coll;
>>>          ChangeTracker ct = null;
>>> @@ -545,7 +618,7 @@
>>>          }
>>>  
>>>          // load values
>>> -        Result res = union.execute(store, fetch);
>>> +        Result res = union.execute(store, fetch, parmList);
>>>          try {
>>>              int seq = -1;
>>>              while (res.next()) {
>>> @@ -569,6 +642,21 @@
>>>              sm.storeObject(field.getIndex(), coll);
>>>      }
>>>  
>>> +    protected Union newUnion(final OpenJPAStateManager sm, final 
>>> JDBCStore store,
>>> +        final JDBCFetchConfiguration fetch, final ClassMapping[] elems,
>>> +        final Joins[] resJoins) {
>>> +        Union union = store.getSQLFactory().newUnion
>>> +        (Math.max(1, elems.length));
>>> +        union.select(new Union.Selector() {
>>> +            public void select(Select sel, int idx) {
>>> +                ClassMapping elem = (elems.length == 0) ? null : 
>>> elems[idx];
>>> +                resJoins[idx] = selectAll(sel, elem, sm, store, fetch,
>>> +                        JDBCFetchConfiguration.EAGER_PARALLEL);
>>> +            }
>>> +        });
>>> +        return union;
>>> +    }
>>> +         /**
>>>       * Select data for loading, starting in field table.
>>>       */
>>>
>>> Modified: 
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java 
>>>
>>> URL: 
>>> http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java?rev=745691&r1=745690&r2=745691&view=diff 
>>>
>>> ============================================================================== 
>>>
>>> --- 
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java 
>>> (original)
>>> +++ 
>>> openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java 
>>> Wed Feb 18 23:27:25 2009
>>> @@ -56,6 +56,7 @@
>>>      private List _subsels = null;
>>>      private List _params = null;
>>>      private List _cols = null;
>>> +    private List _nonFKParams = null;
>>>  
>>>      /**
>>>       * Default constructor.
>>> @@ -146,6 +147,11 @@
>>>                          _cols.add(paramIndex, null);
>>>              }
>>>          }
>>> +        if (buf._nonFKParams != null) {
>>> +            if (_nonFKParams == null)
>>> +                _nonFKParams = new ArrayList();
>>> +            _nonFKParams.addAll(buf._nonFKParams);
>>> +        }
>>>      }
>>>  
>>>      public SQLBuffer append(Table table) {
>>> @@ -265,6 +271,11 @@
>>>                  if (isFK)
>>>                      break;
>>>              }
>>> +            if (!isFK) {
>>> +                if (_nonFKParams == null)
>>> +                    _nonFKParams = new ArrayList();
>>> +                _nonFKParams.add(o);                +            }
>>>          }
>>>          return this;
>>>      }
>>> @@ -388,6 +399,9 @@
>>>          return (_params == null) ? Collections.EMPTY_LIST : _params;
>>>      }
>>>  
>>> +    public List getNonFKParameters() {
>>> +        return (_nonFKParams == null) ? Collections.EMPTY_LIST : 
>>> _nonFKParams;
>>> +    }
>>>      /**
>>>       * Return the SQL for this buffer.
>>>       */
>>>
>>> Modified: 
>>> openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java 
>>>
>>> URL: 
>>> http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java?rev=745691&r1=745690&r2=745691&view=diff 
>>>
>>> ============================================================================== 
>>>
>>> --- 
>>> openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java 
>>> (original)
>>> +++ 
>>> openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java 
>>> Wed Feb 18 23:27:25 2009
>>> @@ -18,10 +18,10 @@
>>>   */
>>>  package org.apache.openjpa.persistence.jdbc.query.cache;
>>>  
>>> -import java.util.List;
>>> +import java.util.ArrayList;
>>> +import java.util.Collection;
>>>  
>>>  import javax.persistence.EntityManager;
>>> -import javax.persistence.EntityTransaction;
>>>  import javax.persistence.Query;
>>>  
>>>  import org.apache.openjpa.persistence.test.SQLListenerTestCase;
>>> @@ -45,21 +45,18 @@
>>>   *   * @author Pinaki Poddar
>>>   * @author Vikram Bhatia
>>> - * @author David Blevins
>>> + *   */
>>>  public class TestNonPrimaryKeyQueryParameters extends 
>>> SQLListenerTestCase {
>>>      private static final int FULLTIME_EMPLOYEE_COUNT = 3;
>>>      private static final int PARTTIME_EMPLOYEE_COUNT = 2;
>>> -    private static final int LINEITEM_PER_INVOICE = 1;
>>>      private static final String DEPT_NAME = "ENGINEERING";
>>>  
>>>      public void setUp() {
>>>          super.setUp(CLEAR_TABLES, Department.class, Employee.class,
>>>                  FullTimeEmployee.class, PartTimeEmployee.class,
>>> -                Invoice.class, LineItem.class,
>>>                  "openjpa.jdbc.QuerySQLCache", "true");
>>>          createDepartment(DEPT_NAME);
>>> -        createInvoice();
>>>          sql.clear();
>>>      }
>>>  
>>> @@ -106,10 +103,6 @@
>>>                  .size());
>>>  
>>>          assertSQL(".* AND t0.TYPE = .*");
>>> -       -        Invoice invoice = em.find(Invoice.class, new 
>>> InvoiceKey(1, "Red"));
>>> -        List<LineItem> list = invoice.getLineItems();
>>> -        assertEquals(LINEITEM_PER_INVOICE, list.size());
>>>          em.close();
>>>      }
>>>  
>>> @@ -161,20 +154,4 @@
>>>          em.close();
>>>  
>>>      }
>>> -   -    private void createInvoice() {
>>> -        EntityManager em = emf.createEntityManager();
>>> -        EntityTransaction tran = em.getTransaction();
>>> -        tran.begin();
>>> -        Invoice invoice = new Invoice(1, "Red", 1.30);
>>> -        for (int i = 1;  i <= LINEITEM_PER_INVOICE; i++) {
>>> -            LineItem item = new LineItem(String.valueOf(i), 10);
>>> -            item.setInvoice(invoice);
>>> -            invoice.getLineItems().add(item);
>>> -            em.persist(invoice);
>>> -        }
>>> -        em.flush();
>>> -        tran.commit();
>>> -        em.close();        -    }    }
>>>
>>>
>>>
>>
>