You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by aw...@apache.org on 2006/08/22 00:29:57 UTC

svn commit: r433399 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/ openjpa-kernel/src/main/java/org/apache/openjpa/datacache/ openjpa-kernel/src/...

Author: awhite
Date: Mon Aug 21 15:29:47 2006
New Revision: 433399

URL: http://svn.apache.org/viewvc?rev=433399&view=rev
Log:
Add some query validations on compilation.


Modified:
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/AbstractVal.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Avg.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/HasContainsExpressionVisitor.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCExpressionFactory.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Max.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Min.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Sum.java
    incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/MethodStoreQuery.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreQuery.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Aggregate.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AggregateVal.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CandidatePath.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Val.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Value.java
    incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/SQLStoreQuery.java Mon Aug 21 15:29:47 2006
@@ -235,14 +235,8 @@
                         res, q.getContext().getResultType());
             } catch (SQLException se) {
                 if (stmnt != null)
-                    try {
-                        stmnt.close();
-                    } catch (SQLException se2) {
-                    }
-                try {
-                    conn.close();
-                } catch (SQLException se2) {
-                }
+                    try { stmnt.close(); } catch (SQLException se2) {}
+                try { conn.close(); } catch (SQLException se2) {}
                 throw SQLExceptions.getStore(se, dict);
             }
 

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/AbstractVal.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/AbstractVal.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/AbstractVal.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/AbstractVal.java Mon Aug 21 15:29:47 2006
@@ -36,6 +36,10 @@
         return false;
     }
 
+    public boolean isAggregate() {
+        return false;
+    }
+
     public void appendIsEmpty(SQLBuffer sql, Select sel, JDBCStore store,
         Object[] params, JDBCFetchConfiguration fetch) {
         sql.append(FALSE);

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java Mon Aug 21 15:29:47 2006
@@ -62,8 +62,8 @@
         _meta = meta;
     }
 
-    public boolean isVariable() {
-        return false;
+    public boolean isAggregate() {
+        return true;
     }
 
     public Class getType() {

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Avg.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Avg.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Avg.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Avg.java Mon Aug 21 15:29:47 2006
@@ -34,7 +34,7 @@
         return "AVG";
     }
 
-    protected boolean isAggregate() {
+    public boolean isAggregate() {
         return true;
     }
 }

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java Mon Aug 21 15:29:47 2006
@@ -48,7 +48,7 @@
         return "COUNT";
     }
 
-    protected boolean isAggregate() {
+    public boolean isAggregate() {
         return true;
     }
 }

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java Mon Aug 21 15:29:47 2006
@@ -41,9 +41,5 @@
     protected String getOperator() {
         return "DISTINCT";
     }
-
-    protected boolean isAggregate() {
-        return false;
-    }
 }
 

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java Mon Aug 21 15:29:47 2006
@@ -70,6 +70,10 @@
         return false;
     }
 
