You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by ht...@apache.org on 2014/03/08 00:04:16 UTC

svn commit: r1575445 - in /openjpa/branches/1.2.x: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/ openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/

Author: hthomann
Date: Fri Mar  7 23:04:16 2014
New Revision: 1575445

URL: http://svn.apache.org/r1575445
Log:
OPENJPA-2475: A query with LEFT FETCH JOIN returns incorrect results - applied changes to 1.2.x.

Added:
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java
Modified:
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.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=1575445&r1=1575444&r2=1575445&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 Fri Mar  7 23:04:16 2014
@@ -224,11 +224,7 @@ public abstract class StoreCollectionFie
                 joins = sel.outer(joins);
             if (!selectOid) {
                 Column[] refs = getJoinForeignKey(elem).getColumns();
-                if (requiresOrderBy()) {
-                    sel.orderBy(refs, true, joins, true);
-                } else {
-                    sel.select(refs, joins);
-                }
+                sel.orderBy(refs, true, joins, true);
             }
             field.orderLocal(sel, elem, joins);
         }
@@ -600,8 +596,4 @@ public abstract class StoreCollectionFie
     protected ForeignKey getJoinForeignKey() {
         return getJoinForeignKey(getDefaultElementMapping(false));
     }
-    
-    boolean requiresOrderBy() {
-    	return List.class.isAssignableFrom(field.getProxyType());
-    }
 }

Added: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java?rev=1575445&view=auto
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java (added)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java Fri Mar  7 23:04:16 2014
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jpql.joins.leftfetch;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.OneToMany;
+import javax.persistence.OrderBy;
+
+@Entity
+public class DepartmentTest{
+	
+    @Id
+    private String primaryKey;
+
+    @OrderBy("name")
+    @OneToMany(mappedBy = "departmentTest")
+	private Set<PersonTest> persons = new HashSet<PersonTest>();
+
+	private String name;
+	
+	public Set<PersonTest> getPersons() {
+		return persons;
+	}
+
+	public void setPersons(Set<PersonTest> persons) {
+		this.persons = persons;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		this.name = name;
+	}
+
+	public String getPrimaryKey() {
+		return this.primaryKey;
+	}
+
+	public void setPrimaryKey(String primaryKey) {
+		this.primaryKey = primaryKey;
+	}
+}

Added: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java?rev=1575445&view=auto
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java (added)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java Fri Mar  7 23:04:16 2014
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jpql.joins.leftfetch;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.ManyToOne;
+
+import org.apache.openjpa.persistence.jdbc.ForeignKey;
+
+@Entity
+public class PersonTest  {
+
+	@Id
+	private String primaryKey;
+
+	@ManyToOne
+    @ForeignKey
+	private DepartmentTest departmentTest;
+	
+    private String name;
+    
+	public DepartmentTest getDepartmentTest() {
+		return departmentTest;
+	}
+
+	public void setDepartmentTest(DepartmentTest departmentTest) {
+		this.departmentTest = departmentTest;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		this.name = name;
+	}
+
+	public String getPrimaryKey() {
+		return this.primaryKey;
+	}
+
+	public void setPrimaryKey(String primaryKey) {
+		this.primaryKey = primaryKey;
+	}
+
+    @Override
+    public String toString()
+    {
+        final StringBuilder sb = new StringBuilder();
+        sb.append(this.getName()).append(" - ").append(this.getPrimaryKey()).append(" ");
+        return sb.toString();
+    }
+}

