persistence@glassfish.java.net

Re: code review for issue 146

From: Michael Bouschen <Michael.Bouschen_at_Sun.COM>
Date: Thu, 23 Mar 2006 23:36:52 +0100

Hi Jielin,

looks good!

Regards Michael
> Hi, Michael, Tom,
>
> Attached files include the change from Michael's comments and also
> include the mod validation test added to
> JUnitEJBQLValidationTestSuite.java. Please review it.
>
> Thanks.
>
> Jielin
>
> Michael Bouschen wrote:
>
>> Hi Jielin,
>>
>> a couple of remarks:
>> - Please move the test of the left argument type to the block that
>> makes sure the left argument is not null. So the test should directly
>> follow the line calling left.validateParameter.
>> - We need the same test for the right argument (called denominator).
>> - We should generalize the error message such that we can reuse it
>> for other function nodes. I propose to call the error constant and
>> the method invalidFunctionArgument. The message could be:
>> Invalid {0} function argument [{1}], expected argument of type [{2}].
>> In the case of the MOD function you would call:
>> invalidFunctionArgument("MOD", left.getAsString(), "integral type");
>>
>> Tom,
>> do we need a Query CTS run for this change?
>>
>> Regards Michael
>>
>>> Hi, Tom, Michael,
>>>
>>> Here is the code change for issue 146 - JPQL validation: Parser not
>>> catching invalid argument type used with MOD.
>>>
>>> With more validation in ModNode, for the query
>>>
>>> select o from Order o WHERE MOD(o.totalPrice, 10) = 5.
>>>
>>> The EJBQLException will be throwed:
>>>
>>> [java] Caused by: Exception [TOPLINK-8020] (Oracle TopLink
>>> Essentials - 2006.3 (Build 060319)):
>>> oracle.toplink.essentials.exceptions.EJBQLException
>>> [java] Exception Description: Invalid MOD argument
>>> [o.totalPrice], expected an integral data type.
>>>
>>> Please review the code.
>>>
>>> Thanks.
>>>
>>> Jielin
>>>
>