Issue 238: Clarify mixing input parameter types for queries

[dazey3]

fixes #238

Signed-off-by: Will Dazey dazeydev.3@gmail.com

[dazey3]

I decided to go with changing "undefined" with "invalid" because the EntityManager API for createQuery(String qlString) states the following:

    @throws IllegalArgumentException if the query string is found to be invalid

I tested current behavior with OpenJPA, EclipseLink, and Hibernate as well. Both OpenJPA/EclipseLink providers currently throw appropriate IllegalArgumentException from EntityManager.createQuery(...) calls when the query is parsed mixing input parameter types.
Hibernate though doesn't fail and creates a query. This seems to violate the current Section 4.6.4: Input Parameters of the specification, but they aren't the reference implementation.

OpenJPA:

org.apache.openjpa.persistence.ArgumentException: Query "SELECT a FROM Story a WHERE a.name = :name AND a.id = ?1" Contains both named and positional parameters this is not allowed by the JPA specification. Detected parameters "[name, 1]".
	at org.apache.openjpa.kernel.jpql.JPQLExpressionBuilder.validateParameters(JPQLExpressionBuilder.java:2462)
...
	at org.apache.openjpa.persistence.EntityManagerImpl.createQuery(EntityManagerImpl.java:102)

NOTE: org.apache.openjpa.persistence.ArgumentException extends java.lang.IllegalArgumentException

EclipseLink:

java.lang.IllegalArgumentException: An exception occurred while creating a query in EntityManager: 
Exception Description: Syntax error parsing [SELECT a FROM Story a WHERE a.name = :name AND a.id = ?1]. 
[37, 42] Named and positional input parameters must not be mixed in a single query.
[54, 56] Named and positional input parameters must not be mixed in a single query.
	at org.eclipse.persistence.internal.jpa.EntityManagerImpl.createQuery(EntityManagerImpl.java:1620)

[beikov]

I don't know how you tested this with Hibernate, but there are compatibility configurations that you must set in order for Hibernate to behave fully spec compliant. I'm pretty sure that if you set those setting, Hibernate will throw an exception as well.

[dazey3]

@beikov

There are compatibility configurations that you must set in order for Hibernate to behave fully spec compliant. I'm pretty sure that if you set those setting, Hibernate will throw an exception as well.

Hibernate is knowingly not spec compliant by default and you must instead set properties to be JPA spec compliant? I have not heard of this before. Mind sharing a link to the documentation for this?

[dazey3]

Regardless, if all providers fail, that adds even more weight that the specification should in fact describe mixing of input parameter types as "invalid" rather than just "undefined"

[beikov]

Here are the various JPA compliance related configurations: https://docs.jboss.org/hibernate/orm/5.6/userguide/html_single/Hibernate_User_Guide.html#configurations-jpa-compliance

[dazey3]

@beikov

Testing with the Hibernate "" property from the documentation you linked did result in Hibernate throwing an exception:

java.lang.IllegalArgumentException: org.hibernate.hql.internal.ast.QuerySyntaxException: Cannot mix positional and named parameters: SELECT a FROM model.Story a WHERE a.name = :name AND a.id = ?1 [SELECT a FROM model.Story a WHERE a.name = :name AND a.id = ?1]
	at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:138)

NOTE: Hibernate 5.2.18 did not change behavior, but 5.5.6 did. This issue must have been fixed at some point.

I understand this Hibernate discussion is a side note to this JPA Spec issue, but does Hibernate have the ability to process the property <property name="hibernate.jpa.compliance" value="true"/>? It would be convenient to simple say "I want to run with a JPA compliant provider requiring only 1 provider specific property".

[sebersole]

@beikov

There are compatibility configurations that you must set in order for Hibernate to behave fully spec compliant. I'm pretty sure that if you set those setting, Hibernate will throw an exception as well.

Hibernate is knowingly not spec compliant by default and you must instead set properties to be JPA spec compliant? I have not heard of this before. Mind sharing a link to the documentation for this?

Is it really that odd that a provider does things better by default? I mean really?

[sebersole]

I understand this Hibernate discussion is a side note to this JPA Spec issue, but does Hibernate have the ability to process the property <property name="hibernate.jpa.compliance" value="true"/>? It would be convenient to simple say "I want to run with a JPA compliant provider requiring only 1 provider specific property".

Yes, I just added this for 6.0 - https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java#L2035-L2051

[dazey3]

@beikov

There are compatibility configurations that you must set in order for Hibernate to behave fully spec compliant. I'm pretty sure that if you set those setting, Hibernate will throw an exception as well.

Hibernate is knowingly not spec compliant by default and you must instead set properties to be JPA spec compliant? I have not heard of this before. Mind sharing a link to the documentation for this?

Is it really that odd that a provider does things better by default? I mean really?

"Better" is a matter of opinion. Also, all I said was that I had not heard of this before for Hibernate and asked for a link to the documentation. Did I say something wrong?

[sebersole]

"Better" is a matter of opinion.

Fair enough. I'll let usage numbers tell the tale of what the general opinion is.

Hibernate existed for many years before JPA. In fact, most parts of JPA were influenced by Hibernate. So some of this is "legacy". Well one - hibernate.jpa.compliance.list is strictly a backwards compatibility setting. Hibernate not only predates JPA, it also predates annotations, and initially there was only XML mapping. Sometimes that XML bias still comes through. This List handling is one.

But honestly, for the most part Hibernate simply does do things better by default in these particular areas.

[sebersole]

And we really should not be having this conversation about Hibernate on the spec GitHub group. Feel free to ping us on Zulip, etc as outlined at https://hibernate.org/community/ if you want to continue discussing

[lukasj]

Is it really that odd that a provider does things better by default? I mean really?

could we leave this sort of comments away from this place and stick to topic, please? IMHO it doesn't matter what the provider does "by default" as long as it satisfies the TCK rules

Thank you.

[lukasj]

The change looks good to me

[sebersole]

could we leave this sort of comments away from this place and stick to topic, please? IMHO it doesn't matter what the provider does "by default" as long as it satisfies the TCK rules

I was answering a direct question asked here. But,

And we really should not be having this conversation about Hibernate on the spec GitHub group...

[dazey3]

@lukasj
Thank you for the review