Hi Wonseok,
I think that the idea is correct.
To make it safer I would limit the fix to valueExpression (that covers both ConstantExpression and ParameterExpression),
instead of:
//GF#1123
valueExpression = Expression.from(value, baseExpression);
use:
if(valueObject instanceof Expression) {
valueExpression = (Expression)value;
} else {
valueExpression = builder.value(value);
}
//GF#1123
if(valueExpression.isValueExpression()) {
valueExpression.setLocalBase(baseExpression);
}
Thanks,
Andrei
----- Original Message -----
From: Wonseok Kim
To: persistence_at_glassfish.dev.java.net ; jielin
Sent: Tuesday, October 03, 2006 8:56 AM
Subject: issue#1123 UPDATE with JPQL does not handle enums correctly
Hi Jielin and team,
I've investigated issue 1123 for some time.
https://glassfish.dev.java.net/issues/show_bug.cgi?id=1123
As you can see my message in issue comments, ParameterExpression in SET clause is not coverting object to data value. It's not JPQL query parsing problem, but UpdateAllQuery problem. Below test also fails.
...
// test UpdateAll
ExpressionBuilder builder = new ExpressionBuilder();
Expression selectionExpression = builder.get("id").equal(emp.getId());
UpdateAllQuery updateQuery = new UpdateAllQuery(Employee.class, selectionExpression);
updateQuery.addUpdate("status", builder.getParameter("status"));
updateQuery.addUpdate(builder.get("payScale"), builder.getParameter("payScale"));
updateQuery.addArgument("status");
updateQuery.addArgument("payScale");
Vector arguments = new Vector();
arguments.add(Employee.EmployeeStatus.PART_TIME );
arguments.add(Employee.SalaryRate.SENIOR);
em = createEntityManager();
em.getTransaction().begin();
try {
Session session = ((oracle.toplink.essentials.ejb.cmp3.EntityManager)em).getActiveSession();
updateQuery.setShouldDeferExecutionInUOW(false);
session.executeQuery(updateQuery, arguments);
em.getTransaction().commit();
} catch (RuntimeException e) {
em.getTransaction().rollback();
throw e;
} finally {
em.close();
}
[exception]
Internal Exception: org.apache.derby.client.am.SqlException: Invalid data conversion: Parameter object type is invalid for requested conversion.Error Code: -99999
Call:UPDATE CMP3_EMPLOYEE SET STATUS = ?, VERSION = (VERSION + 1), PAY_SCALE = ? WHERE EXISTS(SELECT t0.EMP_ID FROM CMP3_EMPLOYEE t0, CMP3_SALARY t1 WHERE ((t0.EMP_ID = 656) AND (t1.EMP_ID = t0.EMP_ID)) AND t0.EMP_ID = CMP3_EMPLOYEE.EMP_ID)
bind => [PART_TIME, SENIOR]
Query:UpdateAllQuery()
...
Caused by: org.apache.derby.client.am.SqlException: Invalid data conversion: Parameter object type is invalid for requested conversion.
at org.apache.derby.client.am.PreparedStatement.setObject(Unknown Source)
While I test another case, I found that the following test also failed.
...
// test UpdateAll
ExpressionBuilder builder = new ExpressionBuilder();
Expression selectionExpression = builder.get("id").equal(emp.getId());
UpdateAllQuery updateQuery = new UpdateAllQuery(Employee.class , selectionExpression);
updateQuery.addUpdate("status", Employee.EmployeeStatus.PART_TIME );
updateQuery.addUpdate(builder.get("payScale"), Employee.SalaryRate.SENIOR );
em = createEntityManager();
em.getTransaction().begin();
try {
Session session = ((oracle.toplink.essentials.ejb.cmp3.EntityManager)em).getActiveSession();
updateQuery.setShouldDeferExecutionInUOW(false);
session.executeQuery(updateQuery);
em.getTransaction().commit();
} catch (RuntimeException e) {
em.getTransaction().rollback();
throw e;
} finally {
em.close();
}
[exception]
Internal Exception: org.apache.derby.client.am.SqlException: Column 'PART_TIME' is either not in any table in the FROM list or appears within a join specification and is outside the scope of the join specification or appears in a HAVING clause and is not in the GROUP BY list. If this is a CREATE or ALTER TABLE statement then 'PART_TIME' is not a column in the target table.Error Code: -1
Call:UPDATE CMP3_EMPLOYEE SET STATUS = PART_TIME, VERSION = (VERSION + 1), PAY_SCALE = SENIOR WHERE EXISTS(SELECT t0.EMP_ID FROM CMP3_EMPLOYEE t0, CMP3_SALARY t1 WHERE ((t0.EMP_ID = 657) AND (t1.EMP_ID = t0.EMP_ID)) AND t0.EMP_ID = CMP3_EMPLOYEE.EMP_ID)
...
I attached above tests(modified entity-persistence-tests).
I think that the cause of above failures is unproperly set 'localBase' value of ParameterExpression and ConstantExpression. To be properly converted from object to data value, localBase value should be QueryKeyExpression, because only QueryKeyExpression.getFieldValue() has actual converting code.
So I tried to modify some code in ExpressionQueryMechanism.prepareUpdateAllQuery() as below diff and it worked well(passed above tests).
Index: src/java/oracle/toplink/essentials/internal/queryframework/ExpressionQueryMechanism.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/queryframework/ExpressionQueryMechanism.java,v
retrieving revision 1.14
diff -c -r1.14 ExpressionQueryMechanism.java
*** src/java/oracle/toplink/essentials/internal/queryframework/ExpressionQueryMechanism.java 9 Jun 2006 19:44:31 -0000 1.14
--- src/java/oracle/toplink/essentials/internal/queryframework/ExpressionQueryMechanism.java 3 Oct 2006 12:44:00 -0000
***************
*** 1435,1440 ****
--- 1435,1441 ----
Object fieldObject = entry.getKey();
DataExpression fieldExpression = null;
+ Expression baseExpression = null;
String attributeName = null;
if(fieldObject instanceof String) {
attributeName = (String)fieldObject;
***************
*** 1453,1458 ****
--- 1454,1460 ----
if(field == null) {
throw QueryException.updateAllQueryAddUpdateDoesNotDefineField(getDescriptor(), getQuery(), attributeName);
}
+ baseExpression = ((UpdateAllQuery)getQuery()).getExpressionBuilder().get(attributeName);
} else if (fieldExpression != null) {
field = getDescriptor().getObjectBuilder().getFieldForQueryKeyName(fieldExpression.getName());
if(field == null) {
***************
*** 1465,1470 ****
--- 1467,1473 ----
}
}
mapping = getDescriptor().getObjectBuilder().getMappingForField(field);
+ baseExpression = fieldExpression;
}
Object valueObject = entry.getValue();
***************
*** 1517,1527 ****
Object value = values.elementAt(i);
Expression valueExpression;
! if(valueObject instanceof Expression) {
! valueExpression = (Expression)value;
! } else {
! valueExpression = builder.value(value);
! }
databaseFieldsToValues.put(field, valueExpression);
}
--- 1520,1527 ----
Object value = values.elementAt(i);
Expression valueExpression;
! //GF#1123
! valueExpression = Expression.from(value, baseExpression);
databaseFieldsToValues.put (field, valueExpression);
}
What do you think?
Thanks
- Wonseok