Hi James,
the ClassCastException is gone now. There are 6 issues
remaining. I have created a bug for this:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=312051
A proposed fix attached. To make the tests positive, I just
changed assertInvalidQuery to assertValidQuery.
Could you please have a look?
Thanks,
Adrian
Von: James Sutherland
[mailto:JAMES.SUTHERLAND@xxxxxxxxxx]
Gesendet: Donnerstag, 6. Mai 2010 14:59
An: Goerler, Adrian
Cc: Dev mailing list for Eclipse Persistence Services
Betreff: RE: AW: [eclipselink-dev] regressions in the WDF test suite
>> java.lang.ClassCastException:
java.lang.String cannot be cast to java.lang.Enum
The test expects an error, and it getting one, so we
could just broaden the error allowed. We could change the error to throw some
type of ConversionException instead, but the class cast does seem to indicate
pretty clearly what is wrong.
I think the JPA Query should be catching any errors a
re-throwing as QueryException or PersistenceException though, so this could
probably be fixed.
We could also just pass through the string values in
the enum converter.
>> I agree. Actually, I think this should have
been done as part of the changes causing the test failure. Who will do this?
I am not familiar with these tests, or their framework,
but can try to update them.
>> The WDF test suite contains many (~160) tests
that assert that invalid queries are rejected. Would you say that in general
such tests are not suitable for EclipseLink?
I would leave them there. Negative tests are useful
to ensure things fail with good exceptions instead of badly.
Obviously if the test is testing something that is
valid, but just not supported yet, when it is supported, then it will need to
be changed to a positive test.
>> I understand that EclipseLink is deferring
type checking to the database now.
No, EclipseLink has always done automatic type
conversion within its Expressions. There were just some (not all) cases in
JPQL where typing errors could be thrown.
In _expression_ queries if you do "where id =
5" and id was a long, EclipseLink would automatically convert the int to
long, instead of throwing a typing error.
JPA, JPQL and EclipseLink have always supported more
than is supported by every database. For example CASE is not supported on
Derby, but is allowed in JPQL. We do our best to isolate any database
differences or limitations in our DatabasePlatform abstraction,
but there will always be some things not supported on
some databases. Most things however are supported on most databases, so we
should not prevent users having access to this functionality just because one
database
does not support it.
-----Original
Message-----
From: Goerler, Adrian [mailto:adrian.goerler@xxxxxxx]
Sent: Thursday, May 06, 2010 3:58 AM
To: James Sutherland
Cc: Dev mailing list for Eclipse Persistence Services
Subject: AW: [eclipselink-dev] regressions in the WDF test suite
Hi
James,
Ø All of these failures seem to be negative tests
that now pass.
Not
quite. The tests TestConditionalExpression.testBetweenHandling3/10 fail in
createQuery with a ClassCastException. The query executed is
"SELECT
p FROM Person p WHERE p.city.type BETWEEN 'bla' AND 'bla'"
City.type
is an enum.
Admittedly,
the query is nonsense and the tests expects it to fail. But I’d consider
a ClassCastException an issue.
java.lang.ClassCastException:
java.lang.String cannot be cast to java.lang.Enum
at org.eclipse.persistence.mappings.converters.EnumTypeConverter.convertObjectValueToDataValue(EnumTypeConverter.java:137)
at
org.eclipse.persistence.mappings.foundation.AbstractDirectMapping.getFieldValue(AbstractDirectMapping.java:808)
at org.eclipse.persistence.internal.expressions.QueryKeyExpression.getFieldValue(QueryKeyExpression.java:372)
at
org.eclipse.persistence.internal.expressions.ConstantExpression.printSQL(ConstantExpression.java:122)
at org.eclipse.persistence.expressions.ExpressionOperator.printCollection(ExpressionOperator.java:1922)
at
org.eclipse.persistence.internal.expressions.FunctionExpression.printSQL(FunctionExpression.java:447)
at org.eclipse.persistence.expressions.ExpressionOperator.printDuo(ExpressionOperator.java:1962)
at
org.eclipse.persistence.internal.expressions.CompoundExpression.printSQL(CompoundExpression.java:277)
at
org.eclipse.persistence.internal.expressions.ExpressionSQLPrinter.translateExpression(ExpressionSQLPrinter.java:306)
at
org.eclipse.persistence.internal.expressions.ExpressionSQLPrinter.printExpression(ExpressionSQLPrinter.java:129)
at org.eclipse.persistence.internal.expressions.SQLSelectStatement.printSQL(SQLSelectStatement.java:1451)
at
org.eclipse.persistence.internal.databaseaccess.DatabasePlatform.printSQLSelectStatement(DatabasePlatform.java:2823)
at org.eclipse.persistence.platform.database.MySQLPlatform.printSQLSelectStatement(MySQLPlatform.java:618)
at
org.eclipse.persistence.internal.expressions.SQLSelectStatement.buildCall(SQLSelectStatement.java:755)
at
org.eclipse.persistence.internal.expressions.SQLSelectStatement.buildCall(SQLSelectStatement.java:765)
at
org.eclipse.persistence.descriptors.ClassDescriptor.buildCallFromStatement(ClassDescriptor.java:671)
at
org.eclipse.persistence.internal.queries.StatementQueryMechanism.setCallFromStatement(StatementQueryMechanism.java:386)
at
org.eclipse.persistence.internal.queries.StatementQueryMechanism.prepareSelectAllRows(StatementQueryMechanism.java:312)
at org.eclipse.persistence.internal.queries.ExpressionQueryMechanism.prepareSelectAllRows(ExpressionQueryMechanism.java:1557)
at
org.eclipse.persistence.queries.ReadAllQuery.prepareSelectAllRows(ReadAllQuery.java:674)
at org.eclipse.persistence.queries.ReadAllQuery.prepare(ReadAllQuery.java:621)
at
org.eclipse.persistence.queries.DatabaseQuery.checkPrepare(DatabaseQuery.java:508)
at
org.eclipse.persistence.queries.ObjectLevelReadQuery.checkPrepare(ObjectLevelReadQuery.java:733)
at org.eclipse.persistence.queries.DatabaseQuery.prepareCall(DatabaseQuery.java:1575)
at
org.eclipse.persistence.internal.jpa.EJBQueryImpl.buildEJBQLDatabaseQuery(EJBQueryImpl.java:247)
at org.eclipse.persistence.internal.jpa.EJBQueryImpl.buildEJBQLDatabaseQuery(EJBQueryImpl.java:180)
at
org.eclipse.persistence.internal.jpa.EJBQueryImpl.<init>(EJBQueryImpl.java:132)
at org.eclipse.persistence.internal.jpa.EJBQueryImpl.<init>(EJBQueryImpl.java:116)
at
org.eclipse.persistence.internal.jpa.EntityManagerImpl.createQuery(EntityManagerImpl.java:1348)
at
org.eclipse.persistence.testing.tests.wdf.jpa1.query.QueryTest.assertInvalidQuery(QueryTest.java:167)
at
org.eclipse.persistence.testing.tests.wdf.jpa1.query.TestConditionalExpressions.testBetweenHandling3(TestConditionalExpressions.java:403)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:73)
at org.eclipse.persistence.testing.framework.wdf.SkipBugzillaTestRunner.runChild(SkipBugzillaTestRunner.java:176)
at
org.eclipse.persistence.testing.framework.wdf.SkipBugzillaTestRunner.runChild(SkipBugzillaTestRunner.java:37)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:180)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:41)
at org.junit.runners.ParentRunner$1.evaluate(ParentRunner.java:173)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
at org.junit.runners.ParentRunner.run(ParentRunner.java:220)
at org.eclipse.persistence.testing.framework.wdf.SkipBugzillaTestRunner.run(SkipBugzillaTestRunner.java:49)
at
org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:46)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
-
We enhanced
our JPQL support to allow a large number of valid queries, or defer to the
database to decide if the query is valid or not.
-
The tests
should either be removed, or change to verify they JPQL works, instead of
fails.
I
agree. Actually, I think this should have been done as part of the changes
causing the test failure. Who will do this?
The
WDF test suite contains many (~160) tests that assert that invalid queries are
rejected. Would you say that in general such tests are not suitable for
EclipseLink?
I
still don’t have a clear understanding of the guarantees given by the
query validation in EclipseLink.
I
have learned that EclipseLink supports a wider range of queries as standard
JPQL. Also, I understand that EclipseLink is deferring type checking to the
database now. I think this makes a lot of sense if you consider the ORM mapper
aspect of EclipseLink only.
But
considering JPA and JPQL a means for database-abstraction, from my point
of view, deferring validation to the database will hamper the portability.
Therefore I am not sure if deferring the checks to the DB is the right
approach. I think there is much value in strong type checking. I’d have
preferred explicit casts to leverage type conversion on the database.
-Adrian
Adrian
Görler
SAP AG
Pflichtangaben/Mandatory Disclosure Statements: http://www.sap.com/company/legal/impressum.epx
Von:
eclipselink-dev-bounces@xxxxxxxxxxx
[mailto:eclipselink-dev-bounces@xxxxxxxxxxx] Im Auftrag von James
Sutherland
Gesendet: Mittwoch, 5. Mai 2010 19:10
An: Dev mailing list for Eclipse Persistence Services
Betreff: RE: [eclipselink-dev] regressions in the WDF test suite
We enhanced
our JPQL support to allow a large number of valid queries, or defer to the
database to decide if the query is valid or not.
All of
these failures seem to be negative tests that now pass. This is
intended. The tests should either be removed, or change to verify they
JPQL works, instead of fails.
-----Original
Message-----
From: Goerler, Adrian [mailto:adrian.goerler@xxxxxxx]
Sent: Wednesday, May 05, 2010 12:58 PM
To: Dev mailing list for Eclipse Persistence Services
Subject: [eclipselink-dev] regressions in the WDF test suite
Hi,
I am
observing regression in the WDF test suite:
(This
is actually a zip file containing the html-junit-report).
Please
check, whoever did changes in the BETWEEN or GROUP BY realm.
-Adrian
Adrian
Görler
SAP AG
Pflichtangaben/Mandatory Disclosure Statements: http://www.sap.com/company/legal/impressum.epx