persistence@glassfish.java.net

Re: code review for issue 136

From: Michael Bouschen <Michael.Bouschen_at_Sun.COM>
Date: Fri, 24 Mar 2006 17:30:09 +0100

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