users@jpa-spec.java.net

[jpa-spec users] [jsr338-experts] Re: Ambiguities in exceptions for EntityManager.createQuery(…)

From: Oliver Gierke <ogierke_at_vmware.com>
Date: Thu, 19 Jul 2012 10:18:27 -0700

Am 18.07.2012 um 19:07 schrieb Pinaki Poddar:

> > I just did a brief check against OpenJpa 2.2.0.
> OpenJPA does "compile" the query in createQuery(). So you will see fail-fast behavior i.e. an exception would be thrown if a query string is syntactically incorrect.
>
> However, JPA spec made the choice that allowed an implementation to choose when (i.e. at construction or at execution) to validate/parse a query. I see that as a choice for a JPA provider rather than a hindrance to application portability of some sort.

If the option stays available no one can implement a validity check of a query in a reliable and portable way, except writing lots of boilerplate code.

try {
  Query query = em.createQuery(queryString);

  // still not sure, what next?

} catch (IllegalArgumentException e) {
  // query broken
}

To summarize:

1. None of the implementors currently returns an Query proxy for an invalid query.
2. EntityManager.createNamedQuery(…) is not specified to allow a proxy to be returned which creates an asymmetry between methods of the same purpose. The TCK doesn't seem to check that actually, see [0].
3. Making use of the option not only would potentially break thousands of clients out there, it would also open up another Pandoras box of needed specification: how shall the Query object behave on method calls like setParameter(…)? Does it throw an exception, because of a cause that could have been introduced earlier on?
4. There *are* implementations that throw other exceptions than the IllegalArgumentException specified to be thrown.

So here are my questions boiled down:

1. What benefit does keeping the option provide? Straightening the spec here would provide more consistency and not break any clients and eventually allow queries to be validated safely without extending the API. Beyond that the current flexibility is not used, causes ambiguities, a less discoverable API.
2. Shall implementors be allowed to throw other exceptions than IllegalArgumentException? If so, why? Once again allowing this makes it harder to write portable and concise client code.

Regards,
Ollie

[0] https://bugs.eclipse.org/bugs/show_bug.cgi?id=322579

> <graycol.gif>Oliver Gierke ---07/18/2012 02:21:19 AM---I just did a brief check against OpenJpa 2.2.0. It's currently rejecting both createQuery(String foo
>
> From: Oliver Gierke <ogierke_at_vmware.com>
> To: "jsr338-experts_at_jpa-spec.java.net" <jsr338-experts_at_jpa-spec.java.net>
> Date: 07/18/2012 02:21 AM
> Subject: [jsr338-experts] Re: Ambiguities in exceptions for EntityManager.createQuery(…)
>
>
>
> I just did a brief check against OpenJpa 2.2.0. It's currently rejecting both createQuery(String foo) and createNamedQuery(String name) if the query is invalid or the named query with the name does not even exist.
>
> What benefits does artificially delaying query validation have? It's the opposite of the fail fast principle. Right now you'd actually be the odd one out if you returned a Query object not being executable which severely impacts portability of applications and thus subverts the purpose of having an API like this in the first place. How shall a client actually deal with the information that the method could throw an exception but may be not?
>
> In every case I think the JavaDoc needs to be updated in case we really decide to go for that option (which I'd like to vote against and which is currently no one using and I don't see any benefit) as it's highly misleading currently.
>
> Cheers,
> Ollie
>
> Am 18.07.2012 um 04:14 schrieb Pinaki Poddar:
>
> > Hi,
> > > I'd like to hear the points of view of the JPA implementations represented here as to whether
> > > it would be desirable to strengthen the requirements here or not.
> >
> > The current spec allows certain flexibility for the implementations to parse a query during execution as opposed to construction. In OpenJPA, we prefer such flexibility.
> >
> > Regards --
> >
> > Pinaki Poddar
> > Chair, Apache OpenJPA Project http://openjpa.apache.org/
> > JPA Expert Group Member
> > Application & Integration Middleware
> >
> >
> >
> >
> >
> >
> > <graycol.gif>Linda DeMichiel ---07/17/2012 02:15:29 PM---On 7/17/2012 2:05 AM, Oliver Gierke wrote: > Hi all,
> >
> > From: Linda DeMichiel <linda.demichiel_at_oracle.com>
> > To: jsr338-experts_at_jpa-spec.java.net
> > Cc: Oliver Gierke <ogierke_at_vmware.com>, "thomas.hackel_at_gmail.com" <thomas.hackel_at_gmail.com>
> > Date: 07/17/2012 02:15 PM
> > Subject: [jsr338-experts] Re: Ambiguities in exceptions for EntityManager.createQuery(…)
> >
> >
> >
> >
> >
> > On 7/17/2012 2:05 AM, Oliver Gierke wrote:
> > > Hi all,
> > >
> > > there are multiple (contradicting) sections in the spec defining the error cases of EntityManager.createQuery(…). The JavaDoc in 3.1.1 states:
> > >
> > >> /**
> > >> * …
> > >> * @throws IllegalArgumentException if the query string is
> > >> * found to be invalid
> > >> */
> > >> public Query createQuery(String qlString);
> > >
> > > Below the code sample we find the following section:
> > >
> > >> If the argument to the createQuery method is not a valid Java Persistence query string or a valid CriteriaQuery object, the IllegalArgumentException may be thrown or the query execution will fail and a PersistenceException will be thrown.
> > >
> > > What does "may be thrown" exactly mean? Why is query execution discussed in this context? Does that mean that createQuery(…) might return a Query object that will only fail eventually when being executed?
> >
> > Yes. The provider may defer this check until the point where the query is to be executed.
> >
> > If so, why is this allowed actually? If so how can one reliably check a query string for syntactical correctness?
> > >
> > > I currently have code that tries to validate a query string by calling em.createQuery(…) and catching IllegalArgumentException as defined in the JavaDoc. Unfortunately I've just discovered Hibernate (3.x and 4.x) throwing IllegalStateException in case the query tree seems valid in general but fails being parsed later on.
> > >
> > > em.createQuery("somethingAbsurd") -> IllegalArgumentException
> > > em.createQuery("select disinct(u) from User u") -> IllegalStateException
> > >
> > > I'd like to get a clear picture on what is actually required before filing a ticket against Hibernate. Is it worth to straighten that out in the upcoming version of the spec?
> > >
> >
> > This point was discussed back in the JPA 1.0 days when this decision was made.
> >
> > I'd like to hear the points of view of the JPA implementations represented here as to whether
> > it would be desirable to strengthen the requirements here or not. For example, we could
> > consider adding a query property to indicate that the query string should be validated early.
> >
> > -LInda
> >
> >
> > > Cheers,
> > > Ollie
> > >
> >
> >
>
> --
> /**
> * @author Oliver Gierke - Senior Member Technical Staff
> *
> * @param email ogierke_at_vmware.com
> * @param phone +49-351-30929001
> * @param fax +49-351-418898439
> * @param skype einsdreizehn
> * @see http://www.olivergierke.de
> */
>
>

-- 
/**
 * @author Oliver Gierke - Senior Member Technical Staff
 *
 * @param email ogierke_at_vmware.com
 * @param phone +49-351-30929001
 * @param fax   +49-351-418898439
 * @param skype einsdreizehn
 * @see http://www.olivergierke.de
 */