Thanks for quick responses!
I modified the fix as Andrei suggested.
The fix also includes a regression test in JUnitEJBQLParameterTestSuite. I
needed to add enum types to Employee class for this test.
It is tested with entity-persistence-tests and QuickLook tests.
Please review!
Diffs and modified files are in the attached file.
If it has no problem, Jielin, could you check in?
Cheers~
- Wonseok
On 10/4/06, jielin <Jie.Leng_at_sun.com> wrote:
>
> 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
> >
>