Add visitor pattern to query expression trees, and redo a couple of our existing

tree interrogation methods to use it.  This will allow us to add more 
validations at query compilation time (we're missing some that are required for
JDO 2, for example) without having to expand the API of every query 
expression/value implementation, in addition to just being more flexible 
overall.



git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@432778 13f79535-47bb-0310-9956-ffa450edef68

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/AbstractVal.java

@@ -19,17 +19,22 @@ import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
 import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
- * Aggregate listener that evaluates to a value.
+ * Abstract value for easy extension.
  *
  * @author Marc Prud'hommeaux
  */
 abstract class AbstractVal
     implements Val {
 
-    private static final String TRUE = "1 = 1";
+    protected static final String TRUE = "1 = 1";
-    private static final String FALSE = "1 <> 1";
+    protected static final String FALSE = "1 <> 1";
+
+    public boolean isVariable() {
+        return false;
+    }
 
     public void appendIsEmpty(SQLBuffer sql, Select sel, JDBCStore store,
         Object[] params, JDBCFetchConfiguration fetch) {
@@ -57,5 +62,10 @@ abstract class AbstractVal
         Object[] params, JDBCFetchConfiguration fetch) {
         sql.append("1");
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Aggregate.java

@@ -26,6 +26,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -34,8 +35,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 class Aggregate
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     private final JDBCAggregateListener _listener;
     private final Val _arg;
@@ -145,12 +145,6 @@ class Aggregate
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        if (_arg != null)
-            return _arg.hasVariable(var);
-        return false;
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         if (_arg != null)
@@ -189,4 +183,11 @@ class Aggregate
             new FilterValueImpl(_arg, sel, store, params, fetch)
         };
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        if (_arg != null)
+            _arg.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/AndExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Combines two expressions.
@@ -81,11 +82,10 @@ class AndExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return _exp1.hasContainsExpression() || _exp2.hasContainsExpression();
+        visitor.enter(this);
-    }
+        _exp1.acceptVisit(visitor);
-
+        _exp2.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _exp1.hasVariable(var) || _exp2.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Args.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.exps.Arguments;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.kernel.exps.Value;
 import org.apache.openjpa.meta.ClassMetaData;
 
@@ -31,7 +32,8 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 public class Args
-    implements Val, Arguments {
+    extends AbstractVal
+    implements Arguments {
 
     private final Val[] _args;
     private Joins _joins = null;
@@ -130,13 +132,6 @@ public class Args
         return null;
     }
 
-    public boolean hasVariable(Variable var) {
-        for (int i = 0; i < _args.length; i++)
-            if (_args[i].hasVariable(var))
-                return true;
-        return false;
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         for (int i = 0; i < _args.length; i++)
@@ -175,4 +170,11 @@ public class Args
     public void appendIsNotNull(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        for (int i = 0; i < _args.length; i++)
+            _args[i].acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/BindVariableAndExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Combines a bind variable expression with another.
@@ -69,11 +70,10 @@ class BindVariableAndExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return true;
+        visitor.enter(this);
-    }
+        _bind.acceptVisit(visitor);
-
+        _exp.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _exp.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/BindVariableExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Binds a variable to a value. Typically, the {@link #initialize} and
@@ -71,11 +72,9 @@ class BindVariableExpression
         return _var.getJoins();
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _var.acceptVisit(visitor);
-
+        visitor.exit(this);
-    public boolean hasVariable(Variable var) {
-        return _var == var;
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/CompareEqualExpression.java

@@ -23,6 +23,7 @@ import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.lib.util.Localizer;
 import org.apache.openjpa.util.UserException;
 
@@ -108,14 +109,6 @@ abstract class CompareEqualExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
-        return false;
-    }
-
-    public boolean hasVariable(Variable var) {
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
-    }
-
     /**
      * Append the SQL for the comparison.
      */
@@ -131,4 +124,11 @@ abstract class CompareEqualExpression
     protected boolean isDirectComparison() {
         return true;
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/CompareExpression.java

@@ -23,6 +23,7 @@ import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.lib.util.Localizer;
 import org.apache.openjpa.util.UserException;
 
@@ -91,11 +92,10 @@ class CompareExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _val1.acceptVisit(visitor);
-
+        _val2.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Concat.java

@@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -35,8 +36,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Marc Prud'hommeaux
  */
 class Concat
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     private final Val _val1;
     private final Val _val2;
@@ -137,10 +137,6 @@ class Concat
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _val1.calculateValue(sel, store, params, null, fetch);
@@ -167,5 +163,12 @@ class Concat
         _val2.appendTo(sql, 0, sel, store, params, fetch);
         sql.append(_part3);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Const.java

@@ -38,7 +38,8 @@ import org.apache.openjpa.meta.JavaTypes;
  * @author Abe White
  */
 abstract class Const
-    implements Val, Constant {
+    extends AbstractVal
+    implements Constant {
 
     private ClassMetaData _meta = null;
     private Column[] _cols = null;
@@ -51,10 +52,6 @@ abstract class Const
         _meta = meta;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     /**
      * Return the column for the value at the specified index, or null.
      */
@@ -152,10 +149,6 @@ abstract class Const
         return Filters.convert(res.getObject(this, code, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return false;
-    }
-
     public int length() {
         return 1;
     }
@@ -164,22 +157,22 @@ abstract class Const
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         Object obj = getValue();
         if (obj instanceof Collection && ((Collection) obj).isEmpty())
-            sql.append("1 = 1");
+            sql.append(TRUE);
         else if (obj instanceof Map && ((Map) obj).isEmpty())
-            sql.append("1 = 1");
+            sql.append(TRUE);
         else
-            sql.append("1 <> 1");
+            sql.append(FALSE);
     }
 
     public void appendIsNotEmpty(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         Object obj = getValue();
         if (obj instanceof Collection && ((Collection) obj).isEmpty())
-            sql.append("1 <> 1");
+            sql.append(FALSE);
         else if (obj instanceof Map && ((Map) obj).isEmpty())
-            sql.append("1 <> 1");
+            sql.append(FALSE);
         else
-            sql.append("1 = 1");
+            sql.append(TRUE);
     }
 
     public void appendSize(SQLBuffer sql, Select sel, JDBCStore store,
@@ -196,16 +189,16 @@ abstract class Const
     public void appendIsNull(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         if (getSQLValue() == null)
-            sql.append("1 = 1");
+            sql.append(TRUE);
         else
-            sql.append("1 <> 1");
+            sql.append(FALSE);
     }
 
     public void appendIsNotNull(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         if (getSQLValue() != null)
-            sql.append("1 = 1");
+            sql.append(TRUE);
         else
-            sql.append("1 <> 1");
+            sql.append(FALSE);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/ConstInstanceofExpression.java

@@ -23,6 +23,7 @@ import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Tests whether a value is an instance of a class.
@@ -67,11 +68,9 @@ class ConstInstanceofExpression
         return _const.getJoins();
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _const.acceptVisit(visitor);
-
+        visitor.exit(this);
-    public boolean hasVariable(Variable var) {
-        return _const.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/ConstPath.java

@@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Broker;
 import org.apache.openjpa.kernel.Filters;
 import org.apache.openjpa.kernel.OpenJPAStateManager;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 import org.apache.openjpa.meta.FieldMetaData;
 
@@ -175,4 +176,10 @@ class ConstPath
         _val = null;
         _sqlVal = null;
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _constant.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/ContainsExpression.java

@@ -59,8 +59,4 @@ class ContainsExpression
     protected boolean isDirectComparison() {
         return false;
     }
-
-    public boolean hasContainsExpression() {
-        return true;
-    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java

@@ -35,7 +35,6 @@ class Count
 
     public void initialize(Select sel, JDBCStore store, boolean nullTest) {
         super.initialize(sel, store, nullTest);
-
         // join into related object if present
         if (getVal()instanceof PCPath)
             ((PCPath) getVal()).joinRelation();

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/EmptyExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * An empty expression.
@@ -47,11 +48,8 @@ class EmptyExpression
         return null;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        visitor.exit(this);
-
-    public boolean hasVariable(Variable var) {
-        return false;
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/EndsWithExpression.java

@@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.sql.DBDictionary;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Test if one string ends with another.
@@ -116,12 +117,11 @@ class EndsWithExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _val1.acceptVisit(visitor);
-
+        _val2.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
     }
 
     /**

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Exp.java

@@ -69,15 +69,4 @@ interface Exp
      * that common joins are moved up the expression tree.
      */
     public Joins getJoins();
-
-    /**
-     * Return true if this expression is or is made up of a contains expression.
-     */
-    public boolean hasContainsExpression();
-
-    /**
-     * Return true if the expression or any subexpression uses the given
-     * variable.
-     */
-    public boolean hasVariable(Variable var);
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Extension.java

@@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -159,11 +160,6 @@ class Extension
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return (_target != null && _target.hasVariable(var))
-            || (_arg != null && _arg.hasVariable(var));
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         if (_target != null)
@@ -209,6 +205,15 @@ class Extension
         };
     }
 
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter((Exp) this);
+        if (_target != null)
+            _target.acceptVisit(visitor);
+        if (_arg != null)
+            _arg.acceptVisit(visitor);
+        visitor.exit((Exp) this);
+    }
+
     //////////////////////
     // Exp implementation
     //////////////////////
@@ -225,8 +230,4 @@ class Extension
         sel.append(sql, getJoins());
         clearParameters();
     }
-
-    public boolean hasContainsExpression() {
-        return false;
-    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/GetObjectId.java

@@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.lib.util.Localizer;
 import org.apache.openjpa.meta.ClassMetaData;
 import org.apache.openjpa.util.ApplicationIds;
@@ -41,8 +42,7 @@ import serp.util.Numbers;
  * @author Abe White
  */
 class GetObjectId
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     private static final Localizer _loc = Localizer.forPackage
         (GetObjectId.class);
@@ -69,10 +69,6 @@ class GetObjectId
         _meta = meta;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return Object.class;
     }
@@ -159,10 +155,6 @@ class GetObjectId
         return _path.load(res, store, true, fetch);
     }
 
-    public boolean hasVariable(Variable var) {
-        return _path.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _path.calculateValue(sel, store, params, null, fetch);
@@ -180,5 +172,11 @@ class GetObjectId
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         _path.appendTo(sql, index, sel, store, params, fetch);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _path.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/HasContainsExpressionVisitor.java

@@ -0,0 +1,29 @@
+package org.apache.openjpa.jdbc.kernel.exps;
+
+import org.apache.openjpa.kernel.exps.AbstractExpressionVisitor;
+import org.apache.openjpa.kernel.exps.Expression;
+import org.apache.openjpa.kernel.exps.Value;
+
+/**
+ * Determines whether the visited expressions include a "contains" expression.
+ * 
+ * @author Abe White
+ */
+class HasContainsExpressionVisitor 
+    extends AbstractExpressionVisitor {
+
+    private boolean _found = false;
+
+    /**
+     * Whether a contains expression has been found.
+     */
+    public boolean foundContainsExpression() {
+        return _found;
+    }
+    
+    public void enter(Expression exp) {
+        if (!_found)
+            _found = exp instanceof ContainsExpression 
+                || exp instanceof BindVariableAndExpression;
+    }
+} 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InExpression.java

@@ -26,6 +26,7 @@ import org.apache.openjpa.jdbc.schema.Column;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Tests whether a value is IN a collection.
@@ -158,18 +159,17 @@ class InExpression
         return _val.getJoins();
     }
 
-    public boolean hasContainsExpression() {
-        return false;
-    }
-
-    public boolean hasVariable(Variable var) {
-        return _const.hasVariable(var) || _val.hasVariable(var);
-    }
-
     /**
      * Return the collection to test for containment with.
      */
     protected Collection getCollection() {
         return (Collection) _const.getValue();
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        _const.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InSubQExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Tests whether a value is IN a subquery.
@@ -69,11 +70,10 @@ class InSubQExpression
         return _val.getJoins();
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _val.acceptVisit(visitor);
-
+        _sub.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _val.hasVariable(var) || _sub.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IndexOf.java

@@ -25,6 +25,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -33,8 +34,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 class IndexOf
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     private final Val _val1;
     private final Val _val2;
@@ -123,10 +123,6 @@ class IndexOf
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _val1.calculateValue(sel, store, params, null, fetch);
@@ -158,5 +154,12 @@ class IndexOf
 
         store.getDBDictionary().indexOf(sql, str, search, start);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InstanceofExpression.java

@@ -26,6 +26,7 @@ import org.apache.openjpa.jdbc.meta.MappingRepository;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.JavaTypes;
 
 /**
@@ -143,12 +144,10 @@ class InstanceofExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _path.acceptVisit(visitor);
-
+        visitor.exit(this);
-    public boolean hasVariable(Variable var) {
-        return _path.hasVariable(var);
     }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IsEmptyExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Tests whether the given value is empty.
@@ -62,11 +63,9 @@ class IsEmptyExpression
         return _val.getJoins();
     }
     
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _val.acceptVisit(visitor);
-
+        visitor.exit(this);
-    public boolean hasVariable(Variable var) {
-        return _val.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/IsNotEmptyExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Tests whether the given value is not empty.
@@ -62,11 +63,9 @@ class IsNotEmptyExpression
         return _val.getJoins();
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _val.acceptVisit(visitor);
-
+        visitor.exit(this);
-    public boolean hasVariable(Variable var) {
-        return _val.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCExpressionFactory.java

@@ -214,7 +214,10 @@ public class JDBCExpressionFactory
 
     public Expression not(Expression exp) {
         Exp e = (Exp) exp;
-        if (e.hasContainsExpression())
+        HasContainsExpressionVisitor visitor = 
+            new HasContainsExpressionVisitor();
+        e.acceptVisit(visitor);
+        if (visitor.foundContainsExpression())
             return new NotContainsExpression(e);
         return new NotExpression(e);
     }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCStringContains.java

@@ -26,8 +26,7 @@ import org.apache.openjpa.util.UserException;
  * Tests if the target contains the given argument. The argument must be
  * a constant.
  *  Examples:<br />
- * <code> "address.street.ext:stringContains (\"main\")"
+ * <code>"address.street.ext:stringContains (\"main\")"</code>
- * </code>
  *
  * @nojavadoc
  * @deprecated Use <code>matches()</code> instead.

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/JDBCWildcardMatch.java

@@ -27,8 +27,7 @@ import org.apache.openjpa.util.UserException;
  * argument. The wildcard '?' is used to represent any single character,
  * while '*' is used to represent any series of 0 or more characters.
  *  Examples:<br />
- * <code> "address.street.ext:wildcardMatch (\"?ain*reet\")"
+ * <code>"address.street.ext:wildcardMatch (\"?ain*reet\")"</code>
- * </code>
  *
  * @nojavadoc
  * @deprecated Use <code>matches()</code> instead.

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/MatchesExpression.java

@@ -23,6 +23,7 @@ import org.apache.openjpa.jdbc.schema.Column;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import serp.util.Strings;
 
 /**
@@ -123,11 +124,10 @@ class MatchesExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _val.acceptVisit(visitor);
-
+        _const.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _val.hasVariable(var) || _const.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Math.java

@@ -25,6 +25,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -33,8 +34,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 class Math
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     public static final String ADD = "+";
     public static final String SUBTRACT = "-";
@@ -134,10 +134,6 @@ class Math
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _val1.calculateValue(sel, store, params, _val2, fetch);
@@ -159,5 +155,12 @@ class Math
             new FilterValueImpl(_val1, sel, store, params, fetch),
             new FilterValueImpl(_val2, sel, store, params, fetch));
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/NotContainsExpression.java

@@ -23,6 +23,7 @@ import org.apache.openjpa.jdbc.sql.DBDictionary;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Negates a contains expression using a subselect to make sure no
@@ -76,11 +77,9 @@ class NotContainsExpression
         return null;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _exp.acceptVisit(visitor);
-
+        visitor.exit(this);
-    public boolean hasVariable(Variable var) {
-        return _exp.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/NotExpression.java

@@ -22,6 +22,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Negates an expression.
@@ -63,11 +64,9 @@ class NotExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return _exp.hasContainsExpression();
+        visitor.enter(this);
-    }
+        _exp.acceptVisit(visitor);
-
+        visitor.exit(this);
-    public boolean hasVariable(Variable var) {
-        return _exp.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/OrExpression.java

@@ -24,6 +24,7 @@ import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 
 /**
  * Combines two expressions.
@@ -97,11 +98,10 @@ class OrExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return _exp1.hasContainsExpression() || _exp2.hasContainsExpression();
+        visitor.enter(this);
-    }
+        _exp1.acceptVisit(visitor);
-
+        _exp2.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _exp1.hasVariable(var) || _exp2.hasVariable(var);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java

@@ -44,6 +44,7 @@ import org.apache.openjpa.util.UserException;
  * @author Abe White
  */
 class PCPath
+    extends AbstractVal
     implements JDBCPath {
 
     private static final int PATH = 0;
@@ -543,6 +544,9 @@ class PCPath
         return ret;
     }
 
+    /**
+     * Whether the given variable appears in this path.
+     */
     public boolean hasVariable(Variable var) {
         if (_actions == null)
             return false;
@@ -585,7 +589,7 @@ class PCPath
     public void appendIsEmpty(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         if (_field == null)
-            sql.append("1 <> 1");
+            sql.append(FALSE);
         else
             _field.appendIsEmpty(sql, sel, _joins);
     }
@@ -593,7 +597,7 @@ class PCPath
     public void appendIsNotEmpty(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         if (_field == null)
-            sql.append("1 <> 1");
+            sql.append(FALSE);
         else
             _field.appendIsNotEmpty(sql, sel, _joins);
     }
@@ -609,7 +613,7 @@ class PCPath
     public void appendIsNull(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         if (_field == null)
-            sql.append("1 <> 1");
+            sql.append(FALSE);
         else
             _field.appendIsNull(sql, sel, _joins);
     }
@@ -617,7 +621,7 @@ class PCPath
     public void appendIsNotNull(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
         if (_field == null)
-            sql.append("1 = 1");
+            sql.append(TRUE);
         else
             _field.appendIsNotNull(sql, sel, _joins);
     }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SQLEmbed.java

@@ -27,8 +27,7 @@ import org.apache.openjpa.util.UserException;
  * Simple listener which embeds its SQL argument into the query. Listens
  * on <code>sql</code>.
  *  Example:<br />
- * <code> "price &lt; sql(\"(SELECT AVG (PRICE) FROM PRODUCT_TABLE)\")"
+ * <code>"price &lt; sql(\"(SELECT AVG (PRICE) FROM PRODUCT_TABLE)\")"</code>
- * </code>
  *
  * @nojavadoc
  */

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java

@@ -205,9 +205,12 @@ class SelectConstructor {
     private void initializeJoins(Select sel, JDBCStore store,
         QueryExpressions exps, Object[] params) {
         Map contains = null;
-        if (((Exp) exps.filter).hasContainsExpression()
+        HasContainsExpressionVisitor visitor = 
-            || (exps.having != null
+            new HasContainsExpressionVisitor();
-            && ((Exp) exps.having).hasContainsExpression()))
+        exps.filter.acceptVisit(visitor);
+        if (!visitor.foundContainsExpression() && exps.having != null)
+            exps.having.acceptVisit(visitor);
+        if (visitor.foundContainsExpression())
             contains = new HashMap(7);
 
         // initialize filter and having expressions

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Size.java

@@ -27,8 +27,7 @@ import org.apache.openjpa.util.InternalException;
  * @author Marc Prud'hommeaux
  */
 class Size
-    extends UnaryOp
+    extends UnaryOp {
-    implements Val {
 
     public Size(Val val) {
         super(val);

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/StartsWithExpression.java

@@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.sql.DBDictionary;
 import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import serp.util.Numbers;
 
 /**
@@ -117,12 +118,11 @@ class StartsWithExpression
         return _joins;
     }
 
-    public boolean hasContainsExpression() {
+    public void acceptVisit(ExpressionVisitor visitor) {
-        return false;
+        visitor.enter(this);
-    }
+        _val1.acceptVisit(visitor);
-
+        _val2.acceptVisit(visitor);
-    public boolean hasVariable(Variable var) {
+        visitor.exit(this);
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
     }
 
     /**

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/StringFunction.java

@@ -25,6 +25,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -33,8 +34,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Marc Prud'hommeaux
  */
 abstract class StringFunction
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     final Val _val;
     ClassMetaData _meta = null;
@@ -111,10 +111,6 @@ abstract class StringFunction
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return _val.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _val.calculateValue(sel, store, params, null, fetch);
@@ -134,5 +130,11 @@ abstract class StringFunction
         _val.appendTo(sql, 0, sel, store, params, fetch);
         sql.append(_post);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SubQ.java

@@ -26,6 +26,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.kernel.exps.QueryExpressions;
 import org.apache.openjpa.kernel.exps.Subquery;
 import org.apache.openjpa.meta.ClassMetaData;
@@ -36,7 +37,8 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 class SubQ
-    implements Val, Subquery {
+    extends AbstractVal
+    implements Subquery {
 
     private final ClassMapping _candidate;
     private final boolean _subs;
@@ -155,25 +157,6 @@ class SubQ
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        for (int i = 0; i < _exps.projections.length; i++)
-            if (((Val) _exps.projections[i]).hasVariable(var))
-                return true;
-        if (_exps.filter != null)
-            if (((Exp) _exps.filter).hasVariable(var))
-                return true;
-        for (int i = 0; i < _exps.grouping.length; i++)
-            if (((Val) _exps.grouping[i]).hasVariable(var))
-                return true;
-        if (_exps.having != null)
-            if (((Exp) _exps.having).hasVariable(var))
-                return true;
-        for (int i = 0; i < _exps.ordering.length; i++)
-            if (((Val) _exps.ordering[i]).hasVariable(var))
-                return true;
-        return false;
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
     }
@@ -222,15 +205,18 @@ class SubQ
         appendTo(sql, 0, sel, store, params, fetch, true);
     }
 
-    public void appendIsNull(SQLBuffer sql, Select sel,
+    public void acceptVisit(ExpressionVisitor visitor) {
-        JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
+        visitor.enter(this);
-        appendTo(sql, 0, sel, store, params, fetch);
+        for (int i = 0; i < _exps.projections.length; i++)
-        sql.append(" IS NULL");
+            _exps.projections[i].acceptVisit(visitor);
-    }
+        if (_exps.filter != null)
-
+            _exps.filter.acceptVisit(visitor);
-    public void appendIsNotNull(SQLBuffer sql, Select sel,
+        for (int i = 0; i < _exps.grouping.length; i++)
-        JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
+            _exps.grouping[i].acceptVisit(visitor);
-        appendTo(sql, 0, sel, store, params, fetch);
+        if (_exps.having != null)
-        sql.append(" IS NOT NULL");
+            _exps.having.acceptVisit(visitor);
+        for (int i = 0; i < _exps.ordering.length; i++)
+            _exps.ordering[i].acceptVisit(visitor);
+        visitor.exit(this);
     }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Substring.java

@@ -25,6 +25,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -33,8 +34,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 class Substring
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     private final Val _val1;
     private final Val _val2;
@@ -119,10 +119,6 @@ class Substring
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return _val1.hasVariable(var) || _val2.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _val1.calculateValue(sel, store, params, null, fetch);
@@ -153,5 +149,12 @@ class Substring
 
         store.getDBDictionary().substring(sql, str, start, end);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Trim.java

@@ -27,6 +27,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.kernel.exps.Literal;
 import org.apache.openjpa.meta.ClassMetaData;
 
@@ -36,8 +37,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Marc Prud'hommeaux
  */
 class Trim
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     private final Val _val;
     private final Val _trimChar;
@@ -133,10 +133,6 @@ class Trim
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return _val.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _val.calculateValue(sel, store, params, null, fetch);
@@ -192,5 +188,12 @@ class Trim
             }
         }
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        _trimChar.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java

@@ -25,6 +25,7 @@ import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
 import org.apache.openjpa.kernel.Filters;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -33,8 +34,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 abstract class UnaryOp
-    extends AbstractVal
+    extends AbstractVal {
-    implements Val {
 
     private final Val _val;
     private ClassMetaData _meta = null;
@@ -123,10 +123,6 @@ abstract class UnaryOp
             JavaSQLTypes.JDBC_DEFAULT, null), getType());
     }
 
-    public boolean hasVariable(Variable var) {
-        return _val.hasVariable(var);
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         _val.calculateValue(sel, store, params, null, fetch);
@@ -167,5 +163,11 @@ abstract class UnaryOp
      * Return the name of this operator.
      */
     protected abstract String getOperator();
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Val.java

@@ -96,11 +96,6 @@ public interface Val
         JDBCFetchConfiguration fetch)
         throws SQLException;
 
-    /**
-     * Return true if this value uses the given variable.
-     */
-    public boolean hasVariable(Variable var);
-
     /**
      * Calculate and cache the SQL for this value. This method is called
      * before <code>length</code> or any <code>append</code> methods.

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Variable.java

@@ -23,6 +23,7 @@ import org.apache.openjpa.jdbc.sql.Joins;
 import org.apache.openjpa.jdbc.sql.Result;
 import org.apache.openjpa.jdbc.sql.SQLBuffer;
 import org.apache.openjpa.jdbc.sql.Select;
+import org.apache.openjpa.kernel.exps.ExpressionVisitor;
 import org.apache.openjpa.meta.ClassMetaData;
 
 /**
@@ -37,7 +38,7 @@ import org.apache.openjpa.meta.ClassMetaData;
  * @author Abe White
  */
 class Variable
-    implements Val {
+    extends AbstractVal {
 
     private final String _name;
     private final Class _type;
@@ -143,10 +144,6 @@ class Variable
         return null;
     }
 
-    public boolean hasVariable(Variable var) {
-        return this == var;
-    }
-
     public void calculateValue(Select sel, JDBCStore store,
         Object[] params, Val other, JDBCFetchConfiguration fetch) {
         if (_path != null)
@@ -185,4 +182,11 @@ class Variable
     public void appendIsNotNull(SQLBuffer sql, Select sel,
         JDBCStore store, Object[] params, JDBCFetchConfiguration fetch) {
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        if (_path != null)
+            _path.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java

@@ -23,6 +23,7 @@ import java.util.Map;
 
 import org.apache.commons.collections.map.LinkedMap;
 import org.apache.openjpa.conf.OpenJPAConfiguration;
+import org.apache.openjpa.kernel.exps.AbstractExpressionVisitor;
 import org.apache.openjpa.kernel.exps.AggregateListener;
 import org.apache.openjpa.kernel.exps.ExpressionFactory;
 import org.apache.openjpa.kernel.exps.ExpressionParser;
@@ -396,16 +397,16 @@ public class ExpressionStoreQuery
             if (_exps[0].projections.length == 0)
                 _projTypes = StoreQuery.EMPTY_CLASSES;
             else {
+                AssertNoVariablesExpressionVisitor novars = new
+                    AssertNoVariablesExpressionVisitor(q.getContext());
                 _projTypes = new Class[_exps[0].projections.length];
                 for (int i = 0; i < _exps[0].projections.length; i++) {
                     _projTypes[i] = _exps[0].projections[i].getType();
                     assertNotContainer(_exps[0].projections[i], q);
-                    assertNotVariable((Val) _exps[0].projections[i],
+                    _exps[0].projections[i].acceptVisit(novars);
-                        q.getContext());
                 }
                 for (int i = 0; i < _exps[0].grouping.length; i++)
-                    assertNotVariable((Val) _exps[0].grouping[i],
+                    _exps[0].grouping[i].acceptVisit(novars);
-                        q.getContext());
             }
         }
 
@@ -413,17 +414,6 @@ public class ExpressionStoreQuery
             return _exps;
         }
 
-        /**
-         * We can't handle in-memory projections or grouping that uses
-         * variables.
-         */
-        private static void assertNotVariable(Val val, QueryContext ctx) {
-            // we can't handle in-mem results that use variables
-            if (val.hasVariables())
-                throw new UnsupportedException(_loc.get("inmem-agg-proj-var",
-                    ctx.getCandidateType(), ctx.getQueryString()));
-        }
-
         public ResultObjectProvider executeQuery(StoreQuery q,
             Object[] params, boolean lrs, long startIdx, long endIdx) {
             // execute in memory for candidate collection;
@@ -517,6 +507,26 @@ public class ExpressionStoreQuery
         public Class[] getProjectionTypes(StoreQuery q) {
             return _projTypes;
         }
+
+        /**
+         * Throws an exception if a variable is found.
+         */
+        private static class AssertNoVariablesExpressionVisitor 
+            extends AbstractExpressionVisitor {
+
+            private final QueryContext _ctx;
+
+            public AssertNoVariablesExpressionVisitor(QueryContext ctx) {
+                _ctx = ctx;
+            }
+
+            public void enter(Value val) {
+                if (!val.isVariable())
+                    return;
+                throw new UnsupportedException(_loc.get("inmem-agg-proj-var", 
+                    _ctx.getCandidateType(), _ctx.getQueryString()));
+            }
+        }
     }
 
     /**

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/SingleFieldManager.java

@@ -90,8 +90,8 @@ class SingleFieldManager
                 if (proxy == null) {
                     proxy = (Proxy) _sm.newFieldProxy(field);
                     if (objval != null)
-                        ((Calendar) proxy)
+                        ((Calendar) proxy).setTime(((Calendar) objval).
-                            .setTime(((Calendar) objval).getTime());
+                            getTime());
                     ret = true;
                 }
                 break;

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AbstractExpressionVisitor.java

@@ -0,0 +1,24 @@
+package org.apache.openjpa.kernel.exps;
+
+
+/**
+ * No-op abstract visitor meant for easy extension.
+ *
+ * @author Abe White
+ * @nojavadoc
+ */
+public abstract class AbstractExpressionVisitor 
+    implements ExpressionVisitor {
+
+    public void enter(Expression exp) {
+    }
+
+    public void exit(Expression exp) {
+    }
+
+    public void enter(Value val) {
+    }
+
+    public void exit(Value val) {
+    }
+}

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Aggregate.java

@@ -43,10 +43,6 @@ class Aggregate
         _arg = arg;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return _listener.getType(getArgTypes());
     }
@@ -54,10 +50,6 @@ class Aggregate
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return (_arg == null) ? false : _arg.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         if (candidate == null)
@@ -88,4 +80,11 @@ class Aggregate
             return ((Args) _arg).getTypes();
         return new Class[]{ _arg.getType() };
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        if (_arg != null)
+            _arg.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AggregateVal.java

@@ -42,10 +42,6 @@ abstract class AggregateVal
         _val = val;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return getType(_val.getType());
     }
@@ -53,10 +49,6 @@ abstract class AggregateVal
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         if (candidate == null)
@@ -85,4 +77,10 @@ abstract class AggregateVal
      * Aggregate the given values.
      */
     protected abstract Object operate(Collection os, Class c);
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/AndExpression.java

@@ -57,5 +57,12 @@ class AndExpression
         return _exp1.evaluate(candidates, ctx, params)
             && _exp2.evaluate(candidates, ctx, params);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _exp1.acceptVisit(visitor);
+        _exp2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Args.java

@@ -49,10 +49,6 @@ class Args
         return (Value[]) _args.toArray(new Value[_args.size()]);
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return Object[].class;
     }
@@ -67,13 +63,6 @@ class Args
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        for (int i = 0; i < _args.size(); i++)
-            if (((Val) _args.get(i)).hasVariables())
-                return true;
-        return false;
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object[] vals = new Object[_args.size()];
@@ -81,4 +70,11 @@ class Args
             vals[i] = ((Val) _args.get(i)).eval(candidate, orig, ctx, params);
         return vals;
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        for (int i = 0; i < _args.size(); i++)
+            ((Val) _args.get(i)).acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/BindVariableExpression.java

@@ -79,5 +79,12 @@ class BindVariableExpression
         Object obj = candidates.iterator().next();
         return eval(obj, obj, ctx, params);
 	}
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _var.acceptVisit(visitor);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/BoundVariable.java

@@ -63,10 +63,6 @@ class BoundVariable
         _type = type;
     }
 
-    public boolean hasVariables() {
-        return true;
-    }
-
     /**
      * Cast this value to the given type.
      */

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CandidatePath.java

@@ -48,10 +48,6 @@ class CandidatePath
         _actions.add(new Traversal(field, nullTraversal));
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         if (_actions == null)
             return getCandidateType();
@@ -73,10 +69,6 @@ class CandidatePath
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return false;
-    }
-
     public FieldMetaData last() {
         if (_actions == null)
             return null;

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Cast.java

@@ -41,10 +41,6 @@ class Cast
         _cast = cast;
     }
 
-    public boolean isVariable() {
-        return _val.isVariable();
-    }
-
     public Class getType() {
         return _cast;
     }
@@ -52,10 +48,6 @@ class Cast
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         return Filters.convert(_val.eval(candidate, orig, ctx, params), _cast);
@@ -72,4 +64,10 @@ class Cast
             casts.add(Filters.convert(itr.next(), _cast));
         return casts;
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CompareExpression.java

@@ -72,4 +72,11 @@ abstract class CompareExpression
      * Compare the two values.
      */
     protected abstract boolean compare(Object o1, Object o2);
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Concat.java

@@ -37,10 +37,6 @@ class Concat
         _args = args;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return String.class;
     }
@@ -48,10 +44,6 @@ class Concat
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables() || _args.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object str = _val.eval(candidate, orig, ctx, params);
@@ -66,5 +58,12 @@ class Concat
 
         return cat.toString();
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        _args.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ContainsExpression.java

@@ -70,5 +70,12 @@ class ContainsExpression
     protected Collection getCollection(Object obj) {
         return (Collection) obj;
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/CurrentDate.java

@@ -27,14 +27,6 @@ import org.apache.openjpa.kernel.StoreContext;
 class CurrentDate
     extends Val {
 
-    public boolean hasVariables() {
-        return false;
-    }
-
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return Date.class;
     }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Distinct.java

@@ -38,10 +38,6 @@ class Distinct
         _val = val;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return Collection.class;
     }
@@ -49,18 +45,12 @@ class Distinct
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         if (candidate == null)
             candidate = Collections.EMPTY_LIST;
-        
         Collection arg = candidate instanceof Collection
             ? (Collection) candidate : Collections.singleton(candidate);
-
         return eval(arg, orig, ctx, params).iterator().next();
     }
 
@@ -69,5 +59,11 @@ class Distinct
         Collection args = _val.eval(candidates, orig, ctx, params);
         return new HashSet(args);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Exp.java

@@ -71,4 +71,9 @@ class Exp
         Object[] params) {
         return true;
 	}
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Expression.java

@@ -16,11 +16,15 @@
 package org.apache.openjpa.kernel.exps;
 
 /**
- * Marker interface for a set of conditions that must be met for the query
+ * Interface for a set of conditions that must be met for the query
  * to be true.
  *
  * @author Abe White
  */
 public interface Expression {
 
+    /**
+     * Accept a visit from a tree visitor.
+     */
+    public void acceptVisit(ExpressionVisitor visitor);
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ExpressionVisitor.java

@@ -0,0 +1,31 @@
+package org.apache.openjpa.kernel.exps;
+
+
+/**
+ * Visits nodes of a query expression tree.
+ *
+ * @author Abe White
+ */
+public interface ExpressionVisitor {
+
+    /**
+     * Enter an expression.  The expression will then invoke visits on its
+     * components.
+     */
+    public void enter(Expression exp);
+
+    /**
+     * Leave an expression.
+     */
+    public void exit(Expression exp);
+
+    /**
+     * Enter a value.  The value will then invoke visits on its components.
+     */
+    public void enter(Value val);
+
+    /**
+     * Leave a value.
+     */
+    public void exit(Value val);
+}

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Extension.java

@@ -39,10 +39,6 @@ class Extension
         _arg = arg;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         Class targetClass = (_target == null) ? null : _target.getType();
         return _listener.getType(targetClass, getArgTypes());
@@ -51,11 +47,6 @@ class Extension
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _target != null && _target.hasVariables()
-            || _arg != null && _arg.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object target = null;
@@ -86,5 +77,14 @@ class Extension
             return (Object[]) arg;
         return new Object[]{ arg };
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        if (_target != null)
+            _target.acceptVisit(visitor);
+        if (_arg != null)
+            _arg.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/GetObjectId.java

@@ -34,10 +34,6 @@ class GetObjectId
         _val = val;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return Object.class;
     }
@@ -45,12 +41,14 @@ class GetObjectId
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         return ctx.getObjectId(_val.eval(candidate, orig, ctx, params));
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/IndexOf.java

@@ -38,10 +38,6 @@ class IndexOf
         _args = args;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return int.class;
     }
@@ -49,10 +45,6 @@ class IndexOf
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables() || _args.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object str = _val.eval(candidate, orig, ctx, params);
@@ -66,4 +58,11 @@ class IndexOf
             idx = str.toString().indexOf(arg.toString());
         return Numbers.valueOf(idx);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        _args.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/InstanceofExpression.java

@@ -57,4 +57,10 @@ class InstanceofExpression
         Object o = (c == null || c.isEmpty()) ? null : c.iterator().next();
         return _cls.isInstance(o);
 	}
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/IsEmptyExpression.java

@@ -63,5 +63,11 @@ class IsEmptyExpression
             return ((Map) obj).isEmpty();
         return false;
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Lit.java

@@ -38,10 +38,6 @@ class Lit
         _ptype = ptype;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Object getValue() {
         return _val;
     }
@@ -66,10 +62,6 @@ class Lit
         _val = Filters.convert(_val, type);
     }
 
-    public boolean hasVariables() {
-        return false;
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         return _val;

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/MathVal.java

@@ -37,10 +37,6 @@ abstract class MathVal
         _val2 = val2;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         Class c1 = _val1.getType();
         Class c2 = _val2.getType();
@@ -50,10 +46,6 @@ abstract class MathVal
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val1.hasVariables() || _val2.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object o1 = _val1.eval(candidate, orig, ctx, params);
@@ -66,4 +58,11 @@ abstract class MathVal
      */
     protected abstract Object operate(Object o1, Class c1, Object o2,
         Class c2);
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val1.acceptVisit(visitor);
+        _val2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/NotExpression.java

@@ -45,5 +45,11 @@ class NotExpression
         Object[] params) {
         return !_exp.evaluate(candidates, ctx, params);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _exp.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Null.java

@@ -25,14 +25,6 @@ import org.apache.openjpa.kernel.StoreContext;
 class Null
     extends Val {
 
-    public boolean hasVariables() {
-        return false;
-    }
-
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return Object.class;
     }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/OrExpression.java

@@ -49,5 +49,12 @@ class OrExpression
         return _exp1.evaluate(candidates, ctx, params)
             || _exp2.evaluate(candidates, ctx, params);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _exp1.acceptVisit(visitor);
+        _exp2.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Param.java

@@ -42,10 +42,6 @@ class Param
         return _name;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return _type;
     }
@@ -54,10 +50,6 @@ class Param
         _type = type;
     }
 
-    public boolean hasVariables() {
-        return false;
-    }
-
     public void setIndex(int index) {
         _index = index;
     }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Path.java

@@ -19,6 +19,7 @@ import org.apache.openjpa.meta.FieldMetaData;
 
 /**
  * A path represents a traversal into fields of a candidate object.
+ * Equivalent paths should compare equal.
  *
  * @author Abe White
  */

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/StringLength.java

@@ -36,10 +36,6 @@ class StringLength
         _val = val;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         if (_cast != null)
             return _cast;
@@ -50,10 +46,6 @@ class StringLength
         _cast = type;
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object eval = _val.eval(candidate, orig, ctx, params);
@@ -62,5 +54,11 @@ class StringLength
 
         return Numbers.valueOf(eval.toString().length());
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/SubQ.java

@@ -46,10 +46,6 @@ class SubQ
         long endIdx) {
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return _type;
     }
@@ -58,10 +54,6 @@ class SubQ
         _type = type;
     }
 
-    public boolean hasVariables() {
-        return false;
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         throw new UnsupportedException(_loc.get("in-mem-subquery"));

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Substring.java

@@ -37,10 +37,6 @@ class Substring
         _args = args;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return String.class;
     }
@@ -48,10 +44,6 @@ class Substring
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables() || _args.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object str = _val.eval(candidate, orig, ctx, params);
@@ -63,4 +55,11 @@ class Substring
         }
         return str.toString().substring(((Number) arg).intValue());
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        _args.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/This.java

@@ -25,14 +25,6 @@ import org.apache.openjpa.kernel.StoreContext;
 class This
     extends Val {
 
-    public boolean hasVariables() {
-        return false;
-    }
-
-    public boolean isVariable() {
-        return false;
-    }
-
     public void setImplicitType(Class type) {
     }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ToLowerCase.java

@@ -34,10 +34,6 @@ class ToLowerCase
         _val = val;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return String.class;
     }
@@ -45,13 +41,14 @@ class ToLowerCase
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
-        return _val.eval(candidate, orig, ctx, params).toString().
+        return _val.eval(candidate, orig, ctx, params).toString().toLowerCase();
-            toLowerCase();
+    }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
     }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ToUpperCase.java

@@ -34,10 +34,6 @@ class ToUpperCase
         _val = val;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return String.class;
     }
@@ -45,13 +41,15 @@ class ToUpperCase
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         return _val.eval(candidate, orig, ctx, params).toString().
             toUpperCase();
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Trim.java

@@ -38,10 +38,6 @@ class Trim
         _where = where;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return String.class;
     }
@@ -49,20 +45,14 @@ class Trim
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object eval = _val.eval(candidate, orig, ctx, params);
-
         if (eval == null)
             return null;
 
         String toTrim = _trimChar.eval(candidate, orig, ctx, params).
             toString();
-
         String str = eval.toString();
 
         // null indicates both, TRUE indicates leading
@@ -76,8 +66,14 @@ class Trim
             while (str.endsWith(toTrim))
                 str = str.substring(0, str.length() - toTrim.length());
         }
-
         return str;
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        _trimChar.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/UnaryMathVal.java

@@ -34,10 +34,6 @@ abstract class UnaryMathVal
         _val = val;
     }
 
-    public boolean isVariable() {
-        return false;
-    }
-
     public Class getType() {
         return getType(_val.getType());
     }
@@ -45,10 +41,6 @@ abstract class UnaryMathVal
     public void setImplicitType(Class type) {
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         Object o1 = _val.eval(candidate, orig, ctx, params);
@@ -65,4 +57,10 @@ abstract class UnaryMathVal
      * Return the result of this mathematical operation on the given value.
      */
     protected abstract Object operate(Object o, Class c);
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/UnboundVariable.java

@@ -58,10 +58,6 @@ class UnboundVariable
         _val = value;
     }
 
-    public boolean hasVariables() {
-        return true;
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         return _val;

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Val.java

@@ -66,11 +66,6 @@ public abstract class Val
         }
     }
 
-    /**
-     * Return true if this value involves the use of variables.
-     */
-    public abstract boolean hasVariables();
-
     /**
      * Return this value for the given candidate.
      */
@@ -103,4 +98,13 @@ public abstract class Val
     public void setMetaData(ClassMetaData meta) {
         _meta = meta;
 	}
+
+    public boolean isVariable() {
+        return false;
+    }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ValExpression.java

@@ -48,4 +48,10 @@ class ValExpression
         Object o = (c == null || c.isEmpty()) ? null : c.iterator().next();
         return o != null && ((Boolean) o).booleanValue();
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Value.java

@@ -52,4 +52,9 @@ public interface Value {
      * Associate a persistent type with this value.
      */
     public void setMetaData(ClassMetaData meta);
+
+    /**
+     * Accept a visit from a tree visitor.
+     */
+    public void acceptVisit(ExpressionVisitor visitor);
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/ValuePath.java

@@ -38,15 +38,17 @@ class ValuePath
         return _val.getType();
     }
 
-    public boolean hasVariables() {
-        return _val.hasVariables();
-    }
-
     protected Object eval(Object candidate, Object orig,
         StoreContext ctx, Object[] params) {
         // evaluate with the value's value
         return super.eval(_val.eval(candidate, orig, ctx, params), orig,
             ctx, params);
     }
+
+    public void acceptVisit(ExpressionVisitor visitor) {
+        visitor.enter(this);
+        _val.acceptVisit(visitor);
+        visitor.exit(this);
+    }
 }