Hi, Wonseok,
I totally agreed with you. It is not JPQL problem. Your fix looks okay.
Thanks.
Jielin
Andrei Ilitchev wrote:
> 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 <mailto:guruwons_at_gmail.com>
> *To:* persistence_at_glassfish.dev.java.net
> <mailto:persistence_at_glassfish.dev.java.net> ; jielin
> <mailto:Jie.Leng_at_sun.com>
> *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
>