+    public boolean isAggregate() {
+        return false;
+    }
+
     public Class getType() {
         if (_cast != null)
             return _cast;

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/HasContainsExpressionVisitor.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/HasContainsExpressionVisitor.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/HasContainsExpressionVisitor.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/HasContainsExpressionVisitor.java Mon Aug 21 15:29:47 2006
@@ -14,13 +14,14 @@
 
     private boolean _found = false;
 
-    /**
-     * Whether a contains expression has been found.
-     */
-    public boolean foundContainsExpression() {
-        return _found;
+    public static boolean hasContains(Expression exp) {
+        if (exp == null)
+            return false;
+        HasContainsExpressionVisitor v = new HasContainsExpressionVisitor();
+        exp.acceptVisit(v);
+        return v._found;
     }
-    
+
     public void enter(Expression exp) {
         if (!_found)
             _found = exp instanceof ContainsExpression 

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCExpressionFactory.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCExpressionFactory.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCExpressionFactory.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCExpressionFactory.java Mon Aug 21 15:29:47 2006
@@ -213,13 +213,9 @@
     }
 
     public Expression not(Expression exp) {
-        Exp e = (Exp) exp;
-        HasContainsExpressionVisitor visitor = 
-            new HasContainsExpressionVisitor();
-        e.acceptVisit(visitor);
-        if (visitor.foundContainsExpression())
-            return new NotContainsExpression(e);
-        return new NotExpression(e);
+        if (HasContainsExpressionVisitor.hasContains(exp))
+            return new NotContainsExpression((Exp) exp);
+        return new NotExpression((Exp) exp);
     }
 
     public Expression bindVariable(Value var, Value val) {

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Max.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Max.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Max.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Max.java Mon Aug 21 15:29:47 2006
@@ -34,7 +34,7 @@
         return "MAX";
     }
 
-    protected boolean isAggregate() {
+    public boolean isAggregate() {
         return true;
     }
 }

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Min.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Min.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Min.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Min.java Mon Aug 21 15:29:47 2006
@@ -34,7 +34,7 @@
         return "MIN";
     }
 
-    protected boolean isAggregate() {
+    public boolean isAggregate() {
         return true;
     }
 }

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java Mon Aug 21 15:29:47 2006
@@ -20,6 +20,7 @@
 import java.util.LinkedList;
 import java.util.ListIterator;
 
+import org.apache.commons.lang.ObjectUtils;
 import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
 import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.meta.ClassMapping;
@@ -626,6 +627,22 @@
             _field.appendIsNotNull(sql, sel, _joins);
     }
 
+    public int hashCode() {
+        if (_actions == null)
+            return _candidate.hashCode();
+        return _candidate.hashCode() ^ _actions.hashCode();
+    }
+
+    public boolean equals(Object other) {
+        if (other == this)
+            return true;
+        if (!(other instanceof PCPath))
+            return false;
+        PCPath path = (PCPath) other;
+        return ObjectUtils.equals(_candidate, path._candidate)
+            && ObjectUtils.equals(_actions, path._actions);
+    }
+
     /**
      * Helper class representing an action.
      */
@@ -644,6 +661,20 @@
 
         public String toString() {
             return op + "|" + data;
+        }
+
+        public int hashCode() {
+            if (data == null)
+                return op;
+            return op ^ data.hashCode();
+        }
+
+        public boolean equals(Object other) {
+            if (other == this)
+                return true;
+            Action a = (Action) other;
+            return op == a.op
+                && ObjectUtils.equals(data, a.data);
         }
     }
 }

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java Mon Aug 21 15:29:47 2006
@@ -205,12 +205,8 @@
     private void initializeJoins(Select sel, JDBCStore store,
         QueryExpressions exps, Object[] params) {
         Map contains = null;
-        HasContainsExpressionVisitor visitor = 
-            new HasContainsExpressionVisitor();
-        exps.filter.acceptVisit(visitor);
-        if (!visitor.foundContainsExpression() && exps.having != null)
-            exps.having.acceptVisit(visitor);
-        if (visitor.foundContainsExpression())
+        if (HasContainsExpressionVisitor.hasContains(exps.filter)
+            || HasContainsExpressionVisitor.hasContains(exps.having))
             contains = new HashMap(7);
 
         // initialize filter and having expressions

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Sum.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Sum.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Sum.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Sum.java Mon Aug 21 15:29:47 2006
@@ -45,7 +45,7 @@
         return "SUM";
     }
 
-    protected boolean isAggregate() {
+    public boolean isAggregate() {
         return true;
     }
 }

Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java Mon Aug 21 15:29:47 2006
@@ -145,13 +145,6 @@
     }
 
     /**
-     * Return whether this operator is an aggregate.
-     */
-    protected boolean isAggregate() {
-        return false;
-    }
-
-    /**
      * Return the type of this value based on the argument type. Returns
      * the argument type by default.
      */

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/datacache/QueryCacheStoreQuery.java Mon Aug 21 15:29:47 2006
@@ -380,6 +380,10 @@
             return EMPTY_STRINGS;
         }
 
