Attachment 176175 Details for Bug 304923 – patch to optimize 1-1 joins

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] patch to optimize 1-1 joins

2010-08-09-onetoone-join-opt.patch (text/plain), 10.76 KB, created by James Sutherland

on 2010-08-09 14:24:32 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: James Sutherland

Created: 2010-08-09 14:24:32 EDT

Size: 10.76 KB

patch

obsolete

>Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/RelationExpression.java
>===================================================================
>--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/RelationExpression.java	(revision 7963)
>+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/expressions/RelationExpression.java	(working copy)
>@@ -468,10 +468,50 @@
>     }
> 
>     /**
>+     * PERF: Optimize out unnecessary joins.
>+     * Check for relation based on foreign keys, i.e. emp.address.id = :id, and avoid join.
>+     * @return null if cannot be optimized, otherwise the optimized normalized expression.
>+     */
>+    protected Expression checkForeignKeyJoinOptimization(Expression first, Expression second, ExpressionNormalizer normalizer) {
>+        if (first.isQueryKeyExpression()
>+                && (((QueryKeyExpression)first).getBaseExpression() != null)
>+                && ((QueryKeyExpression)first).getBaseExpression().isQueryKeyExpression()) {
>+            QueryKeyExpression mappingExpression = (QueryKeyExpression)((QueryKeyExpression)first).getBaseExpression();
>+            if ((mappingExpression.getBaseExpression() != null)
>+                    && mappingExpression.getBaseExpression().isObjectExpression()) {
>+                // Must ensure it has been normalized first.
>+                mappingExpression.getBaseExpression().normalize(normalizer);
>+                DatabaseMapping mapping = mappingExpression.getMapping();
>+                if ((mapping != null) && mapping.isOneToOneMapping()
>+                        && ((OneToOneMapping)mapping).isForeignKeyRelationship()
>+                        && (second.isConstantExpression() || second.isParameterExpression())) {
>+                    DatabaseField targetField = ((QueryKeyExpression)first).getField();
>+                    DatabaseField sourceField = ((OneToOneMapping)mapping).getTargetToSourceKeyFields().get(targetField);
>+                    if (sourceField != null) {
>+                        Expression optimizedExpression = this.operator.expressionFor(mappingExpression.getBaseExpression().getField(sourceField), second);
>+                        // Ensure the base still applies the correct conversion.
>+                        second.setLocalBase(first);
>+                        return optimizedExpression.normalize(normalizer);
>+                    }
>+                }
>+            }
>+        }
>+        return null;
>+    }
>+    
>+    /**
>      * INTERNAL:
>      * Check for object comparison as this requires for the expression to be replaced by the object comparison.
>      */
>     public Expression normalize(ExpressionNormalizer normalizer) {
>+        // PERF: Optimize out unnecessary joins.
>+        Expression optimizedExpression = checkForeignKeyJoinOptimization(this.firstChild, this.secondChild, normalizer);
>+        if (optimizedExpression == null) {
>+            optimizedExpression = checkForeignKeyJoinOptimization(this.secondChild, this.firstChild, normalizer);
>+        }
>+        if (optimizedExpression != null) {
>+            return optimizedExpression;
>+        }
>         if (!isObjectComparison()) {
>             return super.normalize(normalizer);
>         } else {
>Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/IdentityMapManager.java
>===================================================================
>--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/IdentityMapManager.java	(revision 7979)
>+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/IdentityMapManager.java	(working copy)
>@@ -50,7 +50,7 @@
>  * @since TOPLink/Java 1.0
>  */
> public class IdentityMapManager implements Serializable, Cloneable {
>-    protected static final String MONITOR_PREFIX = "Info:Cache:";
>+    protected static final String MONITOR_PREFIX = "Info:CacheSize:";
> 
>     /** A table of identity maps with the key being the domain Class. */
>     protected Map<Class, IdentityMap> identityMaps;
>@@ -104,7 +104,7 @@
>                 releaseReadLock();
>             }
>             this.session.endOperationProfile(SessionProfiler.Caching);
>-            if (cacheKey.getObject() == null) {
>+            if (!this.session.isUnitOfWork() && cacheKey.getObject() == null) {
>                 this.session.updateProfile(MONITOR_PREFIX + domainClass.getSimpleName(), identityMap.getSize());
>             }
>         } else {
>@@ -135,7 +135,7 @@
>                 releaseReadLock();
>             }
>             this.session.endOperationProfile(SessionProfiler.Caching);
>-            if (cacheKey.getObject() == null) {
>+            if (!this.session.isUnitOfWork() && cacheKey.getObject() == null) {
>                 this.session.updateProfile(MONITOR_PREFIX + domainClass.getSimpleName(), identityMap.getSize());
>             }
>         } else {
>@@ -166,7 +166,7 @@
>                 releaseReadLock();
>             }
>             this.session.endOperationProfile(SessionProfiler.Caching);
>-            if (cacheKey.getObject() == null) {
>+            if (!this.session.isUnitOfWork() && cacheKey.getObject() == null) {
>                 this.session.updateProfile(MONITOR_PREFIX + domainClass.getSimpleName(), identityMap.getSize());
>             }
>         } else {
>@@ -197,7 +197,7 @@
>                 releaseReadLock();
>             }
>             this.session.endOperationProfile(SessionProfiler.Caching);
>-            if (cacheKey.getObject() == null) {
>+            if (!this.session.isUnitOfWork() && cacheKey.getObject() == null) {
>                 this.session.updateProfile(MONITOR_PREFIX + domainClass.getSimpleName(), identityMap.getSize());
>             }
>         } else {
>Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/tools/profiler/PerformanceMonitor.java
>===================================================================
>--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/tools/profiler/PerformanceMonitor.java	(revision 7979)
>+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/tools/profiler/PerformanceMonitor.java	(working copy)
>@@ -167,7 +167,7 @@
>         return operationStartTimesByThread;
>     }
> 
>-    protected Map<String, Object> getOperationTimings() {
>+    public Map<String, Object> getOperationTimings() {
>         return operationTimings;
>     }
> 
>Index: jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLComplexTestSuite.java
>===================================================================
>--- jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLComplexTestSuite.java	(revision 7963)
>+++ jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLComplexTestSuite.java	(working copy)
>@@ -56,6 +56,7 @@
> 
> import org.eclipse.persistence.testing.models.jpa.advanced.LargeProject;
> import org.eclipse.persistence.testing.models.jpa.advanced.Project;
>+import org.eclipse.persistence.testing.framework.QuerySQLTracker;
> import org.eclipse.persistence.testing.framework.junit.JUnitTestCase;
> 
> import org.eclipse.persistence.sessions.DatabaseSession;
>@@ -225,6 +226,7 @@
>         suite.addTest(new JUnitJPQLComplexTestSuite("testComplexLike"));
>         suite.addTest(new JUnitJPQLComplexTestSuite("testComplexIn"));
>         suite.addTest(new JUnitJPQLComplexTestSuite("testQueryKeys"));
>+        suite.addTest(new JUnitJPQLComplexTestSuite("complexOneToOneJoinOptimization"));
>         
>         return suite;
>     }
>@@ -3042,13 +3044,13 @@
>             EntityManager em = createEntityManager();
>             String errorMsg = "";
>             Query query;
>-            for(int i=0; i < jpqlStrings.length; i++) {
>-                try{
>-                query = em.createQuery(jpqlStrings[i]);
>-                List result = query.getResultList();
>-                }catch(Exception ex){
>+            for (int i = 0; i < jpqlStrings.length; i++) {
>+                try {
>+                    query = em.createQuery(jpqlStrings[i]);
>+                    query.getResultList();
>+                } catch (Exception ex) {
>                     ex.printStackTrace();
>-                    errorMsg += '\t' + jpqlStrings[i] + " - "+ex+'\n';
>+                    errorMsg += '\t' + jpqlStrings[i] + " - " + ex + '\n';
>                 }
>             }
>             closeEntityManager(em);
>@@ -3061,24 +3063,25 @@
>     
>     /* Test nested functions with FUNC*/
>     public void testNestedFUNCs() {
>-        if(!getServerSession().getPlatform().isOracle()) {
>+        if (!getServerSession().getPlatform().isOracle()) {
>             warning("The test testNestedFUNCs is supported on Oracle only");
>             return;
>-        }else{
>+        } else {
>             String jpqlString = "select FUNC('NVL',FUNC('TO_NUMBER', FUNC('DECODE', FUNC('SUBSTRB', e.firstName,1,1),' ', NULL,FUNC('SUBSTRB',e.lastName,1,10)), null), -99) from Employee e";
>             EntityManager em = createEntityManager();
>             Query query;
>-            try{
>-            query = em.createQuery(jpqlString);
>-            List result = query.getResultList();
>-            }catch(Exception ex){
>+            try {
>+                query = em.createQuery(jpqlString);
>+                query.getResultList();
>+            } catch (Exception ex) {
>                 ex.printStackTrace();
>                 fail("nested fucntions don't work, the error is" + ex.getMessage());
>-            }finally{
>+            } finally {
>                 closeEntityManager(em);
>             }
>         }
>     }
>+    
>     public void testFunctionInSelect() {
>         EntityManager em = createEntityManager();
>         Query query = em.createQuery("Select UPPER(e.firstName) from Employee e");
>@@ -3205,4 +3208,27 @@
>         query.getResultList();
>         closeEntityManager(em);
>     }
>+    
>+    /**
>+     * Test that id comparisons across relationships avoid the join.
>+     */
>+    public void complexOneToOneJoinOptimization()
>+    {
>+        // Count SQL.
>+        QuerySQLTracker counter = new QuerySQLTracker(getServerSession());
>+        try {
>+            EntityManager em = createEntityManager();
>+            String jpql = "SELECT e FROM Employee e where e.address.ID = 5";
>+            Query query = em.createQuery(jpql);
>+            query.getResultList();
>+            String sql = (String)counter.getSqlStatements().get(0);
>+            if (sql.indexOf("CMP3_ADDRESS") != -1) {
>+                fail("Join to address should have been optimized.");
>+            }
>+        } finally {
>+            if (counter != null) {
>+                counter.remove();
>+            }
>+        }
>+    }
> }

Actions: View | Diff

Attachments on bug 304923: 161230 | 176175