From 6937856e2b5e1261d7773abd7c45230ca6ddb844 Mon Sep 17 00:00:00 2001 From: "A. Abram White" Date: Wed, 11 Oct 2006 00:16:53 +0000 Subject: [PATCH] Optimize queries of the form "select e from ... where e.rel.id = :x" to not join across "rel" for std fk->pk joins. git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@462646 13f79535-47bb-0310-9956-ffa450edef68 --- .../openjpa/jdbc/kernel/exps/PCPath.java | 72 ++++++++++--- .../jdbc/meta/strats/RelationStrategies.java | 2 +- .../persistence/query/ManyOneEntity.java | 59 ++++++++++ .../TestQueryIdOfRelationDoesNotJoin.java | 101 ++++++++++++++++++ 4 files changed, 221 insertions(+), 13 deletions(-) create mode 100755 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/ManyOneEntity.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestQueryIdOfRelationDoesNotJoin.java diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java index 33fe27e47..420c0eccb 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java @@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.meta.ClassMapping; import org.apache.openjpa.jdbc.meta.FieldMapping; import org.apache.openjpa.jdbc.meta.ValueMapping; import org.apache.openjpa.jdbc.schema.Column; +import org.apache.openjpa.jdbc.schema.ForeignKey; import org.apache.openjpa.jdbc.schema.Schemas; import org.apache.openjpa.jdbc.sql.Joins; import org.apache.openjpa.jdbc.sql.Result; @@ -352,6 +353,7 @@ class PCPath Action action; Variable var; Iterator itr = (_actions == null) ? null : _actions.iterator(); + FieldMapping field; while (itr != null && itr.hasNext()) { action = (Action) itr.next(); @@ -369,8 +371,18 @@ class PCPath rel.getTable()); } else { // move past the previous field, if any - if (pstate.field != null) + field = (FieldMapping) action.data; + if (pstate.field != null) { + // if this is the second-to-last field and the last is + // the related field this field joins to, no need to + // traverse: just use this field's fk columns + if (!itr.hasNext() && (flags & JOIN_REL) == 0 + && isJoinedField(pstate.field, key, field)) { + pstate.cmpfield = field; + break; + } rel = traverseField(pstate, key, forceOuter, false); + } // mark if the next traversal should go through // the key rather than value @@ -378,7 +390,7 @@ class PCPath forceOuter |= action.op == Action.GET_OUTER; // get mapping for the current field - pstate.field = (FieldMapping) action.data; + pstate.field = field; owner = pstate.field.getDefiningMapping(); if (pstate.field.getManagement() != FieldMapping.MANAGE_PERSISTENT) @@ -420,15 +432,50 @@ class PCPath return pstate; } + /** + * Return whether the given source field joins to the given target field. + */ + private static boolean isJoinedField(FieldMapping src, boolean key, + FieldMapping target) { + ValueMapping vm; + switch (src.getTypeCode()) { + case JavaTypes.ARRAY: + case JavaTypes.COLLECTION: + vm = src.getElementMapping(); + break; + case JavaTypes.MAP: + vm = (key) ? src.getKeyMapping() : src.getElementMapping(); + break; + default: + vm = src; + } + if (vm.getJoinDirection() != ValueMapping.JOIN_FORWARD) + return false; + ForeignKey fk = vm.getForeignKey(); + if (fk == null) + return false; + + // foreign key must join to target columns + Column[] rels = fk.getColumns(); + Column[] pks = target.getColumns(); + if (rels.length != pks.length) + return false; + for (int i = 0; i < rels.length; i++) + if (fk.getPrimaryKeyColumn(rels[i]) != pks[i]) + return false; + return true; + } + /** * Expression state. */ private static class PathExpState extends ExpState { - private FieldMapping field = null; - private Column[] cols = null; - private boolean joinedRel = false; + public FieldMapping field = null; + public FieldMapping cmpfield = null; + public Column[] cols = null; + public boolean joinedRel = false; public PathExpState(Joins joins) { super(joins); @@ -483,15 +530,16 @@ class PCPath public Object toDataStoreValue(Select sel, ExpContext ctx, ExpState state, Object val) { PathExpState pstate = (PathExpState) state; - if (pstate.field != null) { + FieldMapping field = (pstate.cmpfield != null) ? pstate.cmpfield + : pstate.field; + if (field != null) { if (_key) - return pstate.field.toKeyDataStoreValue(val, ctx.store); - if (pstate.field.getElement().getDeclaredTypeCode() - != JavaTypes.OBJECT) - return pstate.field.toDataStoreValue(val, ctx.store); + return field.toKeyDataStoreValue(val, ctx.store); + if (field.getElement().getDeclaredTypeCode() != JavaTypes.OBJECT) + return field.toDataStoreValue(val, ctx.store); - val = pstate.field.getExternalValue(val, ctx.store.getContext()); - return pstate.field.toDataStoreValue(val, ctx.store); + val = field.getExternalValue(val, ctx.store.getContext()); + return field.toDataStoreValue(val, ctx.store); } return _class.toDataStoreValue(val, _class.getPrimaryKeyColumns(), ctx.store); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationStrategies.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationStrategies.java index 61a0eaf34..3eaef9894 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationStrategies.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationStrategies.java @@ -74,7 +74,7 @@ public class RelationStrategies { public static Object toDataStoreValue(ValueMapping vm, Object val, JDBCStore store) { ClassMapping rel; - if (val == null || val.getClass() == vm.getType()) + if (val == null || val.getClass() == vm.getType()) rel = vm.getTypeMapping(); // common case else rel = vm.getMappingRepository().getMapping(val.getClass(), diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/ManyOneEntity.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/ManyOneEntity.java new file mode 100755 index 000000000..8486c9988 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/ManyOneEntity.java @@ -0,0 +1,59 @@ +/* + * Copyright 2006 The Apache Software Foundation. + * + * Licensed 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.query; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Version; + +@Entity +public class ManyOneEntity { + + @Id + @GeneratedValue + private long id; + + private String name; + + @ManyToOne(cascade=CascadeType.ALL) + private ManyOneEntity rel; + + @Version + private Integer optLock; + + public long getId() { + return id; + } + + public ManyOneEntity getRel() { + return rel; + } + + public void setRel(ManyOneEntity rel) { + this.rel = rel; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestQueryIdOfRelationDoesNotJoin.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestQueryIdOfRelationDoesNotJoin.java new file mode 100644 index 000000000..40e308d83 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestQueryIdOfRelationDoesNotJoin.java @@ -0,0 +1,101 @@ +/* + * Copyright 2006 The Apache Software Foundation. + * + * Licensed 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.query; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Persistence; +import javax.persistence.Query; + +import junit.framework.TestCase; +import junit.textui.TestRunner; + +/** + * Test that querying the id of a related many-one (or one-one) does not create + * a join across the tables. + * + * @author Abe White + */ +public class TestQueryIdOfRelationDoesNotJoin + extends TestCase { + + private EntityManagerFactory emf; + private long e3Id; + + public void setUp() { + Map props = new HashMap(); + props.put("openjpa.MetaDataFactory", "jpa(Types=" + + ManyOneEntity.class.getName() + ")"); + emf = Persistence.createEntityManagerFactory("test", props); + + ManyOneEntity e1 = new ManyOneEntity(); + e1.setName("e1"); + ManyOneEntity e2 = new ManyOneEntity(); + e2.setName("e2"); + ManyOneEntity e3 = new ManyOneEntity(); + e3.setName("e3"); + e1.setRel(e3); + e2.setRel(e1); + + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.persist(e1); + em.getTransaction().commit(); + e3Id = e3.getId(); + + // we intentionally create an orphaned reference on e1.rel + em.getTransaction().begin(); + em.remove(e3); + em.getTransaction().commit(); + em.close(); + } + + public void tearDown() { + if (emf == null) + return; + try { + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.createQuery("delete from ManyOneEntity").executeUpdate(); + em.getTransaction().commit(); + em.close(); + emf.close(); + } catch (Exception e) { + e.printStackTrace(); + } + } + + public void testQuery() { + EntityManager em = emf.createEntityManager(); + Query q = em.createQuery("select e from ManyOneEntity e " + + "where e.rel.id = :id").setParameter("id", e3Id); + List res = q.getResultList(); + assertEquals(1, res.size()); + + ManyOneEntity e = (ManyOneEntity) res.get(0); + assertEquals("e1", e.getName()); + assertNull(e.getRel()); + em.close(); + } + + public static void main(String[] args) { + TestRunner.run(TestQueryIdOfRelationDoesNotJoin.class); + } +} +