Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [eclipselink-dev] Tolerance of bad/invalid JPQL
Last week I posted a patch to 440594[1] to resolve this issue and no one has commented on it. I'll give it another day or so, otherwise I'll continue down the path I'm on.

[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=440594


On Mon, Jul 28, 2014 at 10:08 AM, Rick Curtis <curtisr7@xxxxxxxxx> wrote:
> ... Instead a query should be added that will throw an exception the indicates the JPQL is invalid.
Agreed. The intent of my initial prototype was to get something quick and dirty that 'worked-ish' to start a conversation.

> I do not believe there is any proposal to change EclipseLink from failing deployment by default when errors are encountered.
Correct. I intend to preserve the current default EclipseLink behavior. This support will be hidden behind a configuration property for use in specific scenarios.

> Assuming the changes Gordon suggests are taking into account, I'll move my vote from a "-1" to a "0".
10-4.

I'll try to find time early this week to come up with a new patch.

Thanks,
Rick



On Mon, Jul 28, 2014 at 9:47 AM, Tom Ware <tom.ware@xxxxxxxxxx> wrote:
While I still think the Open JPA functionlity we are building portability for is a bug.  I agree we have built flexibility in before.  Assuming the changes Gordon suggests are taking into account, I'll move my vote from a "-1" to a "0".

-Tom


On 25/07/2014 2:26 PM, gordon yorke wrote:
Hello All,
   As Andre mentioned the root issue here is that the structure of the persistence unit configuration is incorrect but we have made changes to EclipseLink in the past to be more flexible as users needed.  I think having a persistence unit property that configures EclipseLink to not halt persistence unit deployment when validating the JPQL is an acceptable approach as long as this property is off by default and only applied by the application in question through it's persistence.xml.  If the desire is to have an application server always set the property by default then I think this is the wrong approach as it can lead to latent errors in a persistence unit. 
This new property is incompatible with validate-only and EclipseLink should override this new property when performing a validate-only deployment.
I do disagree with not adding the query if it is invalid as this will throw a misleading exception.  Instead a query should be added that will throw an exception the indicates the JPQL is invalid.

--Gordon

On 25/07/2014 2:57 PM, Tom Ware wrote:
I am struggling whether this feature helps or hinders the user.  To me it is a flag that says, "don't properly validate my persistence unit", and in the absence of an understanding of the logic behind it, would be inclined to argue the Open JPA functionality was a bug.

What is the reasoning that the Open JPA functionality is desirable?

Does anyone else have an opinion?

-Tom

On 25/07/2014 12:07 PM, Rick Curtis wrote:
My concern with changing some(all?) of the JPQL processing to LAZY is that would typically involve some amount of runtime locking to ensure that processing is consistent when/if there are multiple threads all trying to process JPQL at the same time.

My attached patch essentially just logs a warning when bad JPQL is encountered rather than fail EM creation. Then if someone tries to use an invalid JPQL, they will receive a runtime exception. 


On Thu, Jul 24, 2014 at 12:11 PM, Tom Ware <tom.ware@xxxxxxxxxx> wrote:
There is already some code that builds a placeholder for the query.  (JPQLQuery or JPAQuery, or something like that). I wonder if it would be more productive to look at making that LAZY rather than tolerate bad jpql.

-Tom


On 24/07/2014 12:54 PM, Rick Curtis wrote:
> Why simply not to move the query definition to non-default orm-[pu_name].xml file?
Fair point.

What it boils down to is that I have an existing (OpenJPA based) application that is JPA provider agnostic and I want it to work with EclipseLink. Note, this application sticks strictly to the javax.persistence apis.

EclipseLink differs from OpenJPA in the way that it eagerly processes all NamedQueries at em creation time. OpenJPA delays processing a NamedQuery until the first time that an application tries to use it.




On Thu, Jul 24, 2014 at 9:09 AM, andrei ilitchev <andrei.ilitchev@xxxxxxxxxx> wrote:
Why simply not to move the query definition to non-default orm-[pu_name].xml file?
On 7/24/2014 9:36 AM, Tom Ware wrote:
I wonder if you could use our metadata source to achieve this.

http://wiki.eclipse.org/EclipseLink/Examples/JPA/MetadataSource

i.e. Build a metadata source that would add the query dependent on what persistence unit you were using.

-Tom

On 23/07/2014 4:13 PM, Rick Curtis wrote:
I have a use case where there are multiple distinct persistence units defined in my persistence xml and each unit also has it's own non-default orm-[pu_name].xml file. The wrinkle in this scenario, is that there is also META-INF/orm.xml file that defines a named query that is only valid for one of my persistence units.

Everything works as expected for the persistence unit that the named query is valid for. The other persistence units are unusable as EntityManager creation fails with a PersistenceException[1].

I'm looking into providing a configurable option that would allow a user to tell EclipseLink to tolerate invalid/bad JPQL at EM creation time. I want a WARNING message to be logged when bad JPQL is encountered, but I want the EM to still be created/ mostly functional. If a user then attempts to create a bad named query at runtime, EclipseLink will throw an IllegalArgumentException. 

I've started to prototype support for this behavior, but am not certain if there is a better way to define/access a configuration property. The patch attached below seems to work if I pass this new property (eclipselink.jpql-tolerate-error) to Persistence.createEntityManagerFactory, or as a p.xml property. It doesn't work if the property is passed to emf.createEntityManager(props) call.

Thoughts on my approach? Am I heading in the right direction, or is there a better way to add this support?

Thanks,
Rick

ps -- I'm not sure if this list will allow me to send an attachment?


[1] ...
Caused by: Exception [EclipseLink-28019] (Eclipse Persistence Services - @VERSION@.@QUALIFIER@): org.eclipse.persistence.exceptions.EntityManagerSetupException
Exception Description: Deployment of PersistenceUnit [invalid-named-query] failed. Close all factories for this PersistenceUnit.
Internal Exception: Exception [EclipseLink-0] (Eclipse Persistence Services - @VERSION@.@QUALIFIER@): org.eclipse.persistence.exceptions.JPQLException
Exception Description: Problem compiling [SELECT a FROM Alien a]. 
[14, 19] The abstract schema type 'Alien' is unknown.
at org.eclipse.persistence.exceptions.EntityManagerSetupException.deployFailed(EntityManagerSetupException.java:239)
... 32 more
Caused by: Exception [EclipseLink-0] (Eclipse Persistence Services - @VERSION@.@QUALIFIER@): org.eclipse.persistence.exceptions.JPQLException
Exception Description: Problem compiling [SELECT a FROM Alien a]. 
[14, 19] The abstract schema type 'Alien' is unknown.
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.buildException(HermesParser.java:155)
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.validate(HermesParser.java:347)
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.populateQueryImp(HermesParser.java:278)
at org.eclipse.persistence.internal.jpa.jpql.HermesParser.buildQuery(HermesParser.java:163)
at org.eclipse.persistence.internal.jpa.EJBQueryImpl.buildEJBQLDatabaseQuery(EJBQueryImpl.java:142)
at org.eclipse.persistence.internal.jpa.JPAQuery.processJPQLQuery(JPAQuery.java:221)
at org.eclipse.persistence.internal.jpa.JPAQuery.prepare(JPAQuery.java:182)
at org.eclipse.persistence.queries.DatabaseQuery.prepareInternal(DatabaseQuery.java:621)
at org.eclipse.persistence.internal.sessions.AbstractSession.processJPAQuery(AbstractSession.java:4348)
at org.eclipse.persistence.internal.sessions.AbstractSession.processJPAQueries(AbstractSession.java:4306)
at org.eclipse.persistence.internal.sessions.DatabaseSessionImpl.initializeDescriptors(DatabaseSessionImpl.java:579)
at org.eclipse.persistence.internal.sessions.DatabaseSessionImpl.postConnectDatasource(DatabaseSessionImpl.java:799)
at org.eclipse.persistence.internal.sessions.DatabaseSessionImpl.loginAndDetectDatasource(DatabaseSessionImpl.java:743)
at org.eclipse.persistence.internal.jpa.EntityManagerFactoryProvider.login(EntityManagerFactoryProvider.java:253)
at org.eclipse.persistence.internal.jpa.EntityManagerSetupImpl.deploy(EntityManagerSetupImpl.java:702)
... 30 more

--
Rick Curtis


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



--
Rick Curtis


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



--
Rick Curtis


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



--
Rick Curtis



--
Rick Curtis

Back to the top