Added: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java?rev=1575445&view=auto
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java (added)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java Fri Mar  7 23:04:16 2014
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jpql.joins.leftfetch;
+
+import java.util.List;
+
+import javax.persistence.EntityManager;
+import javax.persistence.Query;
+
+import org.apache.openjpa.persistence.OpenJPAPersistence;
+import org.apache.openjpa.persistence.OpenJPAQuery;
+import org.apache.openjpa.persistence.test.SingleEMFTestCase;
+
+public class TestJoinLeftFetch extends SingleEMFTestCase {    
+    
+    public void setUp() {        
+        setUp("openjpa.jdbc.UpdateManager", "operation-order",
+              DepartmentTest.class, PersonTest.class);
+        dropTables();
+        createTestData();
+    }    
+    
+    /*
+     * This test fails (prior to OJ-2475) because the 
+     * DepartmentTests are not populated with the correct
+     * number of PersonTests
+     */
+    public void testReadDepartmentsWithLeftJoinFetch() {
+
+        EntityManager em = emf.createEntityManager();
+
+        String qStrDIST = "SELECT DISTINCT dept FROM DepartmentTest "
+            + "dept LEFT JOIN FETCH dept.persons";
+        
+        Query query = em.createQuery(qStrDIST);
+        List<DepartmentTest> depts = query.getResultList();
+        verifySize(depts);
+        em.close();
+        dropTables();
+    }
+
+    public void verifySize(List<DepartmentTest> depts){
+        for (DepartmentTest department : depts) {
+            if (department.getPrimaryKey().equals("001")) {           
+//                System.out.println("Dept: " + department.getName());
+  //              Iterator i = department.getPersons().iterator();
+    //            while (i.hasNext()){
+      //              System.out.println("i.next() = " + i.next());
+        //        }
+                assertEquals("Size should be 3", 3, department.getPersons().size());
+            }
+            if (department.getPrimaryKey().equals("002")) {           
+//                System.out.println("Dept: " + department.getName());
+  //              Iterator i = department.getPersons().iterator();
+    //            while (i.hasNext()){
+      //              System.out.println("i.next() = " + i.next());
+        //        }
+                assertEquals("Size should be 2", 2, department.getPersons().size());
+            }
+        }
+    }
+    
+    public void dropTables(){
+        EntityManager em = emf.createEntityManager();        
+        em.getTransaction().begin();
+        em.createQuery("Delete from PersonTest").executeUpdate();
+        em.createQuery("Delete from DepartmentTest").executeUpdate();
+        em.getTransaction().commit();
+        em.close();  
+    }
+    
+    /*
+     * This test works as expected.
+     */
+    public void testReadDepartmentsWithFetchPlan() {
+
+        EntityManager em = emf.createEntityManager();
+
+        OpenJPAQuery query = OpenJPAPersistence.cast(em.createQuery(" SELECT dept FROM "
+            + " DepartmentTest dept "));
+        query.getFetchPlan().addField(DepartmentTest.class, "persons");
+        
+        verifySize(query.getResultList());
+        
+        em.close();
+        dropTables();
+    }
+
+    /*
+     * This test works as expected.
+     */
+    public void testReadDepartmentsWithLeftJoinFetchAndOrderBy() {
+
+        EntityManager em = emf.createEntityManager();
+
+        Query query = em.createQuery(" SELECT dept FROM " + " DepartmentTest dept "
+            + " LEFT JOIN FETCH dept.persons ORDER BY dept.primaryKey");
+        verifySize(query.getResultList());
+        
+        em.close();
+        dropTables();
+    }
+
+    public void createTestData() {
+        // NOTE: This test depends upon the the PersonTest
+        // to be un-ordered w.r.t the DepartmentTest FK.
+        // I've executed a flush after each entity creation 
+        // in an attempt that the FKs will not be ordered.  
+        // @OrderBy is used in the DepartmentTest in order
+        // to ensure things aren't orderd by the FK.
+        
+        EntityManager em = emf.createEntityManager();
+        em.getTransaction().begin();
+        
+        DepartmentTest dt1 = new DepartmentTest();
+        dt1.setPrimaryKey("001");
+        dt1.setName("Dept001");
+        em.persist(dt1);
+                
+        DepartmentTest dt2 = new DepartmentTest();
+        dt2.setPrimaryKey("002");
+        dt2.setName("Dept002");
+        em.persist(dt2);  
+        
+        PersonTest pt = new PersonTest();
+        pt.setPrimaryKey("1");
+        pt.setName("John");
+        pt.setDepartmentTest(dt1);
+        em.persist(pt);
+        em.flush();
+
+        pt = new PersonTest();
+        pt.setPrimaryKey("2");
+        pt.setName("Mark");
+        pt.setDepartmentTest(dt1);
+        em.persist(pt);
+        em.flush();
+        
+        pt = new PersonTest();
+        pt.setPrimaryKey("3");
+        pt.setName("Stuart");
+        pt.setDepartmentTest(dt2);
+        em.persist(pt);
+        em.flush();
+        
+        pt = new PersonTest();
+        pt.setPrimaryKey("4");
+        pt.setName("Jim");
+        pt.setDepartmentTest(dt1);
+        em.persist(pt);
+        em.flush();
+
+        pt = new PersonTest();
+        pt.setPrimaryKey("5");
+        pt.setName("Fred");
+        pt.setDepartmentTest(dt2);
+        em.persist(pt);
+        em.flush();
+
+        em.getTransaction().commit();
+        em.close();
+    }    
+}
+
+
+
+