OPENJPA-806: Interface changes (merge Selectable to Visitable) + String Functions (Concat etc) + Detect recursion

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

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/TestCriteria.java

@@ -19,12 +19,15 @@
 
 package org.apache.openjpa.persistence.criteria;
 
+import javax.persistence.CaseExpression;
 import javax.persistence.DomainObject;
+import javax.persistence.Expression;
 import javax.persistence.QueryBuilder;
 import javax.persistence.QueryDefinition;
 import javax.persistence.SelectItem;
 
 import org.apache.openjpa.persistence.query.AbstractDomainObject;
+import org.apache.openjpa.persistence.query.QueryBuilderImpl;
 import org.apache.openjpa.persistence.query.QueryDefinitionImpl;
 import org.apache.openjpa.persistence.test.SingleEMFTestCase;
 
@@ -45,7 +48,7 @@ import org.apache.openjpa.persistence.test.SingleEMFTestCase;
  *
  */
 public class TestCriteria extends SingleEMFTestCase {
-	protected QueryBuilder qb; 
+	protected QueryBuilderImpl qb; 
 	protected StringComparison comparator = new StringComparison();
 	
 	public void setUp() {
@@ -53,7 +56,7 @@ public class TestCriteria extends SingleEMFTestCase {
 			Department.class, Employee.class, Exempt.class, Item.class,
 			Manager.class, Person.class, VideoStore.class, Order.class, 
 			Customer.class);
-		qb = emf.getQueryBuilder();
+		qb = (QueryBuilderImpl)emf.getQueryBuilder();
 	}
 	
 	public void tearDown() {
@@ -61,19 +64,12 @@ public class TestCriteria extends SingleEMFTestCase {
 	}
 	
 	void compare(String s, QueryDefinition q) {
-		String actual = ((QueryDefinitionImpl)q).toJPQL();
+		String actual = qb.toJPQL(q);
 		if (!comparator.compare(s,actual)) {
 			fail("\r\nExpected: [" + s + "]\r\nActual  : [" + actual + "]");
 		}
 	}
 	
-	void compare(String s, DomainObject q) {
-		String actual = ((AbstractDomainObject)q).getOwner().toJPQL();
-		if (!comparator.compare(s,actual)) {
-			fail("\r\nExpected: [" + s + "]\r\nActual  : [" + actual + "]");
-		}
-	}
-
 	public void testMultipleDomainOfSameClass() {
 		DomainObject o1 = qb.createQueryDefinition(Order.class);
 		DomainObject o2 = o1.addRoot(Order.class);
@@ -353,11 +349,11 @@ public class TestCriteria extends SingleEMFTestCase {
 	DomainObject address = customer.join("address");
 	q.where(address.get("state").equal("CA"))
 	.select(order.get("quantity"), address.get("zipcode"))
-	.orderBy(order.get("quantity"), address.get("zipcode"));
+	.orderBy(order.get("quantity").desc(), address.get("zipcode"));
 	String jpql = "SELECT o.quantity, a.zipcode"
 				+ " FROM Customer c JOIN c.orders o JOIN c.address a"
 				+ " WHERE a.state = 'CA'"
-				+ " ORDER BY o.quantity, a.zipcode";
+				+ " ORDER BY o.quantity DESC, a.zipcode";
 	compare(jpql, q);
 	}
 	
@@ -370,10 +366,10 @@ public class TestCriteria extends SingleEMFTestCase {
 	.and(a.get("county").equal("Santa Clara")))
 	.orderBy(o.get("quantity"), taxedCost, a.get("zipcode"));
 	
-	String jpql = "SELECT o.quantity, o.cost*1.08, a.zipcode" 
+	String jpql = "SELECT o.quantity, o.cost*1.08 as o2, a.zipcode" 
 				+ " FROM Order o JOIN o.customer c JOIN c.address a"
 				+ " WHERE a.state = 'CA' AND a.county = 'Santa Clara'"
-				+ " ORDER BY o.quantity, o.cost*1.08, a.zipcode";
+				+ " ORDER BY o.quantity, o2, a.zipcode";
 	
 	compare(jpql, o);
 	}
@@ -410,4 +406,54 @@ public class TestCriteria extends SingleEMFTestCase {
 		compare(jpql, customer);
 	}
 	
+	public void testTypeList() {
+		DomainObject q = qb.createQueryDefinition(Employee.class);
+		q.where(q.type().in(Exempt.class, Contractor.class));
+		
+		String jpql = "SELECT e "
+			+ " FROM Employee e"
+			+ " WHERE TYPE(e) IN (Exempt, Contractor)";
+		compare(jpql, q);
+	}
+	
+	public void testStringList() {
+		DomainObject q = qb.createQueryDefinition(Employee.class);
+		q.where(q.get("country").in("USA", "UK", "France"));
+		
+		String jpql = "SELECT e "
+			+ " FROM Employee e"
+			+ " WHERE e.country IN ('USA', 'UK', 'France')";
+		compare(jpql, q);
+	}
+	
+	public void testConcat() {
+		DomainObject e = qb.createQueryDefinition(Employee.class);
+		DomainObject f = e.join("frequentFlierPlan");
+		Expression c = 
+		e.generalCase().when(f.get("annualMiles").greaterThan(50000)).then("Platinum")
+		               .when(f.get("annualMiles").greaterThan(25000)).then("Gold")
+		               .elseCase("");
+		e.select(e.get("name"), f.get("name"), e.concat(c,e.literal("Frequent Flyer")));
+		
+		String jpql = "SELECT e.name, f.name, CONCAT(" 
+			+ " CASE WHEN f.annualMiles > 50000 THEN 'Platinum'" 
+			+ "      WHEN f.annualMiles > 25000 THEN 'Gold'" 
+			+ " ELSE '' END, 'Frequent Flyer')" 
+			+ "FROM Employee e JOIN e.frequentFlierPlan f";
+			
+		compare(jpql, e);
+	}
+	
+	public void testRecursiveDefinitionIsNotAllowed() {
+		DomainObject q = qb.createQueryDefinition(Customer.class);
+		q.where(q.exists().and(q.get("name").equal("wrong")));
+		
+		try {
+			compare("?", q);
+			fail();
+		} catch (RuntimeException e) {
+			// good
+		}
+	}
+	
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java

@@ -343,7 +343,7 @@ public class EntityManagerFactoryImpl
     }
 
     public QueryBuilder getQueryBuilder() {
-    	return new QueryBuilderImpl();
+    	return new QueryBuilderImpl(this);
     }
 
     public Set<String> getSupportedProperties() {

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerImpl.java

@@ -66,6 +66,7 @@ import org.apache.openjpa.meta.ClassMetaData;
 import org.apache.openjpa.meta.FieldMetaData;
 import org.apache.openjpa.meta.QueryMetaData;
 import org.apache.openjpa.meta.SequenceMetaData;
+import org.apache.openjpa.persistence.query.QueryBuilderImpl;
 import org.apache.openjpa.util.Exceptions;
 import org.apache.openjpa.util.ImplHelper;
 import org.apache.openjpa.util.RuntimeExceptionTranslator;
@@ -1395,12 +1396,14 @@ public class EntityManagerImpl
             "JPA 2.0 - Method not yet implemented");
     }
 
-    public <T> T find(Class<T> entityClass, Object primaryKey, LockModeType lockMode) {
+    public <T> T find(Class<T> entityClass, Object primaryKey, 
+    	LockModeType lockMode) {
         throw new UnsupportedOperationException(
             "JPA 2.0 - Method not yet implemented");
     }
 
-    public <T> T find(Class<T> entityClass, Object primaryKey, LockModeType lockMode, Map<String, Object> properties) {
+    public <T> T find(Class<T> entityClass, Object primaryKey, 
+    	LockModeType lockMode, Map<String, Object> properties) {
         throw new UnsupportedOperationException(
             "JPA 2.0 - Method not yet implemented");
     }
@@ -1411,8 +1414,7 @@ public class EntityManagerImpl
     }
 
     public QueryBuilder getQueryBuilder() {
-        throw new UnsupportedOperationException(
+        return new QueryBuilderImpl(_emf);
-            "JPA 2.0 - Method not yet implemented");
     }
 
     public Set<String> getSupportedProperties() {
@@ -1420,7 +1422,8 @@ public class EntityManagerImpl
             "JPA 2.0 - Method not yet implemented");
     }
 
-    public void lock(Object entity, LockModeType lockMode, Map<String, Object> properties) {
+    public void lock(Object entity, LockModeType lockMode, Map<String, 
+    	Object> properties) {
         throw new UnsupportedOperationException(
             "JPA 2.0 - Method not yet implemented");
     }
@@ -1430,7 +1433,8 @@ public class EntityManagerImpl
             "JPA 2.0 - Method not yet implemented");
     }
 
-    public void refresh(Object entity, LockModeType lockMode, Map<String, Object> properties) {
+    public void refresh(Object entity, LockModeType lockMode, Map<String, 
+    	Object> properties) {
         throw new UnsupportedOperationException(
             "JPA 2.0 - Method not yet implemented");
     }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AbstractDomainObject.java

@@ -390,6 +390,5 @@ public abstract class AbstractDomainObject extends AbstractPath
 				join.setAlias(ctx);
 	}
 	
-	abstract public String asJoinable(AliasContext ctx);
 
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AbstractPath.java

@@ -20,7 +20,6 @@ package org.apache.openjpa.persistence.query;
 
 import javax.persistence.Aggregate;
 import javax.persistence.Expression;
-import javax.persistence.OrderByItem;
 import javax.persistence.PathExpression;
 import javax.persistence.Predicate;
 
@@ -113,12 +112,4 @@ abstract class AbstractPath extends ExpressionImpl implements
 	public Expression type() {
 		return new TypeExpression(this);
 	}
-	
-	public OrderByItem asc() {
-		return new OrderableItem(this, Boolean.TRUE);
-	}
-
-	public OrderByItem desc() {
-		return new OrderableItem(this, Boolean.FALSE);
-	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AbstractVisitable.java

@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.openjpa.persistence.query;
+
+/**
+ * An abstract implementation that throws UnsupportedOperationException on
+ * every method.
+ * 
+ * @author Pinaki Poddar
+ *
+ */
+abstract class AbstractVisitable implements Visitable {
+
+	public String asExpression(AliasContext ctx) {
+		throw new UnsupportedOperationException(this.getClass().getName());
+	}
+
+	public String asProjection(AliasContext ctx) {
+		throw new UnsupportedOperationException(this.getClass().getName());
+	}
+
+	public String getAliasHint(AliasContext ctx) {
+		throw new UnsupportedOperationException(this.getClass().getName());
+	}
+
+	public void setAlias(AliasContext ctx) {
+		throw new UnsupportedOperationException(this.getClass().getName());
+	}
+	
+	public String asJoinable(AliasContext ctx) {
+		throw new UnsupportedOperationException(this.getClass().getName());
+	}
+
+}

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AliasContext.java

@@ -20,12 +20,21 @@ package org.apache.openjpa.persistence.query;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Stack;
 
 import javax.persistence.Expression;
 
+import org.apache.openjpa.meta.MetaDataRepository;
+
 class AliasContext {
+	private Stack<Object> _operating = new Stack<Object>();
 	private Map<ExpressionImpl, String> _aliases = 
 		new HashMap<ExpressionImpl, String>();
+	private final MetaDataRepository _repos;
+	
+	public AliasContext(MetaDataRepository repos) {
+		_repos = repos;
+	}
 	
 	/**
 	 * Sets alias for the given Expression or gets the alias if the given
@@ -40,10 +49,10 @@ class AliasContext {
 		String alias = _aliases.get(path);
 		if (alias != null)
 			return alias;
-		alias = path.getAliasHint().substring(0,1).toLowerCase();
+		alias = path.getAliasHint(this).substring(0,1).toLowerCase();
 		int i = 2;
 		while (_aliases.containsValue(alias)) {
-			alias = alias + i;
+			alias = alias.substring(0,1) + i;
 			i++;
 		}
 		_aliases.put(path, alias);
@@ -57,4 +66,16 @@ class AliasContext {
 	public boolean hasAlias(Expression path) {
 		return _aliases.containsKey(path);
 	}
+	
+	public AliasContext push(Object e) {
+		if (_operating.contains(e))
+			throw new RuntimeException(e + " is already in this ctx");
+		_operating.add(e);
+		return this;
+	}
+	
+	public String getEntityName(Class cls) {
+		return cls.getSimpleName();
+//		return _repos.getMetaData(cls, null, true).getTypeAlias();
+	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ArrayExpression.java

@@ -0,0 +1,28 @@
+package org.apache.openjpa.persistence.query;
+
+import java.util.Arrays;
+
+public class ArrayExpression extends ExpressionImpl {
+	private final Object[] _values;
+	
+	public ArrayExpression(Object[] values) {
+		_values = values;
+	}
+
+	@Override
+	public String asExpression(AliasContext ctx) {
+		StringBuffer tmp = new StringBuffer("(");
+		for (int i = 0; i < _values.length; i++) {
+			tmp.append(JPQLHelper.toJPQL(ctx, _values[i]))
+			   .append(i == _values.length-1 ? "" : ", ");
+		}
+		tmp.append(")");
+		return tmp.toString();
+	}
+
+	@Override
+	public String asProjection(AliasContext ctx) {
+		return asExpression(ctx);
+	}
+
+}

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/BinaryExpressionPredicate.java

@@ -27,7 +27,8 @@ import javax.persistence.Predicate;
  * @author Pinaki Poddar
  *
  */
-class BinaryExpressionPredicate implements Predicate, Visitable {
+class BinaryExpressionPredicate extends AbstractVisitable 
+    implements Predicate, Visitable {
 	protected final Expression _e1;
 	protected final Expression _e2;
 	protected final BinaryConditionalOperator   _op;

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/BinaryFunctionalOperator.java

@@ -34,9 +34,7 @@ public enum BinaryFunctionalOperator {
 	MOD("MOD"),
 	PLUS("+"),
 	RANGE(","),
-	SUBSTR("SUBSTR"),
+	TIMES("*");
-	TIMES("*"),
-	TRIM("TRIM");
 	
 	private final String _symbol;
 	

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/BinaryOperatorExpression.java

@@ -56,7 +56,9 @@ public class BinaryOperatorExpression extends ExpressionImpl {
 	}
 	
 	public String asProjection(AliasContext ctx) {
-		return ((Selectable)_e1).asProjection(ctx) + _op 
+		return ((Visitable)_e1).asProjection(ctx) + _op 
-		     + ((Selectable)_e2).asProjection(ctx);
+		     + ((Visitable)_e2).asProjection(ctx) 
+		     + (ctx.hasAlias(this) ? " as " + ctx.getAlias(this) : "");
+
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ConcatExpression.java

@@ -18,15 +18,15 @@
  */
 package org.apache.openjpa.persistence.query;
 
-import javax.persistence.SelectItem;
 
 /**
- * An item that can be selected in a query.
+ * Denotes CONCAT(e1,e2,..) Expression.
  * 
  * @author Pinaki Poddar
  *
  */
-public interface Selectable extends SelectItem {
+public class ConcatExpression extends UnaryOperatorExpression {
-	String asProjection(AliasContext ctx);
+	public ConcatExpression(ArrayExpression op) {
-	void setAlias(AliasContext ctx);
+		super(op, UnaryFunctionalOperator.CONCAT);
+	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ConstantExpression.java

@@ -33,28 +33,13 @@ class ConstantExpression extends ExpressionImpl {
 		_value = value;
 	}
 	
-	public Object getValue() {
-		return _value;
-	}
-	
 	@Override
 	public String asExpression(AliasContext ctx) {
-		if (_value.getClass().isArray()) {
+		return JPQLHelper.toJPQL(ctx, _value);
-			return Arrays.asList((Object[])_value).toString();
-		}
-		return quoted(_value);
 	}
 	
 	@Override
 	public String asProjection(AliasContext ctx) {
 		return asExpression(ctx);
 	}
-	
-	String quoted(Object o) {
-		if (o instanceof String)
-			return "'" + o + "'";
-		if (o instanceof Class)
-			return ((Class)o).getSimpleName();
-		return o.toString();
-	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ElseExpression.java

@@ -36,28 +36,14 @@ public class ElseExpression extends ExpressionImpl {
 	@Override
 	public String asExpression(AliasContext ctx) {
 		return _caseClause.toJPQL(ctx) 
-		    + " ELSE " + toJPQL(ctx, _elseClause) 
+		    + " ELSE " + JPQLHelper.toJPQL(ctx, _elseClause) 
 		    + " END ";
 	}
 	
 	@Override
 	public String asProjection(AliasContext ctx) {
 		return _caseClause.toJPQL(ctx) 
-		     + " ELSE " + toJPQL(ctx, _elseClause) 
+		     + " ELSE " + JPQLHelper.toJPQL(ctx, _elseClause) 
 		     + " END ";
 	}
-		
-	String toJPQL(AliasContext ctx, Object o) {
-		if (o instanceof Visitable) {
-			return ((Visitable)o).asExpression(ctx);
-		}
-		return o.toString();
-	}
-	
-	String asProjection(AliasContext ctx, Object o) {
-		if (o instanceof Selectable) {
-			return ((Selectable)o).asProjection(ctx);
-		}
-		return o.toString();
-	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ExistsExpression.java

@@ -26,7 +26,8 @@ package org.apache.openjpa.persistence.query;
  */
 public class ExistsExpression extends UnaryExpressionPredicate {
 	public ExistsExpression(QueryDefinitionImpl op) {
-		super(op, UnaryConditionalOperator.EXISTS, UnaryConditionalOperator.EXISTS_NOT);
+		super(op, UnaryConditionalOperator.EXISTS, 
+			UnaryConditionalOperator.EXISTS_NOT);
 	}
 
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ExpressionImpl.java

@@ -18,6 +18,7 @@
  */
 package org.apache.openjpa.persistence.query;
 
+import java.io.Serializable;
 import java.util.Calendar;
 import java.util.Date;
 
@@ -36,18 +37,19 @@ import javax.persistence.TrimSpec;
  * @author Pinaki Poddar
  *
  */
-abstract class ExpressionImpl implements Expression, Selectable, Visitable {
+abstract class ExpressionImpl extends AbstractVisitable 
+   implements Expression, Visitable {
 	
 	public Expression abs() {
 		return new AbsExpression(this);
 	}
 
 	public Expression concat(String... str) {
-		throw new UnsupportedOperationException();
+		return new ConcatExpression(new ArrayExpression(str));
 	}
 
 	public Expression concat(Expression... str) {
-		throw new UnsupportedOperationException();
+		return new ConcatExpression(new ArrayExpression(str));
 	}
 
 	public Expression dividedBy(Number num) {
@@ -59,27 +61,23 @@ abstract class ExpressionImpl implements Expression, Selectable, Visitable {
 	}
 
 	public Predicate in(String... strings) {
-		return new InExpression(this, new ConstantExpression(strings));
+		return new InExpression(this, new ArrayExpression(strings));
 	}
 
 	public Predicate in(Number... nums) {
-		return new InExpression(this, 
+		return new InExpression(this, new ArrayExpression(nums));
-				new ConstantExpression(nums));
 	}
 
 	public Predicate in(Enum<?>... enums) {
-		return new InExpression(this, 
+		return new InExpression(this, new ArrayExpression(enums));
-				new ConstantExpression(enums));
 	}
 
 	public Predicate in(Class... classes) {
-		return new InExpression(this, 
+		return new InExpression(this, new ArrayExpression(classes));
-				new ConstantExpression(classes));
 	}
 
 	public Predicate in(Expression... params) {
-		return new InExpression(this, 
+		return new InExpression(this, new ArrayExpression(params));
-				new ConstantExpression(params));
 	}
 
 	public Predicate in(Subquery subquery) {
@@ -95,7 +93,7 @@ abstract class ExpressionImpl implements Expression, Selectable, Visitable {
 	}
 
 	public Expression locate(String str) {
-		return locate(str, 0);
+		return new LocateExpression(this, str, 0);
 	}
 
 	public Expression locate(Expression expr) {
@@ -103,11 +101,11 @@ abstract class ExpressionImpl implements Expression, Selectable, Visitable {
 	}
 
 	public Expression locate(String str, int position) {
-		return new LocateExpression(this, new ConstantExpression(str), position);
+		return new LocateExpression(this, str, position);
 	}
 
 	public Expression locate(String str, Expression position) {
-		return new LocateExpression(this, new ConstantExpression(str), position);
+		return new LocateExpression(this, str, position);
 	}
 
 	public Expression locate(Expression str, int position) {
@@ -215,15 +213,15 @@ abstract class ExpressionImpl implements Expression, Selectable, Visitable {
 	}
 
 	public Expression upper() {
-		return new ToUpperExpression(this);
+		return new UpperExpression(this);
 	}
 
 	public OrderByItem asc() {
-		throw new UnsupportedOperationException(this.toString());
+		return new OrderableItem(this, true);
 	}
 
 	public OrderByItem desc() {
-		throw new UnsupportedOperationException(this.toString());
+		return new OrderableItem(this, false);
 	}
 
 	public Predicate between(PredicateOperand arg1, PredicateOperand arg2) {
@@ -404,7 +402,7 @@ abstract class ExpressionImpl implements Expression, Selectable, Visitable {
 	}
 
 	public Predicate like(PredicateOperand pattern) {
-		return new LikeExpression(this, (Expression)pattern, null);
+		return new LikeExpression(this, (Expression)pattern);
 	}
 
 	public Predicate like(PredicateOperand pattern, PredicateOperand escChar) {
@@ -416,7 +414,7 @@ abstract class ExpressionImpl implements Expression, Selectable, Visitable {
 	}
 
 	public Predicate like(String pattern) {
-		return new LikeExpression(this, new ConstantExpression(pattern), null);
+		return new LikeExpression(this, new ConstantExpression(pattern));
 	}
 
 	public Predicate like(String pattern, PredicateOperand escapeChar) {
@@ -468,7 +466,7 @@ abstract class ExpressionImpl implements Expression, Selectable, Visitable {
 		ctx.getAlias(this);
 	}
 	
-	public String getAliasHint() {
+	public String getAliasHint(AliasContext ctx) {
 		return "o";
 	}
 	

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/FetchPath.java

@@ -26,7 +26,8 @@ import javax.persistence.FetchJoinObject;
  * @author Pinaki Poddar
  *
  */
-public class FetchPath implements FetchJoinObject, Visitable {
+public class FetchPath extends AbstractVisitable 
+    implements FetchJoinObject, Visitable {
 	private NavigationPath _path;
 	private PathOperator _joinType;
 	

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/InExpression.java

@@ -1,6 +1,7 @@
 package org.apache.openjpa.persistence.query;
 
 import javax.persistence.Expression;
+import javax.persistence.Subquery;
 
 /**
  * Denotes e1 IN (e2) Expression.
@@ -9,7 +10,13 @@ import javax.persistence.Expression;
  *
  */
 public class InExpression extends BinaryExpressionPredicate  {
-	public InExpression(Expression op, Expression op2) {
+	public InExpression(Expression op, ArrayExpression op2) {
-		super(op, BinaryConditionalOperator.IN, BinaryConditionalOperator.IN_NOT, op2);
+		super(op, BinaryConditionalOperator.IN, 
+			BinaryConditionalOperator.IN_NOT, op2);
+	}
+	
+	public InExpression(Expression op, Expression subquery) {
+		super(op, BinaryConditionalOperator.IN, 
+			BinaryConditionalOperator.IN_NOT, subquery);
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/JPQLHelper.java

@@ -25,9 +25,17 @@ package org.apache.openjpa.persistence.query;
  *
  */
 public class JPQLHelper {
+	private static final String SINGLE_QUOTE = "'";
+	
 	static String toJPQL(AliasContext ctx, Object o) {
+		if (o == null)
+			return "NULL";
 		if (o instanceof Visitable)
 			return ((Visitable)o).asExpression(ctx);
+		if (o instanceof Class)
+			return ctx.getEntityName((Class)o);
+		if (o instanceof String)
+			return SINGLE_QUOTE + (String)o + SINGLE_QUOTE;
 		return o.toString();
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/JoinPath.java

@@ -33,7 +33,7 @@ public class JoinPath extends AbstractDomainObject implements DomainObject {
 	}
 	
 	@Override
-	public String getAliasHint() {
+	public String getAliasHint(AliasContext ctx) {
 		return getLastSegment();
 	}
 	
@@ -47,7 +47,7 @@ public class JoinPath extends AbstractDomainObject implements DomainObject {
 		return (AbstractDomainObject)super.getParent();
 	}
 		
-//	@Override
+	@Override
 	public String asJoinable(AliasContext ctx) {
 		StringBuffer tmp = new StringBuffer(getOperator().toString());
 		tmp.append(getParent().asProjection(ctx))

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/LikeExpression.java

@@ -27,18 +27,28 @@ import javax.persistence.Expression;
  *
  */
 public class LikeExpression extends BinaryExpressionPredicate {
-	public LikeExpression(Expression op1, Expression op2, Object echar) {
+	private final Object  _echar;
-		super(escape(op1, echar), BinaryConditionalOperator.LIKE, 
+	private final boolean _escaped;
-			BinaryConditionalOperator.LIKE_NOT, escape(op2, echar));
+	
+	public LikeExpression(Expression e, Expression pattern, Object echar) {
+		super(e, BinaryConditionalOperator.LIKE, 
+			BinaryConditionalOperator.LIKE_NOT, pattern);
+		_echar = echar;
+		_escaped = true;
 	}
 	
-	static Expression escape(Expression o, Object echar) {
+	public LikeExpression(Expression e, Expression pattern) {
-		if (echar != null && o instanceof ConstantExpression 
+		super(e, BinaryConditionalOperator.LIKE, 
-				&& ((ConstantExpression)o).getValue() instanceof String) {
+			BinaryConditionalOperator.LIKE_NOT, pattern);
-			String escapeChar = echar.toString();
+		
-			return new ConstantExpression(escapeChar + o.toString() + escapeChar);
+		_echar = null;
-		}
+		_escaped = false;
-		return o;
+	}
+	
+	@Override
+	public String asExpression(AliasContext ctx) {
+		return super.asExpression(ctx) 
+		    + (_escaped ? "ESCAPE " + JPQLHelper.toJPQL(ctx, _echar) : "");
 	}
 
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/LocateExpression.java

@@ -21,13 +21,42 @@ package org.apache.openjpa.persistence.query;
 import javax.persistence.Expression;
 
 /**
- * Denotes LOCATE(e)
+ * Denotes LOCATE(e1, e2, n) Expression.
+ * e1 : string to be located
+ * e2 : string to be searched
+ * n  : starting poistion in e2, default is 1
  * 
  * @author Pinaki Poddar
  *
  */
 public class LocateExpression extends BinaryOperatorExpression  {
-	public LocateExpression(Expression op, Expression op2, Object pos) {
+	private final Expression _start;
-		super(op, BinaryFunctionalOperator.LOCATE, op2);
+
+	public LocateExpression(Expression key, String str, int start) {
+		super(key, BinaryFunctionalOperator.LOCATE, new ConstantExpression(str));
+		_start = new ConstantExpression(start);
 	}
+	
+	public LocateExpression(Expression key, Expression str, int start) {
+		super(key, BinaryFunctionalOperator.LOCATE, str);
+		_start = new ConstantExpression(start);
+	}
+	
+	public LocateExpression(Expression key, String str, Expression start) {
+		super(key, BinaryFunctionalOperator.LOCATE, new ConstantExpression(str));
+		_start = start;
+	}
+	
+	public LocateExpression(Expression key, Expression str, Expression start) {
+		super(key, BinaryFunctionalOperator.LOCATE, str);
+		_start = start;
+	}
+	
+	public String asExpression(AliasContext ctx) {
+		return _op + "(" + ((Visitable)_e1).asExpression(ctx) + "," 
+		    + ((Visitable)_e2).asExpression(ctx) 
+		    + (_start == null ? "" : "," + ((Visitable)_start).asExpression(ctx))
+		    + ")";
+	}
+
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/LogicalPredicate.java

@@ -26,7 +26,8 @@ import javax.persistence.Predicate;
  * @author Pinaki Poddar
  *
  */
-public class LogicalPredicate implements Predicate, Visitable {
+public class LogicalPredicate extends AbstractVisitable 
+    implements Predicate, Visitable {
 	private final Predicate _p1;
 	private final Predicate _p2;
 	private final ConditionalOperator _op;

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/NewInstance.java

@@ -18,7 +18,6 @@
  */
 package org.apache.openjpa.persistence.query;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
@@ -31,7 +30,8 @@ import javax.persistence.SelectItem;
  * @author Pinaki Poddar
  *
  */
-public class NewInstance implements Selectable {
+public class NewInstance extends AbstractVisitable 
+	implements SelectItem, Visitable {
 	private final Class _cls;
 	private List<SelectItem> _args;
 	NewInstance(Class cls, SelectItem...args) {
@@ -59,12 +59,9 @@ public class NewInstance implements Selectable {
 		int N = _args.size();
 		for (SelectItem arg : _args) {
 			i++;
-			tmp.append(((Selectable)arg).asProjection(ctx))
+			tmp.append(((Visitable)arg).asProjection(ctx))
 			   .append(i == N ? ")" : ",");
 		}
 		return tmp.toString();
 	}
-	
-	public void setAlias(AliasContext ctx) {
-	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/OperatorPath.java

@@ -37,8 +37,8 @@ public class OperatorPath extends AbstractDomainObject {
 	}
 
 	@Override
-	public String getAliasHint() {
+	public String getAliasHint(AliasContext ctx) {
-		return getParent().getAliasHint();
+		return getParent().getAliasHint(ctx);
 	}
 	
 	@Override

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/OrderableItem.java

@@ -26,22 +26,27 @@ import javax.persistence.OrderByItem;
  * @author Pinaki Poddar
  *
  */
-public class OrderableItem implements OrderByItem {
+public class OrderableItem extends AbstractVisitable 
+    implements OrderByItem, Visitable {
 	private final Boolean _asc;
-	private final ExpressionImpl path;
+	private final ExpressionImpl _e;
 	
 	OrderableItem(ExpressionImpl path) {
 		this(path, null);
 	}
 	
+	public ExpressionImpl getExpression() {
+		return _e;
+	}
+	
 	OrderableItem(ExpressionImpl path, Boolean asc) {
 		super();
 		this._asc = asc;
-		this.path = path;
+		this._e = path;
 	}
 	
-	public String toJPQL(AliasContext ctx) {
+	public String asExpression(AliasContext ctx) {
-		return path.asExpression(ctx) + " " 
+		return (ctx.hasAlias(_e) ? ctx.getAlias(_e) : _e.asExpression(ctx)) 
-		    + (_asc == null ? "" : (_asc ? " ASC " : "DESC"));
+		    + (_asc == null ? "" : (_asc ? " ASC " : " DESC"));
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ParameterExpression.java

@@ -24,13 +24,19 @@ package org.apache.openjpa.persistence.query;
  * @author Pinaki Poddar
  *
  */
-public class ParameterExpression extends ConstantExpression {
+public class ParameterExpression extends ExpressionImpl {
+	private final String _name;
 	public ParameterExpression(String name) {
-		super(name);
+		_name = ":" + name;
 	}
 	
 	@Override
 	public String asExpression(AliasContext ctx) {
-		return ":" + getValue();
+		return _name;
+	}
+	
+	@Override
+	public String asProjection(AliasContext ctx) {
+		return _name;
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/QueryBuilderImpl.java

@@ -18,11 +18,19 @@
  */
 package org.apache.openjpa.persistence.query;
 
+import java.io.Serializable;
+
 import javax.persistence.DomainObject;
 import javax.persistence.PathExpression;
 import javax.persistence.QueryBuilder;
 import javax.persistence.QueryDefinition;
 
+import org.apache.openjpa.meta.ClassMetaData;
+import org.apache.openjpa.meta.MetaDataRepository;
+import org.apache.openjpa.persistence.EntityManagerFactoryImpl;
+import org.apache.openjpa.persistence.OpenJPAEntityManagerFactory;
+import org.apache.openjpa.persistence.OpenJPAEntityManagerFactorySPI;
+
 /**
  * The factory for QueryDefinition.
  * 
@@ -31,25 +39,39 @@ import javax.persistence.QueryDefinition;
  *
  */
 public class QueryBuilderImpl implements QueryBuilder {
+	private final OpenJPAEntityManagerFactorySPI _emf;
+	
+	public QueryBuilderImpl(OpenJPAEntityManagerFactorySPI emf) {
+		_emf = emf;
+	}
+	
 	/**
-	 * Creates a QueryDefinition without a domain root.
+	 * Creates a QueryDefinition without a domain.
 	 */
 	public QueryDefinition createQueryDefinition() {
 		return new QueryDefinitionImpl(this);
 	}
 
 	/**
-	 * Creates a QueryDefinition with given class as domain root.
+	 * Creates a QueryDefinition with given class as domain.
 	 */
 	public DomainObject createQueryDefinition(Class root) {
 		return new QueryDefinitionImpl(this).addRoot(root);
 	}
 
 	/**
-	 * Creates a QueryDefinition that can be used as a subquery to some
+	 * Creates a QueryDefinition that can be used a correlated subquery 
-	 * other query.
+	 * with the given path as domain.
 	 */
 	public DomainObject createSubqueryDefinition(PathExpression path) {
 		return new QueryDefinitionImpl(this).addSubqueryRoot(path);
 	}
+	
+	public String toJPQL(QueryDefinition query) {
+		MetaDataRepository repos = null;//_emf.getConfiguration().getMetaDataRepositoryInstance()
+		AliasContext ctx = new AliasContext(repos);
+		if (query instanceof AbstractDomainObject)
+			return ((AbstractDomainObject)query).getOwner().asExpression(ctx);
+		return ((QueryDefinitionImpl)query).asExpression(ctx);
+	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/QueryDefinitionImpl.java

@@ -49,16 +49,21 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	private final QueryBuilderImpl _builder;
 	private List<AbstractDomainObject> _domains;
 	private List<PathExpression> _groupBys;
-	private List<Subquery> _subqueries;
 	private List<OrderableItem> _orderBys;
-	private List<Selectable>  _projections;
+	private List<SelectItem>  _projections;
 	private boolean  _distinct;
 	private Predicate _where;
 	private Predicate _having;
 	
+	private static enum Visit {PROJECTION, EXPRESSION, JOINABLE};
+	
 	protected static Localizer _loc = 
 		Localizer.forPackage(QueryDefinitionImpl.class);
 	
+	/**
+	 * 
+	 * @param builder
+	 */
 	protected QueryDefinitionImpl(QueryBuilderImpl builder) {
 		_builder = builder;
 	}
@@ -73,14 +78,9 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	}
 	
 	public DomainObject addSubqueryRoot(PathExpression path) {
-		if (_domains != null && _domains.contains(path))
-			throw new IllegalArgumentException(_loc.get("query-subroot-clash", 
-					path).toString());
 		AbstractPath impl = (AbstractPath)path;
-		if (_subqueries == null) 
+		AbstractDomainObject newRoot = new NavigationPath(this,  
-			_subqueries = new ArrayList<Subquery>();
+				impl.getParent(), impl.getLastSegment().toString());
-		AbstractDomainObject newRoot = new NavigationPath(this, impl.getParent(), 
-				impl.getLastSegment().toString());
 		addDomain(newRoot);
 		return newRoot;
 	}
@@ -229,11 +229,13 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	public QueryDefinition orderBy(OrderByItem... orderByItems) {
 		if (_orderBys == null)
 			_orderBys = new ArrayList<OrderableItem>();
+		else
+			_orderBys.clear();
 		for (OrderByItem i : orderByItems) {
 			if (i instanceof OrderableItem)
 				_orderBys.add((OrderableItem)i);
 			else
-				_orderBys.add(new OrderableItem((ExpressionImpl)i, null));
+				_orderBys.add(new OrderableItem((ExpressionImpl)i));
 		}
 		return this;
 	}
@@ -241,6 +243,8 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	public QueryDefinition orderBy(List<OrderByItem> orderByItemList) {
 		if (_orderBys == null)
 			_orderBys = new ArrayList<OrderableItem>();
+		else
+			_orderBys.clear();
 		for (OrderByItem i : orderByItemList) {
 			if (i instanceof OrderableItem)
 				_orderBys.add((OrderableItem)i);
@@ -276,13 +280,13 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	
 	private QueryDefinition select(List<SelectItem> items, boolean isDistinct) {
 		if (_projections == null) {
-			_projections = new ArrayList<Selectable>();
+			_projections = new ArrayList<SelectItem>();
 		} else {
 			_projections.clear();
 		}
 		_distinct = isDistinct;
 		for (SelectItem item : items)
-			_projections.add((Selectable)item);
+			_projections.add(item);
 		return this;
 	}
 
@@ -323,78 +327,48 @@ public class QueryDefinitionImpl extends ExpressionImpl
 		return this;
 	}
 	
-	private List<Selectable> getProjections() {
+	private List<SelectItem> getProjections() {
 		if (_projections == null) {
-			List<Selectable> defaultProjection = new ArrayList<Selectable>();
+			List<SelectItem> defaultProjection = new ArrayList<SelectItem>();
 			defaultProjection.add(_domains.get(0));
 			return defaultProjection;
 		}
 		return _projections;
 	}
 
-	public String toJPQL() {
-		return asExpression(new AliasContext());
-	}
-	
 	/**
 	 * 
 	 */
 	@Override
 	public String asExpression(AliasContext ctx) {
+		ctx.push(this);
 		StringBuffer buffer = new StringBuffer();
 		registerDomains(ctx);
-		buffer.append("SELECT ");
+
-		if (_distinct) 
+		fillBuffer(_distinct ? "SELECT DISTINCT " : "SELECT ", buffer, ctx, 
-			buffer.append("DISTINCT ");
+			getProjections(), Visit.PROJECTION);
-		List<Selectable> projs = getProjections();
-		for (int i=0; i < projs.size(); i++) {
-			projs.get(i).setAlias(ctx);
-			buffer.append(projs.get(i).asProjection(ctx));
-			if (i != projs.size()-1)
-				buffer.append(",");
-		}
 		buffer.append(" FROM ");
 		for (int i=0; _domains != null && i < _domains.size(); i++) {
 			buffer.append(_domains.get(i).asJoinable(ctx));
-			List<JoinPath> joins = _domains.get(i).getJoins();
+			fillBuffer(" ", buffer, ctx, _domains.get(i).getJoins(), 
-			if (joins != null) {
+					Visit.JOINABLE);
-				for (int j = 0; j < joins.size(); j++) {
+			fillBuffer(" ", buffer, ctx, _domains.get(i).getFetchJoins(), 
-					buffer.append(joins.get(j).asJoinable(ctx));
+					Visit.EXPRESSION);
-				}
-			}
-			List<FetchPath> fetchJoins = _domains.get(i).getFetchJoins();
-			if (fetchJoins != null) {
-				for (int j = 0; j < fetchJoins.size(); j++) {
-					buffer.append(fetchJoins.get(j).asExpression(ctx));
-				}
-			}
-			
 			if (i != _domains.size()-1)
 				buffer.append(",");
 		}
 		if (_where != null) {
-			buffer.append(" WHERE ").append(((Visitable)_where).asExpression(ctx));
+			buffer.append(" WHERE ")
+			      .append(((Visitable)_where).asExpression(ctx));
 		}
 		
-		if (_groupBys != null) {
+		fillBuffer(" GROUP BY ", buffer, ctx, _groupBys, Visit.EXPRESSION);
-			buffer.append(" GROUP BY ");
+		
-			for (int i = 0; i<_groupBys.size(); i++) {
-				buffer.append(((ExpressionImpl)_groupBys.get(i)).asExpression(ctx));
-				if (i != _groupBys.size()-1)
-					buffer.append(",");
-			}
-		}
 		if (_having != null) {
-			buffer.append(" HAVING ").append(((Visitable)_having).asExpression(ctx));
+			buffer.append(" HAVING ")
-		}
+			      .append(((Visitable)_having).asExpression(ctx));
-		if (_orderBys != null) {
-			buffer.append(" ORDER BY ");
-			for (int i = 0; i<_orderBys.size(); i++) {
-				buffer.append(((OrderableItem)_orderBys.get(i)).toJPQL(ctx));
-				if (i != _orderBys.size()-1)
-					buffer.append(",");
-			}
 		}
+		fillBuffer(" ORDER BY ", buffer, ctx, _orderBys, Visit.EXPRESSION);
 		
 		return buffer.toString();
 	}
@@ -403,6 +377,26 @@ public class QueryDefinitionImpl extends ExpressionImpl
 		return asExpression(ctx);
 	}
 	
+	public void fillBuffer(String header, StringBuffer buffer, AliasContext ctx, 
+		List list, Visit visit) {
+		if (list == null || list.isEmpty())
+			return;
+		buffer.append(header);
+		for (int i = 0; i < list.size(); i++) {
+			Visitable v = (Visitable)list.get(i);
+			switch(visit) {
+			case PROJECTION : buffer.append(v.asProjection(ctx))
+				                    .append(i != list.size()-1 ? ", " : " ");
+				break;
+			case EXPRESSION : buffer.append(v.asExpression(ctx))
+				                    .append(i != list.size()-1 ? ", " : " ");
+				break;
+			case JOINABLE   : buffer.append(v.asJoinable(ctx)); 
+				break;
+			}
+		}
+	}
+	
 	/**
 	 * Registers each domain with an alias.
 	 * @param ctx
@@ -413,12 +407,11 @@ public class QueryDefinitionImpl extends ExpressionImpl
 				domain.setAlias(ctx);
 			}
 		}
-		if (_subqueries != null) {
+		if (_orderBys != null) {
-			for (Subquery sub : _subqueries) {
+			for (OrderableItem o : _orderBys) {
-				if (sub instanceof QueryDefinitionImpl)
+				ExpressionImpl e = o.getExpression();
-					((QueryDefinitionImpl)sub).registerDomains(ctx);
+				if (_projections != null && _projections.contains(e))
-				else
+					e.setAlias(ctx);
-					((AbstractDomainObject)sub).setAlias(ctx);
 			}
 		}
 	}

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/RootPath.java

@@ -20,8 +20,8 @@ public class RootPath extends AbstractDomainObject implements DomainObject {
 	}
 	
 	@Override
-	public String getAliasHint() {
+	public String getAliasHint(AliasContext ctx) {
-		return getLastSegment().getSimpleName();
+		return ctx.getEntityName(getLastSegment());
 	}
 
 	@Override
@@ -31,7 +31,7 @@ public class RootPath extends AbstractDomainObject implements DomainObject {
 	
 	@Override
 	public String asJoinable(AliasContext ctx) {
-		return getLastSegment().getSimpleName() + " " + ctx.getAlias(this);
+		return ctx.getEntityName(getLastSegment()) + " " + ctx.getAlias(this);
 	}
 	
 	@Override

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/SubStringExpression.java

@@ -26,20 +26,37 @@ import javax.persistence.Expression;
  * @author Pinaki Poddar
  *
  */
-public class SubStringExpression extends BinaryOperatorExpression {
+public class SubStringExpression extends UnaryOperatorExpression {
-	public SubStringExpression(Expression op, Expression op2) {
+	private final Expression _start;
-		this(op, op2, new ConstantExpression(0));
+	private final Expression _length;
+	public SubStringExpression(Expression op, Expression start) {
+		super(op, UnaryFunctionalOperator.SUBSTR);
+		_start  = start;
+		_length = null;
 	}
 	
 	public SubStringExpression(Expression op, int start) {
-		this(op, start, 0);
+		super(op, UnaryFunctionalOperator.SUBSTR);
+		_start  = new ConstantExpression(start);
+		_length = null;
 	}
 	
 	public SubStringExpression(Expression op, int start, int len) {
-		this(op, new ConstantExpression(start), new ConstantExpression(len));
+		super(op, UnaryFunctionalOperator.SUBSTR);
+		_start  = new ConstantExpression(start);
+		_length = new ConstantExpression(len);
 	}
 	
-	public SubStringExpression(Expression op, Expression op2, Expression pos) {
+	public SubStringExpression(Expression op, Expression start, Expression l) {
-		super(op, BinaryFunctionalOperator.SUBSTR, op2);
+		super(op, UnaryFunctionalOperator.SUBSTR);
+		_start  = start;
+		_length = new ConstantExpression(l);
+	}
+	
+	public String asExpression(AliasContext ctx) {
+		return _op + "(" + ((Visitable)_e).asExpression(ctx)  
+			 + "," + ((Visitable)_start).asExpression(ctx)
+			 + (_length == null ? "" : "," + ((Visitable)_length).asExpression(ctx))
+			 + ")";
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/TrimExpression.java

@@ -27,15 +27,29 @@ import javax.persistence.TrimSpec;
  * @author Pinaki Poddar
  *
  */
-public class TrimExpression extends BinaryOperatorExpression {
+public class TrimExpression extends UnaryOperatorExpression {
-	TrimSpec _spec;
+	private final Expression _trimChar;
-	public TrimExpression(Expression op1, char ch, TrimSpec spec) {
+	private final TrimSpec _spec;
-		super(op1, BinaryFunctionalOperator.TRIM, new ConstantExpression(ch));
+	private static final String BLANK = "' '";
+	
+	public TrimExpression(Expression op, char ch, TrimSpec spec) {
+		super(op, UnaryFunctionalOperator.TRIM);
+		_trimChar = new ConstantExpression(ch);
+		_spec     = spec;
+	}
+	
+	public TrimExpression(Expression op, Expression ch, TrimSpec spec) {
+		super(op, UnaryFunctionalOperator.TRIM);
+		_trimChar = ch;
 		_spec = spec;
 	}
 	
-	public TrimExpression(Expression op, Expression op1, TrimSpec spec) {
+	public String asExpression(AliasContext ctx) {
-		super(op, BinaryFunctionalOperator.TRIM, op1);
+		String trim = _trimChar == null ? BLANK 
-		_spec = spec;
+			: ((Visitable)_trimChar).asExpression(ctx);
+		String spec = _spec == null ? "" : _spec.toString();
+		return _op + "(" + spec + " " + trim + " FROM " 
+			+ ((Visitable)_e).asExpression(ctx) + ")";
 	}
+
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/UnaryExpressionPredicate.java

@@ -27,7 +27,8 @@ import javax.persistence.Predicate;
  * @author Pinaki Poddar
  *
  */
-class UnaryExpressionPredicate implements Predicate, Visitable {
+class UnaryExpressionPredicate extends AbstractVisitable 
+	implements Predicate, Visitable {
 	protected final Expression _e;
 	protected final UnaryConditionalOperator _op;
 	private final UnaryConditionalOperator _nop;

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/UnaryFunctionalOperator.java

@@ -32,22 +32,25 @@ public enum UnaryFunctionalOperator {
 	ALL("ALL"),
 	ANY("ANY"),
 	AVG("AVG"),
+	CONCAT("CONCAT"),
 	COUNT("COUNT"),
 	DISTINCT("DISTINCT"),
 	EXISTS("EXISTS"),
 	INDEX("INDEX"),
 	LENGTH("LENGTH"),
 	LOCATE("LOCATE"),
-	LOWER("TOLOWER"),
+	LOWER("LOWER"),
 	MAX("MAX"),
 	MIN("MIN"),
 	MINUS("-"),
 	SIZE("SIZE"),
 	SOME("SOME"),
 	SQRT("SQRT"),
+	SUBSTR("SUBSTRING"),
 	SUM("SUM"),
+	TRIM("TRIM"),
 	TYPE("TYPE"),
-	UPPER("TOUPPER");
+	UPPER("UPPER");
 	
 	private final String _symbol;
 	

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/UnaryOperatorExpression.java

@@ -47,6 +47,7 @@ class UnaryOperatorExpression extends ExpressionImpl implements Aggregate {
 	}
 	
 	public String asProjection(AliasContext ctx) {
-		return _op + "(" + ((Selectable)_e).asProjection(ctx) + ")";
+		return _op + "(" + ((Visitable)_e).asProjection(ctx) + ")" +
+		    (ctx.hasAlias(this) ? " " + ctx.getAlias(this) : "");
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/UpperExpression.java

@@ -26,8 +26,8 @@ import javax.persistence.Expression;
  * @author Pinaki Poddar
  *
  */
-public class ToUpperExpression extends UnaryOperatorExpression {
+public class UpperExpression extends UnaryOperatorExpression {
-	public ToUpperExpression(Expression op) {
+	public UpperExpression(Expression op) {
 		super(op, UnaryFunctionalOperator.UPPER);
 	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/Visitable.java

@@ -18,6 +18,8 @@
  */
 package org.apache.openjpa.persistence.query;
 
+import java.io.Serializable;
+
 /**
  * An element of query that is convertible to a JPQL String given a aliasing 
  * scheme. QueryDefinition visits each of its element and translates them.
@@ -25,9 +27,26 @@ package org.apache.openjpa.persistence.query;
  * @author Pinaki Poddar
  *
  */
-public interface Visitable {
+public interface Visitable extends Serializable {
 	/**
 	 * Get a JPQL fragment as used in a WHERE clause.
 	 */
 	String asExpression(AliasContext ctx);
+	
+	/**
+	 * Gets the string representation in a SELECT projection.
+	 */
+	String asProjection(AliasContext ctx);
+	
+	/**
+	 * Sets alias.
+	 */
+	void setAlias(AliasContext ctx);
+	
+	String getAliasHint(AliasContext ctx);
+	
+	String asJoinable(AliasContext ctx);
+
+
+
 }