persistence@glassfish.java.net

Re: code review for issue 136

From: Tom Ware <tom.ware_at_oracle.com>
Date: Fri, 24 Mar 2006 16:41:24 -0500

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

-- 
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com