persistence@glassfish.java.net

Re: code review for issue 136

From: jie leng <jie.leng_at_Sun.COM>
Date: Fri, 24 Mar 2006 14:11:07 -0800

I test it agianst oracle, the test is working and the IN with dates
return the correct values.

Thanks.

Jielin

Tom Ware wrote:

> The spec says the following:
>
> 4.6.8 In Expressions
> The syntax for the use of the comparison operator [NOT] IN in a
> conditional expression is as follows:
> in_expression ::=
> state_field_path_expression [NOT] IN ( in_item {, in_item}* | subquery)
> in_item ::= literal | input_parameter
> The state_field_path_expression must have a string, numeric, or enum
> value
>
> So, I guess the test can be removed, but I think the fact that it
> works on some databases opens up the debate of whether it should be
> disallowed by validation. I know it's a fine line, but, did the test
> actually work, or just not fail? (i.e. did the IN with dates return
> correct values?)
>
> -Tom
>
> jie leng wrote:
>
>> Hi, Michael, Tom,
>>
>> I ran entity-persistence-tests on the change I made for InNode. The
>> JUnitEJBQLDateTimeTestSuite.testTimestampIn() failed because the
>> validation in InNode.java only allows string, numeric and enum type
>> according to the spec. Is the test for timestamp using IN still
>> valid? Should the test be removed?
>>
>> Thanks.
>>
>> Jielin
>>
>> Michael Bouschen wrote:
>>
>>
>>
>>> Hi Jielin,
>>>
>>> the changes look good. Two remarks:
>>>
>>> - I propose to add a method isString type to the TypeHelper
>>> interface and call this in InNode.validate.
>>> This should go into TypeHelper.java:
>>> /** Returns true if the specified type represents java.lang.String. */
>>> public boolean isStringType(Object type);
>>> and this should be added to BasicTypeHelper.java:
>>> /** Returns true if the specified type represents java.lang.String. */
>>> public boolean isStringType(Object type) {
>>> return type == getStringType();
>>> }
>>>
>>> - Are you planning to add a test to JUnitEJBQLValidationTestSuite in
>>> entity-persistence-tests? I propose to add two test cases: one where
>>> the in clause consists of a list of value and another using a subquery.
>>>
>>> Just a heads-up: I'm also planning to change EJBQLException, so we
>>> need to check for conflicts (e.g. using the same number for the
>>> constant etc.).
>>>
>>> Regards Michael
>>>
>>>
>>>
>>>> Hi, Michael, Tom,
>>>>
>>>> Attached is the fix for issue 136 - JPQL validation: Parser not
>>>> catching invalid expression type with IN expression.
>>>>
>>>> According to spec,
>>>> in_expression ::= state_field_path_expression [NOT] IN (in_item {,
>>>> in_item}* | subquery)
>>>> The state_field_path_expression must have a string, numeric or enum
>>>> value.
>>>> The in_item and the result of subquery must be like the same
>>>> abstrct schema type of the state_field_path_expression.
>>>>
>>>> Added two validation checks in InNode.java.
>>>>
>>>> Please review the change.
>>>>
>>>> Thanks.
>>>>
>>>> Jielin
>>>>
>>>>
>>>>
>>>
>