diff --git a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java index 86309ccb3d..18503e44d2 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java @@ -612,9 +612,9 @@ public interface CoreMessageLogger extends BasicLogger { @Message(value = "Package not found or wo package-info.java: %s", id = 194) void packageNotFound(String packageName); - @LogMessage(level = WARN) - @Message(value = "Parameter position [%s] occurred as both JPA and Hibernate positional parameter", id = 195) - void parameterPositionOccurredAsBothJpaAndHibernatePositionalParameter(Integer position); +// @LogMessage(level = WARN) +// @Message(value = "Parameter position [%s] occurred as both JPA and Hibernate positional parameter", id = 195) +// void parameterPositionOccurredAsBothJpaAndHibernatePositionalParameter(Integer position); @LogMessage(level = ERROR) @Message(value = "Error parsing XML (%s) : %s", id = 196) diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/EntityManagerMessageLogger.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/EntityManagerMessageLogger.java index 8049878292..1adb338e3e 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/EntityManagerMessageLogger.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/EntityManagerMessageLogger.java @@ -108,4 +108,13 @@ public interface EntityManagerMessageLogger extends CoreMessageLogger { "as the entity (type=%s, id=%s) does not exist", id = 15013 ) void ignoringEntityNotFound( String entityName, String identifier); + @LogMessage( level = WARN ) + @Message( + value = "DEPRECATION - attempt to refer to JPA positional parameter [?%1$s] using String name [\"%1$s\"] " + + "rather than int position [%1$s] (generally in Query#setParameter, Query#getParameter or " + + "Query#getParameterValue calls). Hibernate previously allowed such usage, but it is considered " + + "deprecated.", + id = 15014 + ) + void deprecatedJpaPositionalParameterAccess(Integer jpaPositionalParameter); } diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/QueryImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/QueryImpl.java index 783f8f875c..77fd656ad3 100755 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/QueryImpl.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/QueryImpl.java @@ -23,10 +23,13 @@ */ package org.hibernate.jpa.internal; -import static javax.persistence.TemporalType.DATE; -import static javax.persistence.TemporalType.TIME; -import static javax.persistence.TemporalType.TIMESTAMP; - +import javax.persistence.NoResultException; +import javax.persistence.NonUniqueResultException; +import javax.persistence.ParameterMode; +import javax.persistence.PersistenceException; +import javax.persistence.Query; +import javax.persistence.TemporalType; +import javax.persistence.TypedQuery; import java.util.Calendar; import java.util.Collection; import java.util.Collections; @@ -36,27 +39,18 @@ import java.util.List; import java.util.Map; import java.util.Set; -import javax.persistence.NoResultException; -import javax.persistence.NonUniqueResultException; -import javax.persistence.ParameterMode; -import javax.persistence.PersistenceException; -import javax.persistence.Query; -import javax.persistence.StoredProcedureQuery; -import javax.persistence.TemporalType; -import javax.persistence.TypedQuery; +import org.jboss.logging.Logger; import org.hibernate.CacheMode; import org.hibernate.FlushMode; import org.hibernate.HibernateException; import org.hibernate.LockMode; import org.hibernate.SQLQuery; -import org.hibernate.Session; import org.hibernate.TypeMismatchException; import org.hibernate.engine.query.spi.NamedParameterDescriptor; import org.hibernate.engine.query.spi.OrdinalParameterDescriptor; import org.hibernate.engine.query.spi.ParameterMetadata; import org.hibernate.engine.spi.SessionFactoryImplementor; -import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.hql.internal.QueryExecutionRequestException; import org.hibernate.internal.SQLQueryImpl; import org.hibernate.jpa.AvailableSettings; @@ -69,7 +63,10 @@ import org.hibernate.jpa.spi.ParameterBind; import org.hibernate.jpa.spi.ParameterRegistration; import org.hibernate.type.CompositeCustomType; import org.hibernate.type.Type; -import org.jboss.logging.Logger; + +import static javax.persistence.TemporalType.DATE; +import static javax.persistence.TemporalType.TIME; +import static javax.persistence.TemporalType.TIMESTAMP; /** * Hibernate implementation of both the {@link Query} and {@link TypedQuery} contracts. @@ -83,7 +80,6 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, public static final EntityManagerMessageLogger LOG = Logger.getMessageLogger(EntityManagerMessageLogger.class, QueryImpl.class.getName()); private org.hibernate.Query query; - private Set jpaPositionalIndices; public QueryImpl(org.hibernate.Query query, AbstractEntityManagerImpl em) { this( query, em, Collections.emptyMap() ); @@ -104,6 +100,8 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, throw new IllegalStateException( "Unknown query type for parameter extraction" ); } + boolean hadJpaPositionalParameters = false; + final ParameterMetadata parameterMetadata = org.hibernate.internal.AbstractQueryImpl.class.cast( query ).getParameterMetadata(); // extract named params @@ -118,24 +116,30 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, else if ( descriptor.getExpectedType() != null ) { javaType = descriptor.getExpectedType().getReturnedClass(); } - registerParameter( new ParameterRegistrationImpl( this, query, name, javaType ) ); + if ( descriptor.isJpaStyle() ) { - if ( jpaPositionalIndices == null ) { - jpaPositionalIndices = new HashSet(); - } - jpaPositionalIndices.add( Integer.valueOf( name ) ); + hadJpaPositionalParameters = true; + final Integer position = Integer.valueOf( name ); + registerParameter( new JpaPositionalParameterRegistrationImpl( this, query, position, javaType ) ); + } + else { + registerParameter( new ParameterRegistrationImpl( this, query, name, javaType ) ); } } - // extract positional parameters + if ( hadJpaPositionalParameters ) { + if ( parameterMetadata.getOrdinalParameterCount() > 0 ) { + throw new IllegalArgumentException( + "Cannot mix JPA positional parameters and native Hibernate positional/ordinal parameters" + ); + } + } + + // extract Hibernate native positional parameters for ( int i = 0, max = parameterMetadata.getOrdinalParameterCount(); i < max; i++ ) { final OrdinalParameterDescriptor descriptor = parameterMetadata.getOrdinalParameterDescriptor( i + 1 ); Class javaType = descriptor.getExpectedType() == null ? null : descriptor.getExpectedType().getReturnedClass(); registerParameter( new ParameterRegistrationImpl( this, query, i+1, javaType ) ); - Integer position = descriptor.getOrdinalPosition(); - if ( jpaPositionalIndices != null && jpaPositionalIndices.contains(position) ) { - LOG.parameterPositionOccurredAsBothJpaAndHibernatePositionalParameter(position); - } } } @@ -162,7 +166,7 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, private ParameterBind bind; - private ParameterRegistrationImpl( + protected ParameterRegistrationImpl( Query jpaQuery, org.hibernate.Query nativeQuery, String name, @@ -174,7 +178,7 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, this.position = null; } - private ParameterRegistrationImpl( + protected ParameterRegistrationImpl( Query jpaQuery, org.hibernate.Query nativeQuery, Integer position, @@ -186,6 +190,11 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, this.name = null; } + @Override + public boolean isJpaPositionalParameter() { + return false; + } + @Override public Query getQuery() { return jpaQuery; @@ -304,6 +313,39 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, } } + /** + * Specialized handling for JPA "positional parameters". + * + * @param The parameter type type. + */ + public static class JpaPositionalParameterRegistrationImpl extends ParameterRegistrationImpl { + final Integer position; + + protected JpaPositionalParameterRegistrationImpl( + Query jpaQuery, + org.hibernate.Query nativeQuery, + Integer position, + Class javaType) { + super( jpaQuery, nativeQuery, position.toString(), javaType ); + this.position = position; + } + + @Override + public String getName() { + return null; + } + + @Override + public Integer getPosition() { + return position; + } + + @Override + public boolean isJpaPositionalParameter() { + return true; + } + } + public org.hibernate.Query getHibernateQuery() { return query; } @@ -465,11 +507,6 @@ public class QueryImpl extends AbstractQueryImpl implements TypedQuery, } } - @Override - protected boolean isJpaPositionalParameter(int position) { - return jpaPositionalIndices != null && jpaPositionalIndices.contains( position ); - } - @Override @SuppressWarnings({ "unchecked" }) public T unwrap(Class tClass) { diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java index b335785d3b..23bd390de3 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java @@ -450,11 +450,6 @@ public class StoredProcedureQueryImpl extends BaseQueryImpl implements StoredPro // parameters ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - @Override - protected boolean isJpaPositionalParameter(int position) { - return false; - } - public ProcedureCall getHibernateProcedureCall() { return procedureCall; } @@ -487,6 +482,11 @@ public class StoredProcedureQueryImpl extends BaseQueryImpl implements StoredPro return nativeParamRegistration.getType(); } + @Override + public boolean isJpaPositionalParameter() { + return getPosition() != null; + } + @Override public Query getQuery() { return query; diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java index 6a78f200ef..0372ab2559 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java @@ -429,21 +429,50 @@ public abstract class BaseQueryImpl implements Query { @SuppressWarnings("unchecked") protected ParameterRegistration findParameterRegistration(String parameterName) { - return (ParameterRegistration) getParameter( parameterName ); + final Integer jpaPositionalParameter = toNumberOrNull( parameterName ); + + if ( parameterRegistrations != null ) { + for ( ParameterRegistration param : parameterRegistrations ) { + if ( parameterName.equals( param.getName() ) ) { + return (ParameterRegistration) param; + } + + // legacy allowance of the application to access the parameter using the position as a String + if ( param.isJpaPositionalParameter() && jpaPositionalParameter != null ) { + if ( jpaPositionalParameter.equals( param.getPosition() ) ) { + LOG.deprecatedJpaPositionalParameterAccess( jpaPositionalParameter ); + } + return (ParameterRegistration) param; + } + } + } + throw new IllegalArgumentException( "Parameter with that name [" + parameterName + "] did not exist" ); + } + + private Integer toNumberOrNull(String parameterName) { + try { + return Integer.valueOf( parameterName ); + } + catch (NumberFormatException e) { + return null; + } } @SuppressWarnings("unchecked") protected ParameterRegistration findParameterRegistration(int parameterPosition) { - if ( isJpaPositionalParameter( parameterPosition ) ) { - return findParameterRegistration( Integer.toString( parameterPosition ) ); - } - else { - return (ParameterRegistration) getParameter( parameterPosition ); + if ( parameterRegistrations != null ) { + for ( ParameterRegistration param : parameterRegistrations ) { + if ( param.getPosition() == null ) { + continue; + } + if ( parameterPosition == param.getPosition() ) { + return (ParameterRegistration) param; + } + } } + throw new IllegalArgumentException( "Parameter with that position [" + parameterPosition + "] did not exist" ); } - protected abstract boolean isJpaPositionalParameter(int position); - protected static class ParameterBindImpl implements ParameterBind { private final T value; private final TemporalType specifiedTemporalType; @@ -664,21 +693,14 @@ public abstract class BaseQueryImpl implements Query { @Override public Parameter getParameter(String name) { checkOpen( false ); - if ( parameterRegistrations != null ) { - for ( ParameterRegistration param : parameterRegistrations ) { - if ( name.equals( param.getName() ) ) { - return param; - } - } - } - throw new IllegalArgumentException( "Parameter with that name [" + name + "] did not exist" ); + return findParameterRegistration( name ); } @Override @SuppressWarnings("unchecked") public Parameter getParameter(String name, Class type) { checkOpen( false ); - Parameter param = getParameter( name ); + Parameter param = findParameterRegistration( name ); if ( param.getParameterType() != null ) { // we were able to determine the expected type during analysis, so validate it here @@ -698,21 +720,8 @@ public abstract class BaseQueryImpl implements Query { @Override public Parameter getParameter(int position) { - if ( isJpaPositionalParameter( position ) ) { - return getParameter( Integer.toString( position ) ); - } checkOpen( false ); - if ( parameterRegistrations != null ) { - for ( ParameterRegistration param : parameterRegistrations ) { - if ( param.getPosition() == null ) { - continue; - } - if ( position == param.getPosition() ) { - return param; - } - } - } - throw new IllegalArgumentException( "Parameter with that position [" + position + "] did not exist" ); + return findParameterRegistration( position ); } @Override @@ -720,7 +729,7 @@ public abstract class BaseQueryImpl implements Query { public Parameter getParameter(int position, Class type) { checkOpen( false ); - Parameter param = getParameter( position ); + Parameter param = findParameterRegistration( position ); if ( param.getParameterType() != null ) { // we were able to determine the expected type during analysis, so validate it here diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/ParameterRegistration.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/ParameterRegistration.java index d1df5bdd80..eae481650f 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/ParameterRegistration.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/ParameterRegistration.java @@ -39,6 +39,17 @@ import javax.persistence.TemporalType; * @author Steve Ebersole */ public interface ParameterRegistration extends Parameter { + /** + * JPA has a different definition of positional parameters than what legacy Hibernate HQL had. In JPA, + * the parameter holders are labelled (named :/). At any rate the semantics are different and we often + * need to understand which we are dealing with (and applications might too). + * + * @return {@code true} if this is a JPA-style positional parameter; {@code false} would indicate + * we have either a named parameter ({@link #getName()} would return a non-{@code null} value) or a native + * Hibernate positional parameter. + */ + public boolean isJpaPositionalParameter(); + /** * Access to the query that this parameter belongs to. * diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/query/QueryTest.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/query/QueryTest.java index cc4eca89bc..fc6123d3e9 100644 --- a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/query/QueryTest.java +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/query/QueryTest.java @@ -187,6 +187,21 @@ public class QueryTest extends BaseEntityManagerFunctionalTestCase { em.close(); } + @Test + public void testJpaPositionalParameters() { + EntityManager em = getOrCreateEntityManager(); + em.getTransaction().begin(); + + Query query = em.createQuery( "from Item item where item.name =?1 or item.descr = ?1" ); + Parameter p1 = query.getParameter( 1 ); + Assert.assertNotNull( p1 ); + Assert.assertNotNull( p1.getPosition() ); + Assert.assertNull( p1.getName() ); + + em.getTransaction().commit(); + em.close(); + } + @Test public void testParameterList() throws Exception { final Item item = new Item( "Mouse", "Micro$oft mouse" ); @@ -225,6 +240,7 @@ public class QueryTest extends BaseEntityManagerFunctionalTestCase { params = new ArrayList(); params.add( item.getName() ); params.add( item2.getName() ); + // deprecated usage of positional parameter by String q.setParameter( "1", params ); result = q.getResultList(); assertNotNull( result ); @@ -277,6 +293,7 @@ public class QueryTest extends BaseEntityManagerFunctionalTestCase { params = new ArrayList(); params.add( item.getName() ); params.add( item2.getName() ); + // deprecated usage of positional parameter by String q.setParameter( "1", params ); result = q.getResultList(); assertNotNull( result ); @@ -424,6 +441,7 @@ public class QueryTest extends BaseEntityManagerFunctionalTestCase { // next using jpa-style positional parameter, but as a name (which is how Hibernate core treats these query = em.createQuery( "select w from Wallet w where w.brand = ?1" ); + // deprecated usage of positional parameter by String query.setParameter( "1", "Lacoste" ); w = (Wallet) query.getSingleResult(); assertNotNull( w );