Hi,
Do we have testcases (new or modified) that we could integrate as well?
Continually enhancing our test bucket is key to our success.

Thanks,
Kevin

On Tue, Jul 22, 2008 at 12:41 PM, <[EMAIL PROTECTED]> wrote:

> Author: fancy
> Date: Tue Jul 22 10:41:30 2008
> New Revision: 678828
>
> URL: http://svn.apache.org/viewvc?rev=678828&view=rev
> Log:
> OPENJPA-241 Extra SQL on lazy CMR load
> commit openjpa_241.patch on behalf of Fay Wang
>
> Modified:
>
>  
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
>
>  
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java
>
>  
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractResult.java
>
>  
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MergedResult.java
>
>  
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Result.java
>
> Modified:
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java?rev=678828&r1=678827&r2=678828&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
> (original)
> +++
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
> Tue Jul 22 10:41:30 2008
> @@ -349,6 +349,13 @@
>             // from the indicator is a subclass of expected type
>             sm.initialize(type, state);
>
> +            if (info != null && info.result != null) {
> +                FieldMapping mappedByFieldMapping = info.result.
> +                    getMappedByFieldMapping();
> +                Object mappedByObject = info.result.getMappedByValue();
> +                if (mappedByFieldMapping != null && mappedByObject !=
> null)
> +                    setMappedBy(sm, mappedByFieldMapping, mappedByObject);
> +            }
>             // load the selected mappings into the given state manager
>             if (res != null) {
>                 // re-get the mapping in case the instance was a subclass
> @@ -362,7 +369,19 @@
>                 res.close();
>         }
>     }
> -
> +
> +    protected void setMappedBy(OpenJPAStateManager sm,
> +        FieldMapping mappedByFieldMapping, Object mappedByObject) {
> +        ClassMapping mapping = (ClassMapping) sm.getMetaData();
> +        FieldMapping[] fms = mapping.getDeclaredFieldMappings();
> +        for (int i = 0; i < fms.length; i++) {
> +            if (fms[i] == mappedByFieldMapping) {
> +                sm.storeObject(fms[i].getIndex(), mappedByObject);
> +                return;
> +            }
> +        }
> +    }
> +
>     /**
>      * This method is to provide override for non-JDBC or JDBC-like
>      * implementation of getting version from the result set.
>
> Modified:
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java?rev=678828&r1=678827&r2=678828&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java
> (original)
> +++
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java
> Tue Jul 22 10:41:30 2008
> @@ -25,6 +25,7 @@
>  import java.util.List;
>  import java.util.Map;
>
> +import org.apache.openjpa.enhance.PersistenceCapable;
>  import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
>  import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
>  import org.apache.openjpa.jdbc.kernel.JDBCFetchConfigurationImpl;
> @@ -33,12 +34,12 @@
>  import org.apache.openjpa.jdbc.meta.ClassMapping;
>  import org.apache.openjpa.jdbc.meta.FieldMapping;
>  import org.apache.openjpa.jdbc.meta.FieldStrategy;
> +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.sql.Joins;
>  import org.apache.openjpa.jdbc.sql.LogicalUnion;
>  import org.apache.openjpa.jdbc.sql.Result;
> -import org.apache.openjpa.jdbc.sql.SQLBuffer;
>  import org.apache.openjpa.jdbc.sql.Select;
>  import org.apache.openjpa.jdbc.sql.SelectExecutor;
>  import org.apache.openjpa.jdbc.sql.SelectImpl;
> @@ -46,6 +47,7 @@
>  import org.apache.openjpa.kernel.OpenJPAStateManager;
>  import org.apache.openjpa.lib.log.Log;
>  import org.apache.openjpa.lib.util.Localizer;
> +import org.apache.openjpa.meta.ClassMetaData;
>  import org.apache.openjpa.meta.JavaTypes;
>  import org.apache.openjpa.util.ChangeTracker;
>  import org.apache.openjpa.util.Id;
> @@ -305,13 +307,61 @@
>
>             if (field.getOrderColumn() != null)
>                 seq = res.getInt(field.getOrderColumn(), orderJoins) + 1;
> -            add(store, coll, loadElement(null, store, fetch, res,
> dataJoins));
> +
> +            // for inverseEager field
> +            setMappedBy(oid, sm, coll, res);
> +            Object val = loadElement(null, store, fetch, res, dataJoins);
> +            add(store, coll, val);
>         }
>         res.close();
>
>         return rels;
>     }
>
> +    private void setMappedBy(Object oid, OpenJPAStateManager sm, Object
> coll,
> +        Result res) {
> +        // for inverseEager field
> +        FieldMapping mappedByFieldMapping = field.getMappedByMapping();
> +        PersistenceCapable mappedByValue = null;
> +
> +        if (mappedByFieldMapping != null) {
> +            ValueMapping val = mappedByFieldMapping.getValueMapping();
> +            ClassMetaData decMeta = val.getTypeMetaData();
> +            // this inverse field does not have corresponding classMapping
> +            // its value may be a collection/map etc.
> +            if (decMeta == null)
> +                return;
> +
> +            if (oid.equals(sm.getObjectId())) {
> +                mappedByValue = sm.getPersistenceCapable();
> +                res.setMappedByFieldMapping(mappedByFieldMapping);
> +                res.setMappedByValue(mappedByValue);
> +            } else if (coll instanceof Collection &&
> +                ((Collection) coll).size() > 0) {
> +                // Customer (1) <--> Orders(n)
> +                // coll contains the values of the toMany field (Orders)
> +                // get the StateManager of this toMany value
> +                // and find the value of the inverse mappedBy field
> (Customer)
> +                // for this toMacdny field
> +                PersistenceCapable pc = (PersistenceCapable)
> +                    ((Collection) coll).iterator().next();
> +                OpenJPAStateManager sm1 = (OpenJPAStateManager) pc.
> +                    pcGetStateManager();
> +                FieldMapping[] fms = ((ClassMapping) sm1.getMetaData()).
> +                    getDeclaredFieldMappings();
> +                for (int i = 0; i < fms.length; i++) {
> +                    if (fms[i] == mappedByFieldMapping) {
> +                        res.setMappedByValue(sm1.fetchObject(fms[i].
> +                            getIndex()));
> +                        break;
> +                    }
> +                }
> +            } else {
> +                res.setMappedByValue(null);
> +            }
> +        }
> +    }
> +
>     /**
>      * Extract the oid value from the given result. If the next oid is the
>      * same as the given one, returns the given JVM instance.
> @@ -555,6 +605,7 @@
>             while (res.next()) {
>                 if (ct != null && field.getOrderColumn() != null)
>                     seq = res.getInt(field.getOrderColumn());
> +                setMappedBy(sm.getObjectId(), sm, coll, res);
>                        add(store, coll, loadElement(sm, store, fetch, res,
>                        resJoins[res.indexOf()]));
>             }
>
> Modified:
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractResult.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractResult.java?rev=678828&r1=678827&r2=678828&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractResult.java
> (original)
> +++
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/AbstractResult.java
> Tue Jul 22 10:41:30 2008
> @@ -80,6 +80,8 @@
>     private boolean _locking = false;
>     private boolean _ignoreNext = false;
>     private boolean _last = false;
> +    private FieldMapping _mappedByFieldMapping = null;
> +    private Object _mappedByValue = null;
>
>     public Object getEager(FieldMapping key) {
>         Map map = getEagerMap(true);
> @@ -118,6 +120,8 @@
>      */
>     public void close() {
>         closeEagerMap(_eager);
> +        _mappedByFieldMapping = null;
> +        _mappedByValue = null;
>     }
>
>     /**
> @@ -238,6 +242,22 @@
>         _base = base;
>     }
>
> +    public FieldMapping getMappedByFieldMapping() {
> +        return (_gotEager) ? null : _mappedByFieldMapping;
> +    }
> +
> +    public void setMappedByFieldMapping(FieldMapping fieldMapping) {
> +        _mappedByFieldMapping = fieldMapping;
> +    }
> +
> +    public Object getMappedByValue() {
> +        return (_gotEager) ? null : _mappedByValue;
> +    }
> +
> +    public void setMappedByValue(Object mappedByValue) {
> +        _mappedByValue = mappedByValue;
> +    }
> +
>     public int indexOf() {
>         return _index;
>     }
>
> Modified:
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MergedResult.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MergedResult.java?rev=678828&r1=678827&r2=678828&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MergedResult.java
> (original)
> +++
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/MergedResult.java
> Tue Jul 22 10:41:30 2008
> @@ -211,6 +211,22 @@
>         _res[_idx].setBaseMapping(mapping);
>     }
>
> +    public FieldMapping getMappedByFieldMapping() {
> +        return _res[_idx].getMappedByFieldMapping();
> +    }
> +
> +    public void setMappedByFieldMapping(FieldMapping fieldMapping) {
> +        _res[_idx].setMappedByFieldMapping(fieldMapping);
> +    }
> +
> +    public Object getMappedByValue() {
> +        return _res[_idx].getMappedByValue();
> +    }
> +
> +    public void setMappedByValue(Object mappedByValue) {
> +        _res[_idx].setMappedByValue(mappedByValue);
> +    }
> +
>     public int indexOf() {
>         return _res[_idx].indexOf();
>     }
>
> Modified:
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Result.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Result.java?rev=678828&r1=678827&r2=678828&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Result.java
> (original)
> +++
> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Result.java
> Tue Jul 22 10:41:30 2008
> @@ -161,6 +161,36 @@
>     public void setBaseMapping(ClassMapping mapping);
>
>     /**
> +     * If this is the result used to select a toMany relationship,
> +     * the mappedByFieldMapping is field mapping representing
> +     * the inverse relationship. This is to avoid unneeded
> +     * extra sql to retrieve the eager inverse field.
> +     */
> +    public FieldMapping getMappedByFieldMapping();
> +
> +    /**
> +     * If this is the result used to select a toMany relationship,
> +     * the mappedByFieldMapping is field mapping representing
> +     * the inverse relationship. This is to avoid unneeded
> +     * extra sql to retrieve the eager inverse field.
> +     */
> +    public void setMappedByFieldMapping(FieldMapping fieldMapping);
> +
> +    /**
> +     * If this is the result used to select a toMany relationship,
> +     * the mappedByValue is value of the owner of the toMany relationship.
> +     * This is to avoid unneeded extra sql to retrieve the eager inverse
> field.
> +     */
> +    public Object getMappedByValue();
> +
> +    /**
> +     * If this is the result used to select a toMany relationship,
> +     * the mappedByValue is value of the owner of the toMany relationship.
> +     * This is to avoid unneeded extra sql to retrieve the eager inverse
> field.
> +     */
> +    public void setMappedByValue(Object mappedByValue);
> +
> +    /**
>      * The index of the select within the UNION that the current row
>      * corresponds to, or 0.
>      */
>
>
>

Reply via email to