List:Cluster« Previous MessageNext Message »
From:Craig L Russell Date:August 20 2013 4:09am
Subject:Re: CRUD in MySql general log
View as plain text  
Hi Serdyn,

Thanks for your comments and fix. 

I thought you'd like to know I've fixed the problem in clusterj for 7.1, 7.2, and 7.3.

I've made a couple of changes to your suggested patch. This is from 7.1.

Regards,

Craig

------------------------------------------------------------
revno: 4846
committer: Craig L Russell <Craig.Russell@stripped>
branch nick: mysql-5.1-telco-7.1
timestamp: Sun 2013-08-18 11:29:26 -0700
message:
  Support clusterjpa use of cached JPQL queries
  
  This fixes a bug where the second identical query uses a cached query.
  The symptom is the use of em.createQuery with the same query string,
  which results in OpenJPA passing openjpa.prepared.SQL as the query language
  to NdbOpenJPAStoreManager.newQuery.
  
  Thanks to Serdyn du Toit for the report and suggested fix.
  
  The implementation of the fix is adapted from OpenJPA JDBCStoreManager.
diff:
=== modified file 'storage/ndb/clusterj/clusterj-jpatest/Makefile.am'
--- storage/ndb/clusterj/clusterj-jpatest/Makefile.am   2013-06-15 00:30:09 +0000
+++ storage/ndb/clusterj/clusterj-jpatest/Makefile.am   2013-08-18 18:29:26 +0000
@@ -75,6 +75,7 @@
   $(clusterj_jpatest_src)/com/mysql/clusterj/jpatest/model/TimestampAsUtilDateTypes.java
\
   $(clusterj_jpatest_src)/com/mysql/clusterj/jpatest/model/LazyEmployee.java \
   $(clusterj_jpatest_src)/com/mysql/clusterj/jpatest/PersistenceTestCase.java \
+  $(clusterj_jpatest_src)/com/mysql/clusterj/jpatest/QueryCacheTest.java \
   $(clusterj_jpatest_src)/com/mysql/clusterj/jpatest/SingleEMFTestCase.java \
   $(clusterj_jpatest_src)/com/mysql/clusterj/jpatest/SingleEMTestCase.java \
   $(clusterj_jpatest_src)/com/mysql/clusterj/jpatest/TimeAsSqlTimeTest.java \

=== modified file
'storage/ndb/clusterj/clusterj-jpatest/src/main/java/com/mysql/clusterj/jpatest/AbstractJPABaseTest.java'
---
storage/ndb/clusterj/clusterj-jpatest/src/main/java/com/mysql/clusterj/jpatest/AbstractJPABaseTest.java
    2011-12-29 14:39:37 +0000
+++
storage/ndb/clusterj/clusterj-jpatest/src/main/java/com/mysql/clusterj/jpatest/AbstractJPABaseTest.java
    2013-08-18 18:29:26 +0000
@@ -99,6 +99,19 @@
         failOnError();
         em.close();
     }
+
+    public void createEmployeeInstances() {
+        for (int i = 0; i < getNumberOfEmployees(); ++i) {
+            System.out.println("AbstractJPABaseTest creating Employee " + i);
+            Employee e = new Employee();
+            e.setId(i);
+            e.setAge(i);
+            e.setMagic(i);
+            e.setName("Employee " + i);
+            em.persist(e);
+        }
+    }
+
     public void createAll() {
         em = emf.createEntityManager();
         begin();

=== added file
'storage/ndb/clusterj/clusterj-jpatest/src/main/java/com/mysql/clusterj/jpatest/QueryCacheTest.java'
---
storage/ndb/clusterj/clusterj-jpatest/src/main/java/com/mysql/clusterj/jpatest/QueryCacheTest.java
 1970-01-01 00:00:00 +0000
+++
storage/ndb/clusterj/clusterj-jpatest/src/main/java/com/mysql/clusterj/jpatest/QueryCacheTest.java
 2013-08-18 18:29:26 +0000
@@ -0,0 +1,74 @@
+/*
+   Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA
+*/
+
+package com.mysql.clusterj.jpatest;
+
+import javax.persistence.Query;
+
+import com.mysql.clusterj.jpatest.model.IdBase;
+import com.mysql.clusterj.jpatest.model.Employee;
+
+
+/** Test query support.
+ * 
+ * Schema
+ * 
+drop table if exists t_basic;
+create table t_basic (
+  id int not null,
+  name varchar(32),
+  age int,
+  magic int not null,
+  primary key(id)) 
+  engine=ndbcluster;
+create unique index idx_unique_hash_magic using hash on t_basic(magic);
+create index idx_btree_age on t_basic(age);
+
+*/
+public class QueryCacheTest extends AbstractJPABaseTest {
+
+    private int NUMBER_OF_INSTANCES = 4;
+
+    @Override
+    protected int getNumberOfEmployees() {
+        return NUMBER_OF_INSTANCES;
+    }
+
+    /** Subclasses must override this method to provide the model class for the test */
+    protected Class<? extends IdBase> getModelClass() {
+        return Employee.class;
+    }
+
+    public void test() {
+        deleteAll();
+        createAll();
+        em = emf.createEntityManager();
+        em.getTransaction().begin();
+        for (int i = 0; i < NUMBER_OF_INSTANCES ; ++i) {
+            // deliberately use a different query instance each time to test query
caching
+            // the query string is cached with its associated prepared statement
+            Query query = em.createQuery("select e from Employee e where e.id = :id");
+            query.setParameter("id", i);
+            Employee e = (Employee)query.getSingleResult();
+            int id = e.getId();
+            errorIfNotEqual("Fail to verify id", i, id);
+        }
+        em.getTransaction().commit();
+        failOnError();
+    }
+
+}

=== modified file 'storage/ndb/clusterj/clusterj-openjpa/pom.xml'
--- storage/ndb/clusterj/clusterj-openjpa/pom.xml       2012-04-15 03:45:31 +0000
+++ storage/ndb/clusterj/clusterj-openjpa/pom.xml       2013-08-18 18:29:26 +0000
@@ -73,7 +73,7 @@
         <configuration>
           <instructions>
             <Export-Package>com.mysql.clusterj.openjpa.*</Export-Package>
-            <Import-Package>com.mysql.clusterj, com.mysql.clusterj.core,
com.mysql.clusterj.core.metadata, com.mysql.clusterj.core.query,
com.mysql.clusterj.core.spi, com.mysql.clusterj.core.store, com.mysql.clusterj.core.util,
com.mysql.clusterj.query, org.apache.openjpa.enhance, org.apache.openjpa.kernel,
org.apache.openjpa.kernel.exps, org.apache.openjpa.conf, org.apache.openjpa.jdbc.conf,
org.apache.openjpa.jdbc.kernel, org.apache.openjpa.jdbc.meta,
org.apache.openjpa.jdbc.schema, org.apache.openjpa.jdbc.sql, org.apache.openjpa.lib.conf,
org.apache.openjpa.meta, org.apache.openjpa.jdbc.meta.strats,
org.apache.openjpa.util</Import-Package>
+            <Import-Package>com.mysql.clusterj, com.mysql.clusterj.core,
com.mysql.clusterj.core.metadata, com.mysql.clusterj.core.query,
com.mysql.clusterj.core.spi, com.mysql.clusterj.core.store, com.mysql.clusterj.core.util,
com.mysql.clusterj.query, org.apache.openjpa.datacache, org.apache.openjpa.enhance,
org.apache.openjpa.kernel, org.apache.openjpa.kernel.exps, org.apache.openjpa.conf,
org.apache.openjpa.jdbc.conf, org.apache.openjpa.jdbc.kernel,
org.apache.openjpa.jdbc.meta, org.apache.openjpa.jdbc.schema,
org.apache.openjpa.jdbc.sql, org.apache.openjpa.lib.conf, org.apache.openjpa.meta,
org.apache.openjpa.jdbc.meta.strats, org.apache.openjpa.util</Import-Package>
           </instructions>
         </configuration>
       </plugin>

=== modified file
'storage/ndb/clusterj/clusterj-openjpa/src/main/java/com/mysql/clusterj/openjpa/NdbOpenJPAStoreManager.java'
---
storage/ndb/clusterj/clusterj-openjpa/src/main/java/com/mysql/clusterj/openjpa/NdbOpenJPAStoreManager.java
 2012-03-10 19:39:46 +0000
+++
storage/ndb/clusterj/clusterj-openjpa/src/main/java/com/mysql/clusterj/openjpa/NdbOpenJPAStoreManager.java
 2013-08-18 18:29:26 +0000
@@ -24,9 +24,13 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.openjpa.datacache.QueryCache;
+import org.apache.openjpa.datacache.QueryCacheStoreQuery;
 import org.apache.openjpa.jdbc.kernel.ConnectionInfo;
 import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
 import org.apache.openjpa.jdbc.kernel.JDBCStoreManager;
+import org.apache.openjpa.jdbc.kernel.PreparedSQLStoreQuery;
+import org.apache.openjpa.jdbc.kernel.SQLStoreQuery;
 import org.apache.openjpa.jdbc.meta.ClassMapping;
 import org.apache.openjpa.jdbc.meta.ValueMapping;
 import org.apache.openjpa.jdbc.schema.Table;
@@ -61,9 +65,6 @@
 import com.mysql.clusterj.core.util.LoggerFactoryService;
 import com.mysql.clusterj.query.QueryDomainType;
 
-/**
- *
- */
 public class NdbOpenJPAStoreManager extends JDBCStoreManager {
 
     /** My message translator */
@@ -431,12 +432,59 @@
         super.beforeStateChange(sm, fromState, toState);
     }
 
+    /**
+     * First method call for JPQL query string:
+     * - parameter language: javax.persistence.JPQL
+     * - returns: NdbOpenJPAStoreQuery
+     *
+     * Successive method calls for the same JPQL query string send in a different
language -
+     * - parameter language: openjpa.prepared.SQL
+     * - returns: QueryCacheStoreQuery if query cache is active or PreparedSQLStoreQuery
if not
+     * 
+     * Method calls for SQL query string send in a different language -
+     * - parameter language: openjpa.SQL
+     * - returns: QueryCacheStoreQuery if query cache is active or SQLStoreQuery if not
+     */
     @Override
     public StoreQuery newQuery(String language) {
-        ExpressionParser ep = QueryLanguages.parserForLanguage(language);
-        return new NdbOpenJPAStoreQuery(this, ep);
-    }
-
+        ExpressionParser expressionParser = QueryLanguages.parserForLanguage(language);
+        StoreQuery storeQuery = newStoreQuery(language, expressionParser); 
+        if (logger.isDetailEnabled()) {
+            logger.detail(("NdbOpenJPAStoreManager.newQuery language: " + language + "
parser: " + expressionParser));
+        }
+        if (storeQuery == null || expressionParser == null) {
+            // error -- no parser; or SQL or prepared SQL
+            return storeQuery;
+        }
+
+        QueryCache queryCache =
storeContext.getConfiguration().getDataCacheManagerInstance().getSystemQueryCache();
+        if (queryCache == null) {
+            // no query cache is active -- return the StoreQuery
+            return storeQuery;
+        }
+        
+        // query cache is active -- return the QueryCacheStoreQuery
+        return new QueryCacheStoreQuery(storeQuery, queryCache);
+    }
+
+    /** Create a StoreQuery based on the language.
+     * 
+     * @param language languages supported: javax.persistence.JPQL, openjpa.prepared.SQL,
openjpa.SQL
+     * @return StoreQuery for the language or null if the language is not supported
+     */
+    private StoreQuery newStoreQuery(String language, ExpressionParser expressionParser)
{
+        if (expressionParser != null) { 
+            return new NdbOpenJPAStoreQuery(this, expressionParser);
+        }
+        if (QueryLanguages.LANG_SQL.equals(language)) {
+            return new SQLStoreQuery(this);
+        }
+        if (QueryLanguages.LANG_PREPARED_SQL.equals(language)) {
+            return new PreparedSQLStoreQuery(this);
+        }
+        return null;
+    }
+    
     @Override
     public void beginOptimistic() {
         if (logger.isTraceEnabled()) {

=== added file
'storage/ndb/clusterj/clusterj-openjpa/src/test/java/com/mysql/clusterj/openjpatest/QueryCacheTest.java'
---
storage/ndb/clusterj/clusterj-openjpa/src/test/java/com/mysql/clusterj/openjpatest/QueryCacheTest.java
     1970-01-01 00:00:00 +0000
+++
storage/ndb/clusterj/clusterj-openjpa/src/test/java/com/mysql/clusterj/openjpatest/QueryCacheTest.java
     2013-08-18 18:29:26 +0000
@@ -0,0 +1,22 @@
+/*
+   Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA
+*/
+
+package com.mysql.clusterj.openjpatest;
+
+public class QueryCacheTest extends com.mysql.clusterj.jpatest.QueryCacheTest {
+
+}

On Jul 18, 2013, at 6:09 PM, Serdyn du Toit wrote:

> Hi,
> 
> https://issues.apache.org/jira/browse/OPENJPA-2407
> 
> I had a more indepth look at this and fixed it.  Just to emphasize the
> effect of this issue first - its impossible to run the same SELECT query
> more than once as a NullPointerException occurs (using OpenJpa 2.2.2 and
> MySql Cluster 7.3.2)
> 
> Someone from Oracle will need to drive the following change to
> NdbOpenJPAStoreManager.java (modifying the solution if required of course)
> 
> Fix overview:
> The NdbOpenJPAStoreManager#newQuery(String language) implementation is no
> longer sufficient.
> 
> Fix details:
> The modified NdbOpenJPAStoreManager can either be retrieved from the
> issue's fix (https://issues.apache.org/jira/browse/OPENJPA-2407
> ) or just review the old newQuery(..) and its suggested replacement here:
> 
> package com.mysql.clusterj.openjpa;
> 
> public class NdbOpenJPAStoreManager extends JDBCStoreManager {
> 
> /**
> * https://issues.apache.org/jira/browse/OPENJPA-2407
> * Original broken implementation
> *
> * First method call for query:
> * - parameter language: javax.persistence.JPQL
> * - returns: NdbOpenJPAStoreQuery
> *
> * Successive method calls for identical query actually send in a different
> * language - contrary to perhaps older versions of OpenJpa
> * - parameter language: openjpa.prepared.SQL
> * - returns: broken NdbOpenJPAStoreQuery
> * As the language wasn't recognized by the implementation the
> ExpressionParser
> * will be NULL resulting in a NPE.
> *
> * Analogously the JDBCStoreManager will have returned JDBCStoreQuery on the
> * first method call, and subsequently (contrary to perhaps older versions
> of
> * OpenJpa) returned a PreparedSQLStoreQuery.
> *
> * The solution therefore is to just replace the whole method implementation
> * with that from JDBCStoreManager - just renaming variables as necessary
> * and replacing JDBCStoreQuery with NdbOpenJPAStoreQuery
> (NdbOpenJPAStoreQuery
> * is a Ndb-specific subclass of JDBCStoreQuery with a minor customization)
> *
> * Rest of this class remains as previously.
> */
> // @Override
> // public StoreQuery newQuery(String language) {
> //      ExpressionParser ep = QueryLanguages.parserForLanguage(language);
> //      return new NdbOpenJPAStoreQuery(this, ep);
> // }
> /**
> * Copied and adapted from JDBCStoreManager as discussed above.
> */
>    public StoreQuery newQuery(String language) {
>        StoreQuery sq = newStoreQuery(language);
>        if (sq == null || QueryLanguages.parserForLanguage(language) ==
> null) {
>            return sq;
>        }
> 
>        QueryCache queryCache =
> storeContext.getConfiguration().getDataCacheManagerInstance().getSystemQueryCache();
>        if (queryCache == null) {
>            return sq;
>        }
> 
>        return new QueryCacheStoreQuery(sq, queryCache);
>    }
>    private StoreQuery newStoreQuery(String language) {
>        ExpressionParser ep = QueryLanguages.parserForLanguage(language);
>        if (ep != null) {
>            return new NdbOpenJPAStoreQuery(this, ep);
>        }
>        if (QueryLanguages.LANG_SQL.equals(language)) {
>            return new SQLStoreQuery(this);
>        }
>        if (QueryLanguages.LANG_PREPARED_SQL.equals(language)) {
>            return new PreparedSQLStoreQuery(this);
>        }
>        return null;
>    }

Craig L Russell
Architect, Oracle
http://db.apache.org/jdo
408 276-5638 mailto:Craig.Russell@stripped
P.S. A good JDO? O, Gasp!

Thread
CRUD in MySql general logSerdyn du Toit14 Jul
  • Re: CRUD in MySql general logSerdyn du Toit14 Jul
    • Re: CRUD in MySql general logCraig L Russell14 Jul
      • Re: CRUD in MySql general logSerdyn du Toit15 Jul
        • Re: CRUD in MySql general logCraig L Russell15 Jul
          • Re: CRUD in MySql general logSerdyn du Toit15 Jul
            • Re: CRUD in MySql general logSerdyn du Toit19 Jul
              • Re: CRUD in MySql general logSerdyn du Toit22 Jul
              • Re: CRUD in MySql general logCraig L Russell20 Aug
  • Re: CRUD in MySql general logCraig L Russell14 Jul
    • Re: CRUD in MySql general logSerdyn du Toit14 Jul
      • Re: CRUD in MySql general logSerdyn du Toit15 Jul
        • Re: CRUD in MySql general logCraig L Russell15 Jul
          • Re: CRUD in MySql general logSerdyn du Toit16 Jul
            • Re: CRUD in MySql general logSerdyn du Toit16 Jul
              • Re: CRUD in MySql general logSerdyn du Toit22 Jul
                • Re: CRUD in MySql general logCraig L Russell23 Jul
                  • Re: CRUD in MySql general logSerdyn du Toit23 Jul
                    • Re: CRUD in MySql general logSerdyn du Toit23 Jul