persistence@glassfish.java.net

Re: code review for Issue 214 and 273

From: Tom Ware <tom.ware_at_oracle.com>
Date: Tue, 21 Feb 2006 14:28:45 -0500

Hi Jie,

  One minor coment and one question.

Comment: In the following lines of code:
--
            if (!getDatabaseQuery().getArguments().contains(pos)) {
                throw new 
IllegalArgumentException(ExceptionLocalization.buildMessage("ejb30-wrong-argument-index", 
new Object[]{position, getDatabaseQuery().getEJBQLString()}));
            }
            int index = getDatabaseQuery().getArguments().indexOf(pos);
--
  Is it possible to do the look-up for 'index' before the if and then 
use the value of index to determine if we throw the exception?  This 
will avoid one lookup.
Question:
Can regression tests be added to the entity-persistence-testing module 
for these bug fixes.  I woudl suggest adding tests in either in one of 
the EJBQL test suites defined in: 
oracle.toplink.essentials.testing.tests.ejb.ejbqltesting or adding a 
EJBQuery test suite to oracle.toplink.essentials.testing.tests.cmp3.advanced
Other than that, everything looks good.
Thanks,
Tom
jie leng wrote:
>Hi, Tom/Michael,
>
>Here is the fix for glassfish issue 273 and 214.
>
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=273
>
>and 
>
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=214
>
>Please review the change. The change is in EJBQueryImpl.java to handle validation check correctly for position query such as:
>
>1.  String ejbql = "SELECT i FROM Item i WHERE i.name = ?1 AND i.itemId = ?3);
>
>Query query = em.createQuery(ejbql);
>
>query.setParameter(1, "Jie Leng");
>
>query.setParameter(2, "");
>
>query.setParameter(3, new Integer(1));
>
>It should throw:
>
>[java] Exception in thread "main" 
>java.lang.IllegalArgumentException: You have attempted to set a parameter at position 2 which does not exist in this query string SELECT i FROM Item i WHERE i.name = ?1 AND i.itemId = ?3.
>
>2. String ejbql = "SELECT i FROM Item i WHERE i.name = ?1 AND i.itemId = ?3);
>
>Query query = em.createQuery(ejbql);
>
>query.setParameter(1, "Jie Leng");
>
>query.setParameter(3, new Integer(1));
>
>This query should be processed successfully even though ?2 is missing.
>
>
>Thanks.
>
>Jielin
>
>  
>
-- 
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com