+        public void validate(StoreQuery q) {
+            _ex.validate(unwrap(q));
+        }
+
         public Object getOrderingValue(StoreQuery q, Object[] params,
             Object resultObject, int orderIndex) {
             return _ex.getOrderingValue(unwrap(q), params, resultObject,

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java Mon Aug 21 15:29:47 2006
@@ -137,6 +137,9 @@
             return EMPTY_STRINGS;
         }
 
+        public void validate(StoreQuery q) {
+        }
+
         public Object getOrderingValue(StoreQuery q, Object[] params,
             Object resultObject, int orderIndex) {
             return null;

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java Mon Aug 21 15:29:47 2006
@@ -17,9 +17,11 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.collections.map.LinkedMap;
 import org.apache.openjpa.conf.OpenJPAConfiguration;
@@ -285,8 +287,14 @@
         extends AbstractExecutor
         implements Executor {
 
-        abstract QueryExpressions[] getQueryExpressions();
+        /**
+         * Return the parsed query expressions for our candidate types.
+         */
+        protected abstract QueryExpressions[] getQueryExpressions();
 
+        /**
+         * Return the query expressions for one candidate type, or die if none.
+         */
         private QueryExpressions assertQueryExpression() {
             QueryExpressions[] exp = getQueryExpressions();
             if (exp == null || exp.length < 1)
@@ -315,6 +323,11 @@
             }
         }
 
+        public final void validate(StoreQuery q) {
+            QueryExpressions exps = assertQueryExpression();    
+            ValidateGroupingExpressionVisitor.validate(q.getContext(), exps); 
+        }
+
         public final Class getResultClass(StoreQuery q) {
             return assertQueryExpression().resultClass;
         }
@@ -369,6 +382,66 @@
         public boolean isPacking(StoreQuery q) {
             return false;
         }
+
+        /**
+         * Throws an exception if select or having clauses contain 
+         * non-aggregate, non-grouped paths.
+         */
+        private static class ValidateGroupingExpressionVisitor 
+            extends AbstractExpressionVisitor {
+
+            private final QueryContext _ctx;
+            private boolean _grouping = false;
+            private Set _grouped = null;
+            private Value _agg = null;
+
+            /**
+             * Throw proper exception if query does not meet validation.
+             */
+            public static void validate(QueryContext ctx, 
+                QueryExpressions exps) {
+                if (exps.grouping.length == 0)
+                    return;
+
+                ValidateGroupingExpressionVisitor visitor = 
+                    new ValidateGroupingExpressionVisitor(ctx);
+                visitor._grouping = true;
+                for (int i = 0; i < exps.grouping.length; i++)
+                    exps.grouping[i].acceptVisit(visitor);
+                visitor._grouping = false;
+                if (exps.having != null)
+                    exps.having.acceptVisit(visitor);
+                for (int i = 0; i < exps.projections.length; i++)
+                    exps.projections[i].acceptVisit(visitor);
+            }
+
+            public ValidateGroupingExpressionVisitor(QueryContext ctx) {
+                _ctx = ctx;
+            }
+
+            public void enter(Value val) {
+                if (_grouping) {
+                    if (val instanceof Path) {
+                        if (_grouped == null)
+                            _grouped = new HashSet();
+                        _grouped.add(val);
+                    }
+                } else if (_agg == null) {
+                    if (val.isAggregate()) 
+                        _agg = val;
+                    else if (val instanceof Path 
+                        && (_grouped == null || !_grouped.contains(val))) {
+                        throw new UserException(_loc.get("bad-grouping",
+                            _ctx.getCandidateType(), _ctx.getQueryString())); 
+                    }
+                }
+            }
+
+            public void exit(Value val) {
+                if (val == _agg)
+                    _agg = null;
+            }
+        }
     }
 
     /**
@@ -410,7 +483,7 @@
             }
         }
 
-        QueryExpressions[] getQueryExpressions() {
+        protected QueryExpressions[] getQueryExpressions() {
             return _exps;
         }
 
@@ -568,7 +641,7 @@
             }
         }
 
-        QueryExpressions[] getQueryExpressions() {
+        protected QueryExpressions[] getQueryExpressions() {
             return _exps;
         }
 

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/MethodStoreQuery.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/MethodStoreQuery.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/MethodStoreQuery.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/MethodStoreQuery.java Mon Aug 21 15:29:47 2006
@@ -206,47 +206,50 @@
          * result.
          */
         private Object invoke(StoreQuery q, Object[] args) {
-            Method meth = _meth;
-            if (meth == null) {
-                String methName = q.getContext().getQueryString();
-                if (methName == null || methName.length() == 0)
-                    throw new UserException(_loc.get("no-method"));
-
-                int dotIdx = methName.lastIndexOf('.');
-                Class cls;
-                if (dotIdx == -1)
-                    cls = _meta.getDescribedType();
-                else {
-                    cls = q.getContext().classForName(methName.substring(0,
-                        dotIdx), null);
-                    if (cls == null)
-                        throw new UserException(_loc.get("bad-method-class",
-                            methName.substring(0, dotIdx), methName));
-                    methName = methName.substring(dotIdx + 1);
-                }
-
-                Class[] types = (_inMem) ? ARGS_INMEM : ARGS_DATASTORE;
-                try {
-                    meth = cls.getMethod(methName, types);
-                } catch (Exception e) {
-                    String msg = (_inMem) ? "bad-inmem-method"
-                        : "bad-datastore-method";
-                    throw new UserException(_loc.get(msg, methName, cls));
-                }
-                if (!Modifier.isStatic(meth.getModifiers()))
-                    throw new UserException(_loc.get("method-not-static",
-                        meth));
-                _meth = meth;
-            }
-
+            validate(q);
             try {
-                return meth.invoke(null, args);
+                return _meth.invoke(null, args);
             } catch (OpenJPAException ke) {
                 throw ke;
             } catch (Exception e) {
                 throw new UserException(_loc.get("method-error", _meth,
                     Exceptions.toString(Arrays.asList(args))), e);
             }
+        }
+
+        public void validate(StoreQuery q) {
+            if (_meth != null)
+                return;
+
+            String methName = q.getContext().getQueryString();
+            if (methName == null || methName.length() == 0)
+                throw new UserException(_loc.get("no-method"));
+
+            int dotIdx = methName.lastIndexOf('.');
+            Class cls;
+            if (dotIdx == -1)
+                cls = _meta.getDescribedType();
+            else {
+                cls = q.getContext().classForName(methName.substring(0, dotIdx),
+                    null);
+                if (cls == null)
+                    throw new UserException(_loc.get("bad-method-class",
+                        methName.substring(0, dotIdx), methName));
+                methName = methName.substring(dotIdx + 1);
+            }
+
+            Method meth;
+            Class[] types = (_inMem) ? ARGS_INMEM : ARGS_DATASTORE;
+            try {
+                meth = cls.getMethod(methName, types);
+            } catch (Exception e) {
+                String msg = (_inMem) ? "bad-inmem-method"
+                    : "bad-datastore-method";
+                throw new UserException(_loc.get(msg, methName, cls));
+            }
+            if (!Modifier.isStatic(meth.getModifiers()))
+                throw new UserException(_loc.get("method-not-static", meth));
+            _meth = meth;
         }
 
         public LinkedMap getParameterTypes(StoreQuery q) {

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java Mon Aug 21 15:29:47 2006
@@ -576,7 +576,9 @@
         lock();
         try {
             assertOpen();
-            getResultPacker(compileForExecutor());
+            StoreQuery.Executor ex = compileForExecutor();
+            getResultPacker(ex);
+            ex.validate(_storeQuery);
         } finally {
             unlock();
         }
@@ -1875,6 +1877,10 @@
                     results.addAll(Arrays.asList(actions));
             }
             return (String[]) results.toArray(new String[results.size()]);
+        }
+
+        public void validate(StoreQuery q) {
+            _executors[0].validate(q);
         }
 
         public Object getOrderingValue(StoreQuery q, Object[] params,

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreQuery.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreQuery.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreQuery.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreQuery.java Mon Aug 21 15:29:47 2006
@@ -223,6 +223,11 @@
             long startIdx, long endIdx);
 
         /**
+         * Validate components of query.
+         */
+        public void validate(StoreQuery q);
+
+        /**
          * Extract the value of the <code>orderIndex</code>th ordering
          * expression in {@link Query#getOrderingClauses} from the
          * given result object. The result object will be an object from

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Aggregate.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Aggregate.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Aggregate.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Aggregate.java Mon Aug 21 15:29:47 2006
@@ -43,6 +43,10 @@
         _arg = arg;
     }
 
+    public boolean isAggregate() {
+        return true;
+    }
+
     public Class getType() {
         return _listener.getType(getArgTypes());
     }

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AggregateVal.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AggregateVal.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AggregateVal.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AggregateVal.java Mon Aug 21 15:29:47 2006
@@ -42,6 +42,10 @@
         _val = val;
     }
 
+    public boolean isAggregate() {
+        return true;
+    }
+
     public Class getType() {
         return getType(_val.getType());
     }

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CandidatePath.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CandidatePath.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CandidatePath.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CandidatePath.java Mon Aug 21 15:29:47 2006
@@ -19,6 +19,7 @@
 import java.util.LinkedList;
 import java.util.ListIterator;
 
+import org.apache.commons.lang.ObjectUtils;
 import org.apache.openjpa.enhance.PersistenceCapable;
 import org.apache.openjpa.kernel.Broker;
 import org.apache.openjpa.kernel.Filters;
@@ -145,6 +146,18 @@
         return candidate;
     }
 
+    public int hashCode() {
+        return (_actions == null) ? 0 : _actions.hashCode();
+    }
+
+    public boolean equals(Object other) {
+        if (other == this)
+            return true;
+        if (!(other instanceof CandidatePath))
+            return false;
+        return ObjectUtils.equals(_actions, ((CandidatePath) other)._actions);
+    }
+
     /**
      * Represents a traversal through a field.
      */
