You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by mi...@apache.org on 2010/02/11 20:57:07 UTC

svn commit: r909125 - in /openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc: kernel/JDBCStoreManager.java meta/strats/RelationFieldStrategy.java sql/LogicalUnion.java sql/SelectExecutor.java sql/SelectImpl.java

Author: mikedd
Date: Thu Feb 11 19:57:07 2010
New Revision: 909125

URL: http://svn.apache.org/viewvc?rev=909125&view=rev
Log:
OPENJPA-1001:
Prevent index out of bounds exception when checking parameter list.
Submitted By: B.J. Reed.

Modified:
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java

Modified: openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java?rev=909125&r1=909124&r2=909125&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java (original)
+++ openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java Thu Feb 11 19:57:07 2010
@@ -497,90 +497,13 @@
     private Result getInitializeStateResult(OpenJPAStateManager sm,
         ClassMapping mapping, JDBCFetchConfiguration fetch, int subs)
         throws SQLException {
-        List params = new ArrayList();
-        Select sel = newSelect(sm, mapping, fetch, subs, params);
-        if (sel == null) return null;
-        return sel.execute(this, fetch, params);
-    }
-    
-    Map<SelectKey, Select> selectImplCacheMap = null;
-    private Select newSelect(OpenJPAStateManager sm,
-        ClassMapping mapping, JDBCFetchConfiguration fetch, int subs,
-        List params) {
-        if (!_isQuerySQLCache) 
-            return newSelect(sm, mapping, fetch, subs);       
-           
-        if (selectImplCacheMap == null) {
-            selectImplCacheMap =
-                getCacheMapFromQuerySQLCache(JDBCStoreManager.class);
-        }
-         
-        JDBCFetchConfiguration fetchClone = new JDBCFetchConfigurationImpl();
-        fetchClone.copy(fetch);
-        SelectKey selKey = new SelectKey(mapping, null, fetchClone);
-        Select sel = null;
-        boolean found = true;
-        Object obj = selectImplCacheMap.get(selKey);
-        if (obj == null) {
-            synchronized (selectImplCacheMap) {
-                obj = selectImplCacheMap.get(selKey);
-                if (obj == null) {
-                    // Not found in cache, create a new select
-                    obj = newSelect(sm, mapping, fetch, subs);
-                    found = false;
-                }
-                    
-                if (obj == null) {
-                    // If the generated SelectImpl is null, store a generic
-                    // known object in the cache as a placeholder. Some map 
-                    // implementations do not allow null values.
-                    obj = _nullCacheValue;
-                    found = false;
-                }
-                else if (obj != _nullCacheValue)
-                {
-                    sel = (Select)obj;
-                    if (sel.getSQL() == null) {
-                        sel.setSQL(this, fetch);
-                        found = false;
-                    }
-                }
-                if (!found) {
-                    addToSqlCache(selectImplCacheMap, selKey, obj);
-                }
-            }
-        }
-
-        if (obj != null && obj != _nullCacheValue)
-            sel = (Select) obj;
-
-        Log log = _conf.getLog(JDBCConfiguration.LOG_JDBC);
-        if (log.isTraceEnabled()) {
-            if (!found)
-                log.trace(_loc.get("cache-missed", mapping, this.getClass()));
-            else
-                log.trace(_loc.get("cache-hit", mapping, this.getClass()));
-        }
-
-        if (sel == null)
-            return null;
-        
-        Object oid = sm.getObjectId();
-        Column[] cols = mapping.getPrimaryKeyColumns();
-        sel.wherePrimaryKey(mapping, cols, cols, oid, this, 
-        	null, null, params);
-        return sel;
-    }
-
-    protected Select newSelect(OpenJPAStateManager sm,
-        ClassMapping mapping, JDBCFetchConfiguration fetch, int subs) {
         Select sel = _sql.newSelect();
-        if (!select(sel, mapping, subs, sm, null, fetch,
-            JDBCFetchConfiguration.EAGER_JOIN, true, false))
+        if (!select(sel, mapping, subs, sm, null, fetch, JDBCFetchConfiguration.EAGER_JOIN, true, false)) {
             return null;
+        }
         sel.wherePrimaryKey(sm.getObjectId(), mapping, this);
         sel.setExpectedResultCount(1, false);
-        return sel;
+        return sel.execute(this, fetch);
     }
 
     /**

Modified: openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java?rev=909125&r1=909124&r2=909125&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java (original)
+++ openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java Thu Feb 11 19:57:07 2010
@@ -617,99 +617,6 @@
         final int subs = field.getSelectSubclasses();
         final Joins[] resJoins = new Joins[rels.length];
 
-        //cache union for field here
-        //select data for this sm
-        Union union = null;
-        SelectImpl sel = null;
-        List parmList = null;
-
-        if (!((JDBCStoreManager)store).isQuerySQLCacheOn())
-            union = newUnion(sm, store, fetch, rels, subs, resJoins);
-        else {
-            if (relationFieldUnionCache == null) {
-                relationFieldUnionCache =
-                    ((JDBCStoreManager) store)
-                        .getCacheMapFromQuerySQLCache(RelationFieldStrategy.class);
-            }
-            boolean found = true;
-            JDBCFetchConfiguration fetchClone = new JDBCFetchConfigurationImpl();
-            fetchClone.copy(fetch);
-            JDBCStoreManager.SelectKey selKey = 
-                new JDBCStoreManager.SelectKey(null, field, fetch);
-            Object[] obj = relationFieldUnionCache.get(selKey);
-            if (obj != null) {
-                union = (Union) obj[0];
-                resJoins[0] = (Joins)obj[1];
-            } else {
-                synchronized(relationFieldUnionCache) {
-                    obj = relationFieldUnionCache.get(selKey);
-                    if (obj != null) {
-                        union = (Union) obj[0];
-                        resJoins[0] = (Joins) obj[1];
-                    } else {
-                        // select related mapping columns; joining from the 
-                        // related type back to our fk table if not an inverse 
-                        // mapping (in which case we can just make sure the 
-                        // inverse cols == our pk values)
-                        union = newUnion(sm, store, fetch, rels, subs, 
-                                resJoins);
-                        found = false;                
-                    }
-                    sel = ((LogicalUnion.UnionSelect)union.getSelects()[0]).
-                        getDelegate();
-                    SQLBuffer buf = sel.getSQL();
-                    if (buf == null) {
-                    	((SelectImpl)sel).setSQL(store, fetch);
-                        found = false;
-                    }
-
-                    // only cache the union when elems length is 1 for now
-                    if (!found && rels.length == 1) {
-                        Object[] obj1 = new Object[2];
-                        obj1[0] = union;
-                        obj1[1] = resJoins[0];
-                        ((JDBCStoreManager)store).addToSqlCache(
-                            relationFieldUnionCache, selKey, obj1);
-                    }
-                }
-            }
-            Log log = store.getConfiguration().
-                getLog(JDBCConfiguration.LOG_JDBC);
-            if (log.isTraceEnabled()){
-                if (found) 
-                    log.trace(_loc.get("cache-hit", field, this.getClass()));                        
-                else
-                    log.trace(_loc.get("cache-missed", field, this.getClass()));
-            }
-
-            parmList = new ArrayList();
-            ClassMapping mapping = field.getDefiningMapping();
-            Object oid = sm.getObjectId();
-            Column[] cols = mapping.getPrimaryKeyColumns();
-            if (sel == null)
-                sel = ((LogicalUnion.UnionSelect)union.getSelects()[0]).
-                getDelegate();
-
-            sel.wherePrimaryKey(mapping, cols, cols, oid, store, 
-            	null, null, parmList);
-        }
-        
-        Result res = union.execute(store, fetch, parmList);
-        try {
-            Object val = null;
-            if (res.next())
-                val = res.load(rels[res.indexOf()], store, fetch,
-                    resJoins[res.indexOf()]);
-            sm.storeObject(field.getIndex(), val);
-        } finally {
-            res.close();
-        }
-    }
-    
-    protected Union newUnion(final OpenJPAStateManager sm, 
-        final JDBCStore store, final JDBCFetchConfiguration fetch, 
-        final ClassMapping[] rels, final int subs, 
-        final Joins[] resJoins) {
         Union union = store.getSQLFactory().newUnion(rels.length);
         union.setExpectedResultCount(1, false);
         if (fetch.getSubclassFetchMode(field.getTypeMapping())
@@ -717,20 +624,29 @@
             union.abortUnion();
         union.select(new Union.Selector() {
             public void select(Select sel, int idx) {
-                if (field.getJoinDirection() == field.JOIN_INVERSE)
-                    sel.whereForeignKey(field.getForeignKey(rels[idx]),
-                        sm.getObjectId(), field.getDefiningMapping(), store);
+                if (field.getJoinDirection() == field.JOIN_INVERSE) {
+                    sel.whereForeignKey(field.getForeignKey(rels[idx]), sm.getObjectId(), field.getDefiningMapping(),
+                        store);
+                }
                 else {
-                    resJoins[idx] = sel.newJoins().joinRelation(field.getName(),
-                        field.getForeignKey(rels[idx]), rels[idx],
-                        field.getSelectSubclasses(), false, false);
+                    resJoins[idx] =
+                        sel.newJoins().joinRelation(field.getName(), field.getForeignKey(rels[idx]), rels[idx],
+                            field.getSelectSubclasses(), false, false);
                     field.wherePrimaryKey(sel, sm, store);
                 }
-                sel.select(rels[idx], subs, store, fetch, fetch.EAGER_JOIN, 
-                    resJoins[idx]);
+                sel.select(rels[idx], subs, store, fetch, fetch.EAGER_JOIN, resJoins[idx]);
             }
         });
-        return union;
+        Result res = union.execute(store, fetch);
+        try {
+            Object val = null;
+            if (res.next()) {
+                val = res.load(rels[res.indexOf()], store, fetch, resJoins[res.indexOf()]);
+            }
+            sm.storeObject(field.getIndex(), val);
+        } finally {
+            res.close();
+        }
     }
 
     public Object toDataStoreValue(Object val, JDBCStore store) {

Modified: openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java?rev=909125&r1=909124&r2=909125&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java (original)
+++ openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java Thu Feb 11 19:57:07 2010
@@ -192,6 +192,15 @@
                 return false;
         return true;
     }
+    
+    public boolean hasMultipleSelects() {
+        for (UnionSelect sel : sels) {
+            if (sel.hasMultipleSelects()) {
+                return true;
+            }
+        }
+        return false;
+    }
 
     public int getCount(JDBCStore store)
         throws SQLException {
@@ -202,32 +211,21 @@
     }
 
     public Result execute(JDBCStore store, JDBCFetchConfiguration fetch)
-            throws SQLException {
-        return execute(store, fetch, null);
-    }    
-
-    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
-        int lockLevel)
-        throws SQLException {
-        return execute(store, fetch, lockLevel, null);
-    }
-    
-    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch, 
-        List params)
         throws SQLException {
-        if (fetch == null)
+        if (fetch == null) {
             fetch = store.getFetchConfiguration();
-        return execute(store, fetch, fetch.getReadLockLevel(), params);
+        }
+        return execute(store, fetch, fetch.getReadLockLevel());
     }
 
     public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
-        int lockLevel, List params)
+        int lockLevel)
         throws SQLException {
         if (fetch == null)
             fetch = store.getFetchConfiguration();
 
         if (sels.length == 1) {
-            Result res = sels[0].execute(store, fetch, lockLevel, params);
+            Result res = sels[0].execute(store, fetch, lockLevel);
             ((AbstractResult) res).setBaseMapping(mappings[0]);
             return res;
         }
@@ -236,7 +234,7 @@
             AbstractResult res;
             for (int i = 0; i < sels.length; i++) {
                 res = (AbstractResult) sels[i].execute(store, fetch,
-                    lockLevel, params);
+                    lockLevel);
                 res.setBaseMapping(mappings[i]);
                 res.setIndexOf(i);
 
@@ -268,7 +266,7 @@
             List l;
             for (int i = 0; i < res.length; i++) {
                 res[i] = (AbstractResult) sels[i].execute(store, fetch,
-                    lockLevel, params);
+                    lockLevel);
                 res[i].setBaseMapping(mappings[i]);
                 res[i].setIndexOf(i);
 
@@ -402,24 +400,16 @@
         public boolean supportsLocking() {
             return sel.supportsLocking();
         }
+        
+        public boolean hasMultipleSelects() {
+            return sel.hasMultipleSelects();
+        }
 
         public int getCount(JDBCStore store)
             throws SQLException {
             return sel.getCount(store);
         }
 
-        public Result execute(JDBCStore store, JDBCFetchConfiguration fetch, 
-            List params)
-            throws SQLException {
-            return sel.execute(store, fetch, params);
-        }
-
-        public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
-            int lockLevel, List params)
-            throws SQLException {
-            return sel.execute(store, fetch, lockLevel, params);
-        }
-
         public Result execute(JDBCStore store, JDBCFetchConfiguration fetch)
             throws SQLException {
             return sel.execute(store, fetch);

Modified: openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java?rev=909125&r1=909124&r2=909125&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java (original)
+++ openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java Thu Feb 11 19:57:07 2010
@@ -134,20 +134,12 @@
      * Execute this select in the context of the given store manager.
      */
     public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
-        List params) 
+        int lockLevel) 
         throws SQLException;
-
     /**
      * Execute this select in the context of the given store manager.
+     * Affirm if this receiver requires more than one selects to fetch its
+     * data. 
      */
-    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
-        int lockLevel, List params)
-        throws SQLException;
-
-    /**
-     * Execute this select in the context of the given store manager.
-     */
-    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
-        int lockLevel)
-        throws SQLException;
+    public boolean hasMultipleSelects();
 }

Modified: openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java?rev=909125&r1=909124&r2=909125&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java (original)
+++ openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java Thu Feb 11 19:57:07 2010
@@ -294,6 +294,20 @@
     public boolean supportsLocking() {
         return _dict.supportsLocking(this);
     }
+    
+    public boolean hasMultipleSelects() {
+        if (_eager == null) {
+            return false;
+        }
+        Map.Entry entry;
+        for (Iterator itr = _eager.entrySet().iterator(); itr.hasNext();) {
+            entry = (Map.Entry) itr.next();
+            if (entry.getValue() != this) {
+                return true;
+            }
+        }
+        return false;
+    }
 
     public int getCount(JDBCStore store)
         throws SQLException {
@@ -306,7 +320,7 @@
             stmnt = prepareStatement(conn, sql, null, 
                 ResultSet.TYPE_FORWARD_ONLY, 
                 ResultSet.CONCUR_READ_ONLY, false);
-            rs = executeQuery(conn, stmnt, sql, false, store, null);
+            rs = executeQuery(conn, stmnt, sql, false, store);
             return getCount(rs);
         } finally {
             if (rs != null)
@@ -318,31 +332,21 @@
         }
     }
 
-    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch, 
-        List parms) throws SQLException {
+    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch)
+        throws SQLException {
         if (fetch == null)
             fetch = store.getFetchConfiguration();
         return execute(store.getContext(), store, fetch,
-            fetch.getReadLockLevel(), parms);
-    }
-
-    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch) 
-        throws SQLException {
-        return execute(store, fetch, null);
-     }
-
-    public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
-        int lockLevel, List parms)
-        throws SQLException {
-            if (fetch == null)
-                fetch = store.getFetchConfiguration();
-            return execute(store.getContext(), store, fetch, lockLevel, parms);
+            fetch.getReadLockLevel());
     }
 
     public Result execute(JDBCStore store, JDBCFetchConfiguration fetch,
         int lockLevel)
         throws SQLException {
-        return execute(store, fetch, lockLevel, null);
+        if (fetch == null) {
+            fetch = store.getFetchConfiguration();
+        }
+        return execute(store.getContext(), store, fetch, lockLevel);
     }
 
     /**
@@ -350,7 +354,7 @@
      * context is passed in separately for profiling purposes.
      */
     protected Result execute(StoreContext ctx, JDBCStore store, 
-        JDBCFetchConfiguration fetch, int lockLevel, List params)
+        JDBCFetchConfiguration fetch, int lockLevel)
         throws SQLException {
         boolean forUpdate = isForUpdate(store, lockLevel);
         
@@ -373,15 +377,13 @@
         ResultSet rs = null;
         try {
             if (isLRS) 
-                stmnt = prepareStatement(conn, _sql, fetch, rsType, -1, true, 
-                        params); 
+                stmnt = prepareStatement(conn, _sql, fetch, rsType, -1, true); 
             else
-                stmnt = prepareStatement(conn, _sql, null, rsType, -1, false, 
-                        params);
+                stmnt = prepareStatement(conn, _sql, null, rsType, -1, false);
             
             setTimeout(stmnt, forUpdate, fetch);
             
-            rs = executeQuery(conn, stmnt, _sql, isLRS, store, params);
+            rs = executeQuery(conn, stmnt, _sql, isLRS, store);
         } catch (SQLException se) {
             // clean up statement
             if (stmnt != null)
@@ -390,8 +392,7 @@
             throw se;
         }
 
-        return getEagerResult(conn, stmnt, rs, store, fetch, forUpdate, 
-            _sql.getSQL(), params);
+        return getEagerResult(conn, stmnt, rs, store, fetch, forUpdate, _sql);
     }
     
     private boolean isForUpdate(JDBCStore store, int lockLevel) {
@@ -409,7 +410,7 @@
      * to the given result.
      */
     private static void addEagerResults(SelectResult res, SelectImpl sel,
-        JDBCStore store, JDBCFetchConfiguration fetch, List params)
+        JDBCStore store, JDBCFetchConfiguration fetch)
         throws SQLException {
         if (sel._eager == null)
             return;
@@ -428,7 +429,7 @@
                 eres = res;
             else
                 eres = ((SelectExecutor) entry.getValue()).execute(store,
-                    fetch, params);
+                    fetch);
 
             eager = res.getEagerMap(false);
             if (eager == null) {
@@ -447,22 +448,10 @@
     protected PreparedStatement prepareStatement(Connection conn, 
         SQLBuffer sql, JDBCFetchConfiguration fetch, int rsType, 
         int rsConcur, boolean isLRS) throws SQLException {
-        // add comments why we pass in null as the last parameter
-        return prepareStatement(conn, sql, fetch, rsType, rsConcur, isLRS, 
-                null);
-    }
-
-    /**
-     * This method is to provide override for non-JDBC or JDBC-like 
-     * implementation of preparing statement.
-     */
-    protected PreparedStatement prepareStatement(Connection conn, 
-        SQLBuffer sql, JDBCFetchConfiguration fetch, int rsType, 
-        int rsConcur, boolean isLRS, List params) throws SQLException {
         if (fetch == null)
-            return sql.prepareStatement(conn, rsType, rsConcur, params);
+            return sql.prepareStatement(conn, rsType, rsConcur);
         else
-            return sql.prepareStatement(conn, fetch, rsType, -1, params);
+            return sql.prepareStatement(conn, fetch, rsType, -1);
     }
     
     /**
@@ -504,7 +493,7 @@
      * implementation of executing query.
      */
     protected ResultSet executeQuery(Connection conn, PreparedStatement stmnt, 
-        SQLBuffer sql, boolean isLRS, JDBCStore store, List params) 
+        SQLBuffer sql, boolean isLRS, JDBCStore store) 
         throws SQLException {
         return stmnt.executeQuery();
     }
@@ -524,15 +513,14 @@
      */
     protected Result getEagerResult(Connection conn, 
         PreparedStatement stmnt, ResultSet rs, JDBCStore store, 
-        JDBCFetchConfiguration fetch, boolean forUpdate, String sqlStr,
-        List params) 
+        JDBCFetchConfiguration fetch, boolean forUpdate, SQLBuffer sql) 
         throws SQLException {
         SelectResult res = new SelectResult(conn, stmnt, rs, _dict);
         res.setSelect(this);
         res.setStore(store);
         res.setLocking(forUpdate);
         try {
-            addEagerResults(res, this, store, fetch, params);
+            addEagerResults(res, this, store, fetch);
         } catch (SQLException se) {
             res.close();
             throw se;
@@ -1471,10 +1459,11 @@
                 val = pks[mapping.getField(join.getFieldIndex()).
                     getPrimaryKeyIndex()];
                 val = join.getJoinValue(val, toCols[i], store);
-                if (parmList != null)
+                
+                if (parmList != null) {
                 	parmList.add(val);
+                }
             }
-            
             if (collectParmValueOnly) 
             	continue;