openjpa-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ppod...@apache.org
Subject svn commit: r686438 - in /openjpa/branches/1.2.x: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main...
Date Sat, 16 Aug 2008 02:30:37 GMT
Author: ppoddar
Date: Fri Aug 15 19:30:37 2008
New Revision: 686438

URL: http://svn.apache.org/viewvc?rev=686438&view=rev
Log:
Merge 686325, 686349, 686419

Added:
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java
Modified:
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
    openjpa/branches/1.2.x/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
    openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
    openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
    openjpa/branches/1.2.x/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestException.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity2.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestDupNamedQuery.java
    openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/PersistenceTestCase.java
    openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
    openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java
    openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
    openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java
    openjpa/branches/1.2.x/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties
    openjpa/branches/1.2.x/openjpa-slice/src/test/java/org/apache/openjpa/slice/TestQuery.java

Modified: openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java (original)
+++ openjpa/branches/1.2.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java Fri Aug 15 19:30:37 2008
@@ -28,8 +28,10 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.openjpa.jdbc.meta.ClassMapping;
@@ -118,13 +120,17 @@
             }
         }
 
+        int distinctParams = countDistinct(paramOrder);
+        if (params.size() < distinctParams)
+        	throw new UserException(_loc.get("sqlquery-fewer-params", 
+        		new Object[] {sql, distinctParams, params.size(), params}));
         // now go through the paramOrder list and re-order the params array
         List translated = new ArrayList();
         for (Iterator i = paramOrder.iterator(); i.hasNext();) {
             int index = ((Number) i.next()).intValue() - 1;
             if (index >= params.size())
                 throw new UserException(_loc.get("sqlquery-missing-params",
-                    sql, String.valueOf(index), params));
+                    sql, String.valueOf(index+1), params));
             translated.add(params.get(index));
         }
 
@@ -133,6 +139,18 @@
         params.addAll(translated);
         return buf.toString();
     }
+    
+    static int countDistinct(List list) {
+    	if (list == null || list.isEmpty())
+    		return 0;
+    	int distinct = 0;
+    	Set set = new HashSet();
+    	for (Object o : list) {
+    		if (set.add(o))
+    			distinct++;
+    	}
+    	return distinct;
+    }
 
     public boolean supportsParameterDeclarations() {
         return false;

Modified: openjpa/branches/1.2.x/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties (original)
+++ openjpa/branches/1.2.x/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties Fri Aug 15 19:30:37 2008
@@ -22,6 +22,8 @@
 	unjoined subclasses: {0}
 sqlquery-missing-params: SQL query "{0}" declares a parameter index "{1}" for \
 	which no value was given.  The given parameters were: {2}
+sqlquery-fewer-params: SQL query "{0}" declares {1} distinct parameter(s), \
+	but only {2} parameters are given. Given parameter values are "{3}".  
 no-sql: You have not specified a SQL filter to execute in your SQL query.
 del-ins-cycle: An unresolvable constraint cycle was detected.  This typically \
 	means that you are persisting a new object with the same primary key value \

Modified: openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java (original)
+++ openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java Fri Aug 15 19:30:37 2008
@@ -908,7 +908,11 @@
             else if (key instanceof Number) {
                 if (base == -1)
                     base = positionalParameterBase(params.keySet());
-                arr[((Number) key).intValue() - base] = entry.getValue();
+                int arrayIndex = ((Number) key).intValue() - base;
+                if (arrayIndex >= arr.length)
+                	throw new UserException(_loc.get("gap-query-param", 
+                		new Object[]{_query, key, params.size(), params}));
+                arr[arrayIndex] = entry.getValue();
             } else
                 throw new UserException(_loc.get("bad-param-name", key));
         }

Modified: openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java (original)
+++ openjpa/branches/1.2.x/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java Fri Aug 15 19:30:37 2008
@@ -1618,10 +1618,15 @@
         ClassLoader envLoader) {
         if (name == null)
             return null;
-
+        QueryMetaData qm = null;
+        if (cls == null) {
+        	qm = searchQueryMetaDataByName(name);
+        	if (qm != null)
+        		return qm;
+        }
         // check cache
         Object key = getQueryKey(cls, name);
-        QueryMetaData qm = (QueryMetaData) _queries.get(key);
+        qm = (QueryMetaData) _queries.get(key);
         if (qm != null)
             return qm;
 
@@ -1694,6 +1699,18 @@
             return false;
         return _queries.remove(getQueryKey(cls, name)) != null;
     }
