diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java index a6a3f604a..2e629b885 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java @@ -220,11 +220,7 @@ public abstract class StoreCollectionFieldStrategy 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); } @@ -601,10 +597,6 @@ public abstract class StoreCollectionFieldStrategy return getJoinForeignKey(getDefaultElementMapping(false)); } - boolean requiresOrderBy() { - return List.class.isAssignableFrom(field.getProxyType()); - } - /** * Gets the identity value of the given instance that is suitable to join to the given foreign key. * The special case of the foreign key being a relation identifier will encode the value. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMultipleLevelDerivedIdentity1.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMultipleLevelDerivedIdentity1.java index 9c285b561..051b4f8bd 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMultipleLevelDerivedIdentity1.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMultipleLevelDerivedIdentity1.java @@ -215,7 +215,7 @@ public class TestMultipleLevelDerivedIdentity1 extends SQLListenerTestCase { EntityManager em = emf.createEntityManager(); Library1 lib = em.find(Library1.class, LIBRARY_NAME); assertNotNull(lib); - assertSQLFragnments(sql, "ORDER BY t1.LIBRARY_NAME ASC, t1.BOOK_NAME ASC"); + assertSQLFragnments(sql, "ORDER BY", "t1.LIBRARY_NAME ASC, t1.BOOK_NAME ASC"); em.close(); } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java new file mode 100644 index 000000000..1e04fb1b3 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/DepartmentTest.java @@ -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 persons = new HashSet(); + + private String name; + + public Set getPersons() { + return persons; + } + + public void setPersons(Set 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; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java new file mode 100644 index 000000000..74392ccb6 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/PersonTest.java @@ -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(); + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java new file mode 100644 index 000000000..c20d9ad18 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/joins/leftfetch/TestJoinLeftFetch.java @@ -0,0 +1,169 @@ +/* + * 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(DROP_TABLES, DepartmentTest.class, PersonTest.class); + 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 depts = query.getResultList(); + verifySize(depts); + em.close(); + } + + public void verifySize(List 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()); + } + } + } + + /* + * 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(); + } + + /* + * 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(); + } + + 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(); + } +} + + + +