From dev-return-72024-apmail-geronimo-dev-archive=geronimo.apache.org@geronimo.apache.org Fri Feb 20 15:36:59 2009 Return-Path: Delivered-To: apmail-geronimo-dev-archive@www.apache.org Received: (qmail 32813 invoked from network); 20 Feb 2009 15:36:59 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 20 Feb 2009 15:36:59 -0000 Received: (qmail 86951 invoked by uid 500); 20 Feb 2009 15:36:58 -0000 Delivered-To: apmail-geronimo-dev-archive@geronimo.apache.org Received: (qmail 86552 invoked by uid 500); 20 Feb 2009 15:36:57 -0000 Mailing-List: contact dev-help@geronimo.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: dev@geronimo.apache.org List-Id: Delivered-To: mailing list dev@geronimo.apache.org Received: (qmail 86542 invoked by uid 99); 20 Feb 2009 15:36:57 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 20 Feb 2009 07:36:57 -0800 X-ASF-Spam-Status: No, hits=1.2 required=10.0 tests=SPF_NEUTRAL X-Spam-Check-By: apache.org Received-SPF: neutral (athena.apache.org: local policy) Received: from [76.13.13.45] (HELO smtp106.prem.mail.ac4.yahoo.com) (76.13.13.45) by apache.org (qpsmtpd/0.29) with SMTP; Fri, 20 Feb 2009 15:36:49 +0000 Received: (qmail 94891 invoked from network); 20 Feb 2009 15:36:27 -0000 Received: from unknown (HELO Macintosh.local) (dwoods@75.177.173.152 with plain) by smtp106.prem.mail.ac4.yahoo.com with SMTP; 20 Feb 2009 15:36:27 -0000 X-YMail-OSG: xwal4nYVM1kv0gdnYeVkeqJG.YtQtuFn9sqqmHIG8NL6vGrUUnAT79zYWlN_QofzQwfoA58p2FlI9ucOKTaJbBURhab8FvPrwB.5R2FIoO.ZFzB6THEE6_pewRwXpVY75Z.i2DflSVSjTvXH9vPrC.cinG.6UBIvXgBFOak_AaW7PTnSIzMfP.ck7sg- X-Yahoo-Newman-Property: ymail-3 Message-ID: <499ECDFA.2000803@apache.org> Date: Fri, 20 Feb 2009 10:36:26 -0500 From: Donald Woods 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> In-Reply-To: <20090218232726.8FF57238889F@eris.apache.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Checked: Checked by ClamAV on apache.org 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 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 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(); > - } > } > > >