persistence@glassfish.java.net

Re: code review for issue 146

From: jie leng <jie.leng_at_Sun.COM>
Date: Thu, 23 Mar 2006 13:30:37 -0800

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
>>