+    
+    /**
+     * Searches all cached query metadata by name. 
+     */
+    public QueryMetaData searchQueryMetaDataByName(String name) {
+    	for (Object key : _queries.keySet()) {
+    		if (key instanceof QueryKey)
+    			if (StringUtils.equals(((QueryKey)key).name, name))
+    				return (QueryMetaData)_queries.get(key);
+    	}
+    	return null;
+    }
 
     /**
      * Return a unique key for a given QueryMetaData.

Modified: openjpa/branches/1.2.x/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties (original)
+++ openjpa/branches/1.2.x/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties Fri Aug 15 19:30:37 2008
@@ -400,3 +400,6 @@
     an active connection to the database.
 no-interface-metadata: No metadata was found for managed interface {0}.
 fetch-configuration-stack-empty: Fetch configuration stack is empty.
+gap-query-param: Parameter {1} for query "{0}" exceeds the number of {2} \
+	bound parameters with following values "{3}". This can happen if you have \
+	decalred but missed to bind values for one or more parameters.

Modified: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestException.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestException.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestException.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestException.java Fri Aug 15 19:30:37 2008
@@ -26,6 +26,7 @@
 import javax.persistence.EntityManager;
 import javax.persistence.EntityNotFoundException;
 import javax.persistence.OptimisticLockException;
+import javax.persistence.Query;
 import javax.persistence.TransactionRequiredException;
 
 import org.apache.openjpa.jdbc.sql.DBDictionary;
@@ -158,12 +159,38 @@
 	}
 	
 	/**
+	 * Invalid query throws IllegalArgumentException on construction 
+	 * as per JPA spec.
+	 */
+	public void testIllegalArgumennExceptionOnInvalidQuery() {
+	    EntityManager em = emf.createEntityManager();
+	    try {
+	      em.createQuery("This is not a valid JPQL query");
+          fail("Did not throw IllegalArgumentException for invalid query.");
+	    } catch (Throwable t) {
+		   assertException(t, IllegalArgumentException.class);
+	    }
+	}
+	
+	/**
+	 * Invalid named query fails as per spec on factory based construction. 
+	 */
+     public void testIllegalArgumennExceptionOnInvalidNamedQuery() {
+         EntityManager em = emf.createEntityManager();
+         try {
+             Query query = em.createNamedQuery("This is invalid Named query");
+         } catch (Throwable t) {
+             assertException(t, IllegalArgumentException.class);
+         }
+      }
+	
+	/**
 	 * Asserts that the given expected type of the exception is equal to or a
 	 * subclass of the given throwable or any of its nested exception.
 	 * Otherwise fails assertion and prints the given throwable and its nested
 	 * exception on the console. 
 	 */
-	void assertException(Throwable t, Class expectedType) {
+	public void assertException(Throwable t, Class expectedType) {
 		if (!isExpectedException(t, expectedType)) {
 			t.printStackTrace();
 			print(t, 0);

Added: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java?rev=686438&view=auto
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java (added)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/TestQueryParameterBinding.java Fri Aug 15 19:30:37 2008
@@ -0,0 +1,292 @@
+/*
+ * 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.jdbc.query;
+
+import javax.persistence.EntityManager;
+import javax.persistence.Query;
+
+import org.apache.openjpa.persistence.ArgumentException;
+import org.apache.openjpa.persistence.jdbc.query.domain.Binder;
+import org.apache.openjpa.persistence.test.SingleEMFTestCase;
+
+/**
+ * Tests validation of positional and named parameter binding for JPQL queries.
+ *  
+ *  
+ * @author Pinaki Poddar
+ *
+ */
+public class TestQueryParameterBinding extends SingleEMFTestCase {
+	private static String JPQL = "SELECT p FROM Binder p ";
+	
+	private static int INT_VALUE    = 1;
+	private static String STR_VALUE = "2";
+	private static double DBL_VALUE = 3.0;
+	
+	private EntityManager em;
+	@Override
+	public void setUp() throws Exception {
+		super.setUp(CLEAR_TABLES, Binder.class);
+		
+		em = emf.createEntityManager();
+		em.getTransaction().begin();
+		em.persist(new Binder(INT_VALUE, STR_VALUE, DBL_VALUE));
+		em.getTransaction().commit();
+	}
+	
+	public void testPositionalParameterWithPositionalBindingSucceeds() {
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL);
+		q.setParameter(1, INT_VALUE);
+		q.setParameter(2, STR_VALUE);
+		q.setParameter(3, DBL_VALUE);
+		
+		assertEquals(1, q.getResultList().size());
+	}
+	
+	public void testPositionalParameterWithNamedBindingFails() {
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL);
+		q.setParameter("p1", INT_VALUE);
+		q.setParameter("p2", STR_VALUE);
+		q.setParameter("p3", DBL_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testPositionalParameterWithInsufficientValuesFails() {
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL);
+		q.setParameter(1, INT_VALUE);
+		q.setParameter(2, STR_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testPositionalParameterWithExtraValuesFails() {
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL);
+		q.setParameter(1, INT_VALUE);
+		q.setParameter(2, STR_VALUE);
+		q.setParameter(3, DBL_VALUE);
+		q.setParameter(4, 4);
+		
+		fail(q);
+	}
+
+	public void testPositionalParameterWithRepeatedValuesSucceeds() {
+		String jPQL_POSITIONAL_REPEATED_PARAM  = 
+			JPQL + "WHERE p.p1=?1 OR p.p1=?1 AND p.p3=?2";
+		Query q = em.createQuery(jPQL_POSITIONAL_REPEATED_PARAM);
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  DBL_VALUE);
+		
+		assertEquals(1,q.getResultList().size());
+	}
+	
+	public void testPositionalParameterWithGapSucceeds() {
+		String JPQL_POSITIONAL_GAP_IN_PARAM  = 
+			JPQL + "WHERE p.p1=?1 AND p.p2=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL_GAP_IN_PARAM);
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(3,  STR_VALUE);
+		
+		assertEquals(1,q.getResultList().size());
+	}
+	
+	public void testPositionalParameterWithGapFails() {
+		String JPQL_POSITIONAL_GAP_IN_PARAM  = 
+			JPQL + "WHERE p.p1=?1 AND p.p3=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL_GAP_IN_PARAM);
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  STR_VALUE);
+		q.setParameter(3,  DBL_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testNamedParameterWithNamedBindingSucceeds() {
+		String JPQL_NAMED  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
+		Query q = em.createQuery(JPQL_NAMED);
+		q.setParameter("p1", INT_VALUE);
+		q.setParameter("p2", STR_VALUE);
+		q.setParameter("p3", DBL_VALUE);
+		
+		assertEquals(1, q.getResultList().size());
+	}
+	
+	public void testNamedParameterWithPositionalBindingFails() {
+		String JPQL_NAMED  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
+		Query q = em.createQuery(JPQL_NAMED);
+		q.setParameter(1, INT_VALUE);
+		q.setParameter(2, STR_VALUE);
+		q.setParameter(3, DBL_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testNamedParameterWithInsufficientValuesFails() {
+		String JPQL_NAMED  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
+		Query q = em.createQuery(JPQL_NAMED);
+		q.setParameter("p1", INT_VALUE);
+		q.setParameter("p2", STR_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testNamedParameterWithExtraValuesFails() {
+		String JPQL_NAMED  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
+		Query q = em.createQuery(JPQL_NAMED);
+		q.setParameter("p1", INT_VALUE);
+		q.setParameter("p2", STR_VALUE);
+		q.setParameter("p3", DBL_VALUE);
+		q.setParameter("p4", 4);
+		
+		fail(q);
+	}
+
+	public void testNamedParameterWithRepeatedValuesSucceeds() {
+		String jPQL_NAMED_REPEATED_PARAM  = 
+			JPQL + "WHERE p.p1=:p1 OR p.p1=:p1 AND p.p3=:p2";
+		Query q = em.createQuery(jPQL_NAMED_REPEATED_PARAM);
+		q.setParameter("p1",  INT_VALUE);
+		q.setParameter("p2",  DBL_VALUE);
+		
+		assertEquals(1,q.getResultList().size());
+	}
+	
+	public void testNamedParameterWithGapSucceeds() {
+		String JPQL_NAMED_GAP_IN_PARAM  = 
+			JPQL + "WHERE p.p1=:p1 AND p.p2=:p3";
+		Query q = em.createQuery(JPQL_NAMED_GAP_IN_PARAM);
+		q.setParameter("p1",  INT_VALUE);
+		q.setParameter("p3",  STR_VALUE);
+		
+		assertEquals(1,q.getResultList().size());
+	}
+	
+	public void testNamedParameterWithGapFails() {
+		String JPQL_NAMED_GAP_IN_PARAM  = 
+			JPQL + "WHERE p.p1=:p1 AND p.p3=:p3";
+		Query q = em.createQuery(JPQL_NAMED_GAP_IN_PARAM);
+		q.setParameter("p1",  INT_VALUE);
+		q.setParameter("p2",  STR_VALUE);
+		q.setParameter("p3",  DBL_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testNamedParameterWithWrongType() {
+		String JPQL_NAMED  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
+		Query q = em.createQuery(JPQL_NAMED);
+		q.setParameter("p1",  INT_VALUE);
+		q.setParameter("p2",  DBL_VALUE);
+		q.setParameter("p3",  STR_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testPositionalParameterWithWrongType() {
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL);
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  DBL_VALUE);
+		q.setParameter(3,  STR_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testNamedParameterWithNullValue() {
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=:p1 AND p.p2=:p2 AND p.p3=:p3";
+		Query q = em.createQuery(JPQL_POSITIONAL);
+		q.setParameter("p1",  INT_VALUE);
+		q.setParameter("p2",  null);
+		q.setParameter("p3",  null);
+		
+		fail(q);
+	}
+	
+	public void testPositionalParameterWithNullValue() {
+		String JPQL_POSITIONAL  = JPQL + "WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3";
+		Query q = em.createQuery(JPQL_POSITIONAL);
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  null);
+		q.setParameter(3,  null);
+		
+		fail(q);
+	}
+	
+	public void testPositionalParameterWithSingleResult() {
+		Query q = em.createNamedQuery("JPQL_POSITIONAL");
+		// "SELECT p FROM Binder p WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  null);
+		q.setParameter(3,  null);
+		
+		fail(q, true);
+	}
+	
+	public void testPositionalParameterWithNativeQuery() {
+		Query q = em.createNamedQuery("SQL_POSITIONAL");
+		// "SELECT p.id FROM Binder WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  STR_VALUE);
+		q.setParameter(3,  DBL_VALUE);
+		
+		assertEquals(1,q.getResultList().size());
+	}
+	
+	public void testPositionalParameterWithNativeQueryFails() {
+		Query q = em.createNamedQuery("SQL_POSITIONAL");
+		// "SELECT p.id FROM Binder WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(2,  STR_VALUE);
+		
+		fail(q);
+	}
+	
+	public void testPositionalParameterWithNativeQueryFailsWithGap() {
+		Query q = em.createNamedQuery("SQL_POSITIONAL");
+		// "SELECT p.id FROM Binder WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3"
+		q.setParameter(1,  INT_VALUE);
+		q.setParameter(3,  DBL_VALUE);
+		
+		fail(q);
+	}
+	
+	
+	void fail(Query q) {
+		fail(q, false);
+	}
+	
+	void fail(Query q, boolean single) {
+		try {
+			if (single) 
+				q.getSingleResult();
+			else 
+				q.getResultList();
+			fail("Expeceted " + ArgumentException.class.getName());
+		} catch (IllegalArgumentException ex) {
+		// good
+			System.err.println("*** ERROR " + getName());
+			System.err.println("*** ERROR " + ex.getMessage());
+		}
+	}
+	
+}

Modified: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestNonPrimaryKeyQueryParameters.java Fri Aug 15 19:30:37 2008
@@ -80,7 +80,6 @@
 		EntityManager em = emf.createEntityManager();
 
 		Query query = em.createQuery("SELECT d from Department d");
-		query.setParameter(1, DEPT_NAME);
 		Department dept = (Department) query.getSingleResult();
 
 		assertEquals(FULLTIME_EMPLOYEE_COUNT, dept.getFullTimeEmployees()

Added: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java?rev=686438&view=auto
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java (added)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/domain/Binder.java Fri Aug 15 19:30:37 2008
@@ -0,0 +1,51 @@
+/*
+ * 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.jdbc.query.domain;
+
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.NamedNativeQuery;
+import javax.persistence.NamedQuery;
+
+@Entity
+@NamedQuery(name="JPQL_POSITIONAL", 
+		query="SELECT p FROM Binder p WHERE p.p1=?1 AND p.p2=?2 AND p.p3=?3")
+@NamedNativeQuery(name="SQL_POSITIONAL",
+		query="SELECT id, p1 FROM Binder WHERE p1=?1 AND p2=?2 AND p3=?3")
+public class Binder {
+	@Id
+	@GeneratedValue
+	private long id;
+	
+	private int p1;
+	private String p2;
+	private double p3;
+	
+	protected Binder() {
+		this(-1, "-1" , -1.0);
+	}
+	
+	public Binder(int i1, String i2, double i3) {
+		p1 = i1;
+		p2 = i2;
+		p3 = i3;
+	}
+	
+}

Modified: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity.java Fri Aug 15 19:30:37 2008
@@ -33,11 +33,11 @@
 import javax.persistence.SqlResultSetMapping;
 import javax.persistence.Table;
 
-@NamedQuery(name="FindXTwo", query="select s from simple s where s.name = :fname")
+@NamedQuery(name="FindXTwo", query="select s from simple s where s.name = ?1")
 
 @NamedQueries( {
-    @NamedQuery(name="FindOne", query="select s from simple s where s.name = :fname"),
-    @NamedQuery(name="FindOne", query="select s from simple s where s.name = :fname"),
+    @NamedQuery(name="FindOne", query="select s from simple s where s.name = ?1"),
+    @NamedQuery(name="FindOne", query="select s from simple s where s.name = ?1"),
     @NamedQuery(name="FindAll", query="select s from simple s")
 })
 

Modified: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity2.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity2.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity2.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/SimpleEntity2.java Fri Aug 15 19:30:37 2008
@@ -27,11 +27,11 @@
 import javax.persistence.NamedQuery;
 import javax.persistence.Table;
 
-@NamedQuery(name="FindXTwo", query="select s from simple2 s where s.name = :fname")
+@NamedQuery(name="FindXTwo", query="select s from simple2 s where s.name = ?1")
 
 @NamedQueries( {
-    @NamedQuery(name="FindOne", query="select s from simple2 s where s.name = :fname"),
-    @NamedQuery(name="Find2One", query="select s from simple2 s where s.name = :fname"),
+    @NamedQuery(name="FindOne", query="select s from simple2 s where s.name = ?1"),
+    @NamedQuery(name="Find2One", query="select s from simple2 s where s.name = ?1"),
     @NamedQuery(name="Find2All", query="select s from simple2 s")
 })
 

Modified: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestDupNamedQuery.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestDupNamedQuery.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestDupNamedQuery.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestDupNamedQuery.java Fri Aug 15 19:30:37 2008
@@ -60,8 +60,8 @@
         assertNotNull(list);
         assertEquals(list.size(), 1);
         Object o = list.get(0);
-        assertSame(o.getClass(), simple2 ? SimpleEntity2.class
-            : SimpleEntity.class);
+        assertTrue(simple2 ? o instanceof SimpleEntity2 
+        		: o instanceof SimpleEntity);
         assertEquals(simple2 ? ((SimpleEntity2) o).getValue()
             : ((SimpleEntity) o).getValue(), ValueOne);
 
@@ -71,8 +71,8 @@
             assertEquals(list.size(), 2);
             for (Iterator resultIter = list.iterator(); resultIter.hasNext();) {
                 o = resultIter.next();
-                assertSame(o.getClass(), simple2 ? SimpleEntity2.class
-                    : SimpleEntity.class);
+                assertTrue(simple2 ? o instanceof SimpleEntity2 
+                		: o instanceof SimpleEntity);
                 String n = null;
                 String v = null;
                 if (simple2) {

Modified: openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/PersistenceTestCase.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/PersistenceTestCase.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/PersistenceTestCase.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/PersistenceTestCase.java Fri Aug 15 19:30:37 2008
@@ -18,7 +18,13 @@
  */
 package org.apache.openjpa.persistence.test;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.lang.reflect.Modifier;
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -296,4 +302,99 @@
         else if (o1.equals(o2))
             fail("expected args to be different; compared equal.");
     }
+
+    /**
+     * Round-trip a serializable object to bytes.
+     */
+    public static Object roundtrip(Object o) 
+        throws ClassNotFoundException, IOException {
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+        ObjectOutputStream out = new ObjectOutputStream(bytes);
+        out.writeObject(o);
+        out.flush();
+        ObjectInputStream in = new ObjectInputStream(
+            new ByteArrayInputStream(bytes.toByteArray()));
+        return in.readObject();
+    }
+    
+    // ================================================ 
+    // Utility methods for exception handling
+    // ================================================
+    /**
+	 * Asserts that the given targetType is assignable from given actual 
+	 * Throwable.
+	 */
+    protected void assertException(final Throwable actual, Class targetType) {
+		assertException(actual, targetType, null);
+	}
+	
+	/**
+	 * Asserts that the given targetType is assignable from given actual 
+	 * Throwable. Asserts that the nestedType is nested (possibly recursively) 
+	 * within the given actual Throwable.
+	 * 
+	 * @param actual is the actual throwable to be tested
+	 * @param targetType is expected type or super type of actual. If null, then
+	 * the check is omitted.
+	 * @param nestedTargetType is expected type of exception nested within
+	 * actual. If null this search is omitted. 
+	 * 
+	 */
+    protected void assertException(final Throwable actual, Class targetType,
+			Class nestedTargetType) {
+		assertNotNull(actual);
+		Class actualType = actual.getClass();
+		if (targetType != null && !targetType.isAssignableFrom(actualType)) {
+			actual.printStackTrace();
+			fail(targetType.getName() + " is not assignable from "
+					+ actualType.getName());
+		}
+
+		if (nestedTargetType != null) {
+			Throwable nested = actual.getCause();
+			Class nestedActualType = (nested == null) ? null : nested.getClass();
+			while (nestedActualType != null) {
+				if (nestedTargetType.isAssignableFrom(nestedActualType)) {
+					return;
+				} else {
+					Throwable next = nested.getCause();
+					if (next == null || next == nested)
+						break;
+					nestedActualType = next.getClass();
+					nested     = next;
+				}
+			}
+			actual.printStackTrace();
+			fail("No nested type " + nestedTargetType + " in " + actual);
+		}
+	}
+
+	/**
+	 * Assert that each of given keys are present in the message of the given
+	 * Throwable.
+	 */
+    protected void assertMessage(Throwable actual, String... keys) {
+		if (actual == null || keys == null)
+			return;
+		String message = actual.getMessage();
+		for (String key : keys) {
+			assertTrue(key + " is not in " + message, message.contains(key));
+		}
+	}
+    
+    public void printException(Throwable t) {
+    	printException(t, 2);
+    }
+    
+    public void printException(Throwable t, int tab) {
+		if (t == null) return;
+		for (int i=0; i<tab*4;i++) System.out.print(" ");
+		String sqlState = (t instanceof SQLException) ? 
+			"(SQLState=" + ((SQLException)t).getSQLState() + ":" 
+				+ t.getMessage() + ")" : "";
+		System.out.println(t.getClass().getName() + sqlState);
+		if (t.getCause() == t) 
+			return;
+		printException(t.getCause(), tab+2);
+	}
 }

Modified: openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java Fri Aug 15 19:30:37 2008
@@ -1582,14 +1582,17 @@
             if (_log.isTraceEnabled())
                 _log.trace(_loc.get("parse-query", query.name()));
 
-            meta = getRepository().getCachedQueryMetaData(null, query.name());
+            meta = getRepository().searchQueryMetaDataByName(query.name());
             if (meta != null) {
-                if (_log.isWarnEnabled())
-                    _log.warn(_loc.get("dup-query", query.name(), el));
+            	Class definingType = meta.getDefiningType();
+                if ((definingType == null || definingType != _cls) 
+                  && _log.isWarnEnabled()) {
+                    _log.warn(_loc.get("dup-query", query.name(), el, 
+                    		definingType));
+                }
                 continue;
             }
-
-            meta = getRepository().addQueryMetaData(null, query.name());
+            meta = getRepository().addQueryMetaData(_cls, query.name());
             meta.setQueryString(query.query());
             meta.setLanguage(JPQLParser.LANG_JPQL);
             for (QueryHint hint : query.hints())
@@ -1623,10 +1626,12 @@
             if (_log.isTraceEnabled())
                 _log.trace(_loc.get("parse-native-query", query.name()));
 
-            meta = getRepository().getCachedQueryMetaData(null, query.name());
+            meta = getRepository().searchQueryMetaDataByName(query.name());
             if (meta != null) {
-                if (_log.isWarnEnabled())
-                    _log.warn(_loc.get("dup-query", query.name(), el));
+            	Class defType = meta.getDefiningType();
+                if ((defType != _cls) && _log.isWarnEnabled()) {
+                    _log.warn(_loc.get("dup-query", query.name(), el, defType));
+                }
                 continue;
             }
 

Modified: openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java Fri Aug 15 19:30:37 2008
@@ -87,7 +87,7 @@
     private Map<FetchConfiguration,FetchPlan> _plans =
         new IdentityHashMap<FetchConfiguration,FetchPlan>(1);
 
-    private RuntimeExceptionTranslator ret =
+    private RuntimeExceptionTranslator _ret =
         PersistenceExceptions.getRollbackTranslator(this);
 
     public EntityManagerImpl() {
@@ -104,8 +104,8 @@
 
     private void initialize(EntityManagerFactoryImpl factory, Broker broker) {
         _emf = factory;
-        _broker = new DelegatingBroker(broker, ret);
-        _broker.setImplicitBehavior(this, ret);
+        _broker = new DelegatingBroker(broker, _ret);
+        _broker.setImplicitBehavior(this, _ret);
     }
 
     /**
@@ -867,7 +867,16 @@
 
     public OpenJPAQuery createQuery(String language, String query) {
         assertNotCloseInvoked();
-        return new QueryImpl(this, ret, _broker.newQuery(language, query));
+        try {
+            org.apache.openjpa.kernel.Query q = _broker.newQuery(language, 
+                query);
+            // have to validate JPQL according to spec
+            if (JPQLParser.LANG_JPQL.equals(language))
+                q.compile(); 
+            return new QueryImpl(this, _ret, q);
+        } catch (RuntimeException re) {
+            throw PersistenceExceptions.toPersistenceException(re);
+        }
     }
 
     public OpenJPAQuery createQuery(Query query) {
@@ -875,7 +884,7 @@
             return createQuery((String) null);
         assertNotCloseInvoked();
         org.apache.openjpa.kernel.Query q = ((QueryImpl) query).getDelegate();
-        return new QueryImpl(this, ret, _broker.newQuery(q.getLanguage(),
+        return new QueryImpl(this, _ret, _broker.newQuery(q.getLanguage(),
             q));
     }
 
@@ -891,7 +900,7 @@
             meta.setInto(del);
             del.compile();
 
-            OpenJPAQuery q = new QueryImpl(this, ret, del);
+            OpenJPAQuery q = new QueryImpl(this, _ret, del);
             String[] hints = meta.getHintKeys();
             Object[] values = meta.getHintValues();
             for (int i = 0; i < hints.length; i++)
@@ -917,7 +926,7 @@
         org.apache.openjpa.kernel.Query kernelQuery = _broker.newQuery(
             QueryLanguages.LANG_SQL, query);
         kernelQuery.setResultMapping(null, mappingName);
-        return new QueryImpl(this, ret, kernelQuery);
+        return new QueryImpl(this, _ret, kernelQuery);
     }
 
     /**
@@ -1235,7 +1244,7 @@
     public void readExternal(ObjectInput in)
         throws IOException, ClassNotFoundException {
         try {
-            ret = PersistenceExceptions.getRollbackTranslator(this);
+            _ret = PersistenceExceptions.getRollbackTranslator(this);
 
             // this assumes that serialized Brokers are from something
             // that extends AbstractBrokerFactory.
@@ -1254,7 +1263,7 @@
             initialize(emf, broker);
         } catch (RuntimeException re) {
             try {
-                re = ret.translate(re);
+                re = _ret.translate(re);
             } catch (Exception e) {
                 // ignore
             }
@@ -1276,7 +1285,7 @@
             out.writeObject(baos.toByteArray());
         } catch (RuntimeException re) {
             try {
-                re = ret.translate(re);
+                re = _ret.translate(re);
             } catch (Exception e) {
                 // ignore
             }

Modified: openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java Fri Aug 15 19:30:37 2008
@@ -21,6 +21,7 @@
 import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.Collections;
@@ -30,6 +31,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
+
 import javax.persistence.FlushModeType;
 import javax.persistence.Query;
 import javax.persistence.TemporalType;
@@ -67,18 +70,17 @@
     private transient EntityManagerImpl _em;
     private transient FetchPlan _fetch;
 
-    private Map _named;
-    private List _positional;
+	private Map<String, Object> _named;
+	private Map<Integer, Object> _positional;
+
+	private static Object GAP_FILLER = new Object();
 
     /**
      * Constructor; supply factory exception translator and delegate.
      * 
-     * @param em
-     *            The EntityManager which created this query
-     * @param ret
-     *            Exception translater for this query
-     * @param query
-     *            The underlying "kernel" query.
+     * @param em  The EntityManager which created this query
+     * @param ret Exception translater for this query
+     * @param query The underlying "kernel" query.
      */
     public QueryImpl(EntityManagerImpl em, RuntimeExceptionTranslator ret,
         org.apache.openjpa.kernel.Query query) {
@@ -246,47 +248,133 @@
 
         validateParameters();
 
-        // handle which types of parameters we are using, if any
-        if (_positional != null)
-            return _query.execute(_positional.toArray());
-        if (_named != null)
-            return _query.execute(_named);
-        return _query.execute();
-    }
-
-    /**
-     * Validate that the types of the parameters are correct.
-     */
-    private void validateParameters() {
-        if (_positional != null) {
-            LinkedMap types = _query.getParameterTypes();
-            for (int i = 0,
-                size = Math.min(_positional.size(), types.size());
-                i < size; i++)
-                validateParameter(String.valueOf(i),
-                    (Class) types.getValue(i), _positional.get(i));
-        } else if (_named != null) {
-            Map types = _query.getParameterTypes();
-            for (Iterator i = _named.entrySet().iterator(); i.hasNext();) {
-                Map.Entry entry = (Map.Entry) i.next();
-                String name = (String) entry.getKey();
-                validateParameter(name, (Class) types.get(name),
-                    entry.getValue());
-            }
-        }
-    }
-
-    private void validateParameter(String paramDesc, Class type, Object param) {
-        // null parameters are allowed, so are not validated
-        if (param == null || type == null)
-            return;
+		// handle which types of parameters we are using, if any
+		if (_positional != null)
+			return _query.execute(_positional);
+		if (_named != null)
+			return _query.execute(_named);
+		return _query.execute();
+	}
+	
+	/**
+	 * Validate that the types of the parameters are correct.
+	 * The idea is to catch as many validation error as possible at the facade
+	 * layer itself.
+	 * For native SQL queries, however, parameter validation is bypassed as
+	 * we do not parse SQL.
+	 * 
+	 * The expected parameters are parsed from the query and in a LinkedMap 
+	 *	key   : name of the parameter as declared in query
+	 *  value : expected Class of allowed value
+	 *  
+	 * The bound parameters depends on positional or named parameter style
+	 * 
+	 * TreeMap<Integer, Object> for positional parameters:
+	 *   key   : 1-based Integer index
+	 *   value : bound value. GAP_FILLER if the position is not set. This
+	 *   simplifies validation at the kernel layer
+	 *   
+	 * Map<String, Object> for named parameters:
+	 *   key   : parameter name
+	 *   value : the bound value
+	 *   
+	 *  Validation accounts for 
+	 *    a) gaps in positional parameters
+	 *       SELECT p FROM PObject p WHERE p.a1=?1 AND p.a3=?3
+	 *    
+	 *    b) repeated parameters
+	 *       SELECT p FROM PObject p WHERE p.a1=?1 AND p.a2=?1 AND p.a3=?2
+	 *       
+	 *    c) parameter is bound but not declared
+	 *    
+	 *    d) parameter is declared but not bound
+	 *    
+	 *    e) parameter does not match the value type
+	 *    
+	 *    f) parameter is primitive type but bound to null value
+	 */
+	private void validateParameters() {
+		if (isNative()) {
+			removeGaps(_positional);
+			return;
+		}
+		String query = getQueryString();
+		if (_positional != null) {
+			LinkedMap expected = _query.getParameterTypes();
+			Map<Integer, Object> actual = _positional;
+			for (Object o : expected.keySet()) {
+				String position = (String) o;
+				Class expectedParamType = (Class) expected.get(position);
+				try {
+					Integer.parseInt(position);
+				} catch (NumberFormatException ex) {
+					newValidationException("param-style-mismatch", query,
+							expected.asList(),
+							Arrays.toString(actual.keySet().toArray()));
+				}
+				Object actualValue = actual.get(Integer.parseInt(position));
+				boolean valueUnspecified = (actualValue == GAP_FILLER)
+						|| (actualValue == null && (actual.size() < expected
+								.size()));
+				if (valueUnspecified) 
+					newValidationException("param-missing", position, query,
+							Arrays.toString(actual.keySet().toArray()));
+				
+				if (expectedParamType.isPrimitive() && actualValue == null)
+					newValidationException("param-type-null", 
+							position, query, expectedParamType.getName());
+				if (actualValue != null &&
+				   !Filters.wrap(expectedParamType).isInstance(actualValue)) 
+					newValidationException("param-type-mismatch",
+							position, query, actualValue,
+							actualValue.getClass().getName(),
+							expectedParamType.getName());
+				
+			}
+			for (Integer position : actual.keySet()) {
+				Object actualValue = actual.get(position);
+				Class expectedParamType = (Class) expected.get("" + position);
+				boolean paramExpected = expected.containsKey("" + position);
+				if (actualValue == GAP_FILLER) {
+					if (paramExpected) {
+						newValidationException("param-missing", position, query,
+								Arrays.toString(actual.keySet().toArray()));
+					}
+				} else {
+					if (!paramExpected)
+						newValidationException("param-extra", position, query,
+								expected.asList());
+					if (expectedParamType.isPrimitive() && actualValue == null)
+						newValidationException("param-type-null", 
+								position, query, expectedParamType.getName());
+					if (actualValue != null 
+					 && !Filters.wrap(expectedParamType).isInstance(actualValue)) 
+						newValidationException("param-type-mismatch",
+								position, query, actualValue,
+								actualValue.getClass().getName(),
+								expectedParamType.getName());
+					
+				}
+			}
+		}
+	}
+	
+	Map<Integer, Object> removeGaps(Map<Integer, Object> map) {
+		if (map == null || !map.containsValue(GAP_FILLER))
+			return map;
+		List<Integer> gaps = new ArrayList<Integer>();
+		for (Integer key : map.keySet())
+			if (map.get(key) == GAP_FILLER)
+				gaps.add(key);
+		for (Integer gap : gaps) {
+			map.remove(gap);
+		}
+		return map;
+	}
 
-        // check the parameter against the wrapped type
-        if (!Filters.wrap(type).isInstance(param))
-            throw new ArgumentException(_loc.get("bad-param-type",
-                paramDesc, param.getClass().getName(), type.getName()),
-                null, null, false);
-    }
+	void newValidationException(String msgKey, Object...args) {
+		throw new ArgumentException(_loc.get(msgKey, args), null, null, false);
+	}
 
     public List getResultList() {
         _em.assertNotCloseInvoked();
@@ -324,7 +412,7 @@
         if (_query.getOperation() == QueryOperations.OP_DELETE) {
             // handle which types of parameters we are using, if any
             if (_positional != null)
-                return asInt(_query.deleteAll(_positional.toArray()));
+                return asInt(_query.deleteAll(_positional));
             if (_named != null)
                 return asInt(_query.deleteAll(_named));
             return asInt(_query.deleteAll());
@@ -332,7 +420,7 @@
         if (_query.getOperation() == QueryOperations.OP_UPDATE) {
             // handle which types of parameters we are using, if any
             if (_positional != null)
-                return asInt(_query.updateAll(_positional.toArray()));
+                return asInt(_query.updateAll(_positional));
             if (_named != null)
                 return asInt(_query.updateAll(_named));
             return asInt(_query.updateAll());
@@ -441,40 +529,39 @@
         return setParameter(position, value);
     }
 
-    public OpenJPAQuery setParameter(int position, Object value) {
-        _query.assertOpen();
-        _em.assertNotCloseInvoked();
-        _query.lock();
-        try {
-        	if (isNative() && position < 1) {
-        		throw new IllegalArgumentException(_loc.get("bad-pos-params", 
-        		      position, _query.getQueryString()).toString());
-        	}
-            // not allowed to mix positional and named parameters (EDR2 3.6.4)
-            if (_named != null)
-                throw new InvalidStateException(_loc.get
-                    ("no-pos-named-params-mix", _query.getQueryString()),
-                    null, null, false);
-
-            if (position < 1)
-                throw new InvalidStateException(_loc.get
-                    ("illegal-index", position), null, null, false);
-
-            if (_positional == null)
-                _positional = new ArrayList();
+	public OpenJPAQuery setParameter(int position, Object value) {
+		_query.assertOpen();
+		_em.assertNotCloseInvoked();
+		_query.lock();
+		try {
+			if (isNative() && position < 1) {
+				throw new IllegalArgumentException(_loc.get("bad-pos-params",
+						position, _query.getQueryString()).toString());
+			}
+			// not allowed to mix positional and named parameters (EDR2 3.6.4)
+			if (_named != null)
+				throw new InvalidStateException(_loc.get(
+						"no-pos-named-params-mix", _query.getQueryString()),
+						null, null, false);
+
+			if (position < 1)
+				throw new InvalidStateException(_loc.get("illegal-index",
+						position), null, null, false);
+
+			if (_positional == null)
+				_positional = new TreeMap<Integer, Object>();
+
+			_positional.put(position, value);
+			for (int i = 1; i < position; i++)
+				if (!_positional.containsKey(i))
+					_positional.put(i, GAP_FILLER);
+
+			return this;
+		} finally {
+			_query.unlock();
+		}
+	}
 
-            // make sure it is at least the requested size
-            while (_positional.size() < position)
-                _positional.add(null);
-
-            // note that we add it to position - 1, since setPosition
-            // starts at 1, while List starts at 0
-            _positional.set(position - 1, value);
-            return this;
-        } finally {
-            _query.unlock();
-        }
-    }
 
     public OpenJPAQuery setParameter(String name, Calendar value,
         TemporalType t) {
@@ -518,14 +605,21 @@
         return _positional != null;
     }
 
-    public Object[] getPositionalParameters() {
-        _query.lock();
-        try {
-            return (_positional == null) ? EMPTY_ARRAY : _positional.toArray();
-        } finally {
-            _query.unlock();
-        }
-    }
+	/**
+	 * Gets the array of positional parameter values. A value of
+	 * <code>GAP_FILLER</code> indicates that user has not set the
+	 * corresponding positional parameter. A value of null implies that user has
+	 * set the value as null.
+	 */
+	public Object[] getPositionalParameters() {
+		_query.lock();
+		try {
+			return (_positional == null) ? EMPTY_ARRAY : _positional.values()
+					.toArray();
+		} finally {
+			_query.unlock();
+		}
+	}
 
     public OpenJPAQuery setParameters(Object... params) {
         _query.assertOpen();

Modified: openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java (original)
+++ openjpa/branches/1.2.x/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java Fri Aug 15 19:30:37 2008
@@ -1391,9 +1391,15 @@
         if (log.isTraceEnabled())
             log.trace(_loc.get("parse-query", name));
 
-        QueryMetaData meta = getRepository().getCachedQueryMetaData(null, name);
-        if (meta != null && log.isWarnEnabled())
-            log.warn(_loc.get("override-query", name, currentLocation()));
+        QueryMetaData meta = getRepository().searchQueryMetaDataByName(name);
+        if (meta != null) {
+        	Class defType = meta.getDefiningType();
+            if ((defType != _cls) && log.isWarnEnabled()) {
+            	log.warn(_loc.get("dup-query", name, currentLocation(), defType));
+            }
+            pushElement(meta);
+            return true;
+        }
 
         meta = getRepository().addQueryMetaData(null, name);
         meta.setDefiningType(_cls);

Modified: openjpa/branches/1.2.x/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties (original)
+++ openjpa/branches/1.2.x/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties Fri Aug 15 19:30:37 2008
@@ -34,7 +34,8 @@
 dup-sequence: Found duplicate generator "{0}" in "{1}".  Ignoring.
 override-sequence: Found duplicate generator "{0}" in "{1}".  Overriding \
 	previous definition.
-dup-query: Found duplicate query "{0}" in "{1}".  Ignoring.
+dup-query: Ignoring duplicate query "{0}" in "{1}". A query with the same name \
+	been already declared in "{2}".
 override-query: Found duplicate query "{0}" in "{1}".  Overriding previous \
 	definition.
 no-seq-name: The sequence generator in "{0}" must declare a name.
@@ -148,4 +149,13 @@
 EntityManagerFactory-displayorder: 50
 EntityManagerFactory-expert: true
 EntityManagerFactory-interface: org.apache.openjpa.persistence.EntityManagerFactoryImpl
-
+param-style-mismatch: Query "{0}" is declared with named parameters "{1}" but \
+	actual parameters "{2}" are bound by position.
+param-missing: Parameter "{0}" declared in "{1}" but is missing from the bound \
+	parameters "{2}".
+param-extra: Parameter "{0}" is bound to "{1}" but is missing from the \
+	declared parameters "{2}".
+param-type-mismatch: Parameter "{0}" declared in "{1}" is set to value of \
+	"{2}" of type "{3}", but this parameter is bound to a field of type "{4}".
+param-type-null: Parameter "{0}" declared in "{1}" is set to null, \
+	but this parameter is bound to a field of primitive type "{2}".

Modified: openjpa/branches/1.2.x/openjpa-slice/src/test/java/org/apache/openjpa/slice/TestQuery.java
URL: http://svn.apache.org/viewvc/openjpa/branches/1.2.x/openjpa-slice/src/test/java/org/apache/openjpa/slice/TestQuery.java?rev=686438&r1=686437&r2=686438&view=diff
==============================================================================
--- openjpa/branches/1.2.x/openjpa-slice/src/test/java/org/apache/openjpa/slice/TestQuery.java (original)
+++ openjpa/branches/1.2.x/openjpa-slice/src/test/java/org/apache/openjpa/slice/TestQuery.java Fri Aug 15 19:30:37 2008
@@ -111,6 +111,10 @@
         em.getTransaction().rollback();
     }
     
+    /**
+     * Retired temporarily. Most likely side-effect of eager compilation of
+     * query introduced recently.
+     */
     public void testHint() {
         List<String> targets = new ArrayList<String>();
         targets.add("Even");



Mime
View raw message