@@ -156,6 +169,16 @@
         private Traversal(FieldMetaData field, boolean nullTraversal) {
             this.field = field;
             this.nullTraversal = nullTraversal;
+        }
+
+        public int hashCode() {
+            return field.hashCode();
+        }
+
+        public boolean equals(Object other) {
+            if (other == this)
+                return true;
+            return ((Traversal) other).field.equals(field);
         }
 	}
 }

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Val.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Val.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Val.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Val.java Mon Aug 21 15:29:47 2006
@@ -103,6 +103,10 @@
         return false;
     }
 
+    public boolean isAggregate() {
+        return false;
+    }
+
     public void acceptVisit(ExpressionVisitor visitor) {
         visitor.enter(this);
         visitor.exit(this);

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Value.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Value.java?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Value.java (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Value.java Mon Aug 21 15:29:47 2006
@@ -44,6 +44,11 @@
     public boolean isVariable();
 
     /**
+     * Return true if this value is an aggregate.
+     */
+    public boolean isAggregate();
+
+    /**
      * Return any associated persistent type.
      */
     public ClassMetaData getMetaData();

Modified: incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties?rev=433399&r1=433398&r2=433399&view=diff
==============================================================================
--- incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties (original)
+++ incubator/openjpa/trunk/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties Mon Aug 21 15:29:47 2006
@@ -199,7 +199,7 @@
 	have a version field.
 inmem-agg-proj-var: Queries with aggregates or projections using variables \
 	currently cannot be executed in-memory.  Either set IgnoreCache to true, \
-	set the org.apache.openjpa.FlushBeforeQueries property to true, or execute the query \
+	set the openjpa.FlushBeforeQueries property to true, or execute the query \
 	before changing any instances in the transaction.  The offending query was \
 	on type "{0}" with filter "{1}".
 merged-order-with-result: This query on candidate type "{0}" with filter "{1}" \
@@ -207,6 +207,9 @@
 	You have chosen to order the results on "{2}", but you have not selected \
 	this data in your setResult() clause.  Please include this ordering data \
 	in setResult() so that OpenJPA can extract it for in-memory ordering.
+bad-grouping: Your query on type "{0}" with filter "{1}" is invalid.  Your \
+    select and having clauses must only include aggregates or values that also \
+    appear in your grouping clause.
 query-nosupport: The "{0}" query type does not support this operation.
 range-too-big: The range of the query is too big. Start index: "{0}", end \
 	index: "{1}". The range must be less than Integer.MAX_VALUE.