Skip to content

Commit

Permalink
Fixed failing tests and code to treat '?' parameters as 0-based ordin…
Browse files Browse the repository at this point in the history 
…als and '?<position>' parameters as named 1-based parameters.
  • Loading branch information
@Naros @sebersole
Naros authored and sebersole committed May 6, 2016
1 parent 8a746b9 commit a54da75
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 45 deletions.
5 changes: 5 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/query/internal/AbstractProducedQuery.java
View file
Expand Up @@ -484,6 +484,11 @@ public <T> Parameter<T> getParameter(String name, Class<T> type) {


@Override @Override
public Parameter<?> getParameter(int position) { public Parameter<?> getParameter(int position) {
// lookup jpa-based positional parameters first by name.
if ( parameterMetadata.getPositionalParameterCount() == 0 ) {
return parameterMetadata.getQueryParameter( String.valueOf( position ) );
}
// fallback to oridinal lookup
return parameterMetadata.getQueryParameter( position ); return parameterMetadata.getQueryParameter( position );
} }


Expand Down
39 changes: 20 additions & 19 deletions hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingsImpl.java
View file
Expand Up @@ -20,6 +20,7 @@
import org.hibernate.HibernateException; import org.hibernate.HibernateException;
import org.hibernate.Incubating; import org.hibernate.Incubating;
import org.hibernate.QueryException; import org.hibernate.QueryException;
import org.hibernate.QueryParameterException;
import org.hibernate.dialect.Dialect; import org.hibernate.dialect.Dialect;
import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.jdbc.spi.JdbcServices;
import org.hibernate.engine.query.spi.NamedParameterDescriptor; import org.hibernate.engine.query.spi.NamedParameterDescriptor;
Expand Down Expand Up @@ -196,34 +197,34 @@ public QueryParameterBinding getBinding(String name) {
} }


public QueryParameterBinding getBinding(int position) { public QueryParameterBinding getBinding(int position) {
final boolean isJpaBootstrap = sessionFactory.getSessionFactoryOptions().isJpaBootstrap();

QueryParameterBinding binding = null; QueryParameterBinding binding = null;
if ( parameterMetadata != null ) { if ( parameterMetadata != null ) {
if ( isJpaBootstrap && parameterMetadata.getPositionalParameterCount() == 0 ) { if ( parameterMetadata.getPositionalParameterCount() == 0 ) {
// no positional parameters, assume jpa named. // no positional parameters, assume jpa named.
binding = locateBinding( String.valueOf( position ) ); binding = locateBinding( String.valueOf( position ) );
} }
else { else {
final int positionIndex = ( isJpaBootstrap ? position - 1 : position ); try {
final int parameterIndex = ( isJpaBootstrap ? position : position + 1 ); if ( position < positionalParameterBindings.size() ) {

binding = positionalParameterBindings.get( position );
if ( positionIndex < positionalParameterBindings.size() ) { if ( binding == null ) {
binding = positionalParameterBindings.get( positionIndex ); // metadata parameters are 1-based
if ( binding == null ) { binding = makeBinding( parameterMetadata.getQueryParameter( position + 1 ) );
positionalParameterBindings.set( position, binding );
}
}
else {
for ( int i = 0; i < position - positionalParameterBindings.size(); i++ ) {
positionalParameterBindings.add( null );
}
// metadata parameters are 1-based // metadata parameters are 1-based
binding = makeBinding( parameterMetadata.getQueryParameter( parameterIndex ) ); QueryParameter queryParameter = parameterMetadata.getQueryParameter( position + 1 );
positionalParameterBindings.set( positionIndex, binding ); binding = makeBinding( queryParameter );
positionalParameterBindings.add( binding );
} }
} }
else { catch ( QueryParameterException e ) {
for ( int i = 0; i < positionIndex - positionalParameterBindings.size(); i++ ) { // treat this as null binding
positionalParameterBindings.add( null );
}
// metadata parameters are 1-based
QueryParameter queryParameter = parameterMetadata.getQueryParameter( parameterIndex );
binding = makeBinding( queryParameter );
positionalParameterBindings.add( binding );
} }
} }
} }
Expand Down
69 changes: 43 additions & 26 deletions hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/query/QueryTest.java
View file
Expand Up @@ -12,11 +12,18 @@
import java.util.GregorianCalendar; import java.util.GregorianCalendar;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import javax.persistence.*; import javax.persistence.CacheRetrieveMode;

import javax.persistence.CacheStoreMode;
import org.junit.Test; import javax.persistence.EntityManager;
import javax.persistence.Parameter;
import javax.persistence.PersistenceException;
import javax.persistence.Query;
import javax.persistence.TemporalType;
import javax.persistence.Tuple;


import junit.framework.Assert;
import org.hibernate.Hibernate; import org.hibernate.Hibernate;
import org.hibernate.QueryParameterException;
import org.hibernate.cfg.AvailableSettings; import org.hibernate.cfg.AvailableSettings;
import org.hibernate.dialect.Oracle8iDialect; import org.hibernate.dialect.Oracle8iDialect;
import org.hibernate.dialect.PostgreSQL9Dialect; import org.hibernate.dialect.PostgreSQL9Dialect;
Expand All @@ -27,12 +34,10 @@
import org.hibernate.jpa.test.Item; import org.hibernate.jpa.test.Item;
import org.hibernate.jpa.test.Wallet; import org.hibernate.jpa.test.Wallet;
import org.hibernate.stat.Statistics; import org.hibernate.stat.Statistics;

import junit.framework.Assert;

import org.hibernate.testing.SkipForDialect; import org.hibernate.testing.SkipForDialect;
import org.hibernate.testing.SkipForDialects; import org.hibernate.testing.SkipForDialects;
import org.hibernate.testing.TestForIssue; import org.hibernate.testing.TestForIssue;
import org.junit.Test;


import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertNull;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -111,16 +116,16 @@ public void testNullPositionalParameter() throws Exception {
Item item = new Item( "Mouse", "Micro$oft mouse" ); Item item = new Item( "Mouse", "Micro$oft mouse" );
em.persist( item ); em.persist( item );
Query q = em.createQuery( "from Item i where i.intVal=?" ); Query q = em.createQuery( "from Item i where i.intVal=?" );
q.setParameter( 1, null ); q.setParameter( 0, null );
List results = q.getResultList(); List results = q.getResultList();
// null != null // null != null
assertEquals( 0, results.size() ); assertEquals( 0, results.size() );
q = em.createQuery( "from Item i where i.intVal is null and ? is null" ); q = em.createQuery( "from Item i where i.intVal is null and ? is null" );
q.setParameter( 1, null ); q.setParameter( 0, null );
results = q.getResultList(); results = q.getResultList();
assertEquals( 1, results.size() ); assertEquals( 1, results.size() );
q = em.createQuery( "from Item i where i.intVal is null or i.intVal = ?" ); q = em.createQuery( "from Item i where i.intVal is null or i.intVal = ?" );
q.setParameter( 1, null ); q.setParameter( 0, null );
results = q.getResultList(); results = q.getResultList();
assertEquals( 1, results.size() ); assertEquals( 1, results.size() );
em.getTransaction().rollback(); em.getTransaction().rollback();
Expand Down Expand Up @@ -316,16 +321,16 @@ public void testNativeQueryNullPositionalParameter() throws Exception {
// native queries don't seem to flush by default ?!? // native queries don't seem to flush by default ?!?
em.flush(); em.flush();
Query q = em.createNativeQuery( "select * from Item i where i.intVal=?" ); Query q = em.createNativeQuery( "select * from Item i where i.intVal=?" );
q.setParameter( 1, null ); q.setParameter( 0, null );
List results = q.getResultList(); List results = q.getResultList();
// null != null // null != null
assertEquals( 0, results.size() ); assertEquals( 0, results.size() );
q = em.createNativeQuery( "select * from Item i where i.intVal is null and ? is null" ); q = em.createNativeQuery( "select * from Item i where i.intVal is null and ? is null" );
q.setParameter( 1, null ); q.setParameter( 0, null );
results = q.getResultList(); results = q.getResultList();
assertEquals( 1, results.size() ); assertEquals( 1, results.size() );
q = em.createNativeQuery( "select * from Item i where i.intVal is null or i.intVal = ?" ); q = em.createNativeQuery( "select * from Item i where i.intVal is null or i.intVal = ?" );
q.setParameter( 1, null ); q.setParameter( 0, null );
results = q.getResultList(); results = q.getResultList();
assertEquals( 1, results.size() ); assertEquals( 1, results.size() );
em.getTransaction().rollback(); em.getTransaction().rollback();
Expand Down Expand Up @@ -549,8 +554,9 @@ public void testJpaPositionalParameters() {
Query query = em.createQuery( "from Item item where item.name =?1 or item.descr = ?1" ); Query query = em.createQuery( "from Item item where item.name =?1 or item.descr = ?1" );
Parameter p1 = query.getParameter( 1 ); Parameter p1 = query.getParameter( 1 );
Assert.assertNotNull( p1 ); Assert.assertNotNull( p1 );
Assert.assertNotNull( p1.getPosition() ); // in 5.2, '?<position' parameters are named while '?' are position-based.
Assert.assertNull( p1.getName() ); Assert.assertNotNull( p1.getName() );
Assert.assertNull( p1.getPosition() );


em.getTransaction().commit(); em.getTransaction().commit();
em.close(); em.close();
Expand Down Expand Up @@ -802,7 +808,7 @@ public void testPositionalParameterForms() throws Exception {


// finally using hql-style positional parameter // finally using hql-style positional parameter
query = em.createQuery( "select w from Wallet w where w.brand = ?" ); query = em.createQuery( "select w from Wallet w where w.brand = ?" );
query.setParameter( 1, "Lacoste" ); query.setParameter( 0, "Lacoste" );
w = (Wallet) query.getSingleResult(); w = (Wallet) query.getSingleResult();
assertNotNull( w ); assertNotNull( w );


Expand All @@ -822,20 +828,31 @@ public void testPositionalParameterWithUserError() throws Exception {
em.persist( w ); em.persist( w );
em.flush(); em.flush();



// using jpa-style, position index should match syntax '?<position'.
Query query = em.createQuery( "select w from Wallet w where w.brand = ?1 and w.model = ?3" ); Query jpaQuery = em.createQuery( "select w from Wallet w where w.brand = ?1 and w.model = ?3" );
query.setParameter( 1, "Lacoste" ); jpaQuery.setParameter( 1, "Lacoste" );
try { try {
query.setParameter( 2, "Expensive" ); jpaQuery.setParameter( 2, "Expensive" );
fail( "Should fail due to a user error in parameters" ); fail( "Should fail due to a user error in parameters" );
} }
catch ( IllegalArgumentException e ) { catch( IllegalArgumentException e ) {
//success // success, expected
}

// using hql-style, should be 0-based
Query hqlQuery = em.createQuery( "select w from Wallet w where w.brand = ? and w.model = ?" );
try {
hqlQuery.setParameter( 1, "Lacoste" );
hqlQuery.setParameter( 2, "Expensive" );
fail( "Should fail due to a user error in parameters" );
} }
finally { catch( QueryParameterException e ) {
em.getTransaction().rollback(); // success expected
em.close(); e.printStackTrace();
} }

em.getTransaction().rollback();
em.close();
} }


@Test @Test
Expand All @@ -850,7 +867,7 @@ public void testNativeQuestionMarkParameter() throws Exception {
em.getTransaction().commit(); em.getTransaction().commit();
em.getTransaction().begin(); em.getTransaction().begin();
Query query = em.createNativeQuery( "select * from Wallet w where w.brand = ?", Wallet.class ); Query query = em.createNativeQuery( "select * from Wallet w where w.brand = ?", Wallet.class );
query.setParameter( 1, "Lacoste" ); query.setParameter( 0, "Lacoste" );
w = (Wallet) query.getSingleResult(); w = (Wallet) query.getSingleResult();
assertNotNull( w ); assertNotNull( w );
em.remove( w ); em.remove( w );
Expand All @@ -875,7 +892,7 @@ public void testNativeQueryWithPositionalParameter() {
assertNotNull( item ); assertNotNull( item );
assertEquals( "Micro$oft mouse", item.getDescr() ); assertEquals( "Micro$oft mouse", item.getDescr() );
query = em.createNativeQuery( "select * from Item where name = ?", Item.class ); query = em.createNativeQuery( "select * from Item where name = ?", Item.class );
query.setParameter( 1, "Mouse" ); query.setParameter( 0, "Mouse" );
item = (Item) query.getSingleResult(); item = (Item) query.getSingleResult();
assertNotNull( item ); assertNotNull( item );
assertEquals( "Micro$oft mouse", item.getDescr() ); assertEquals( "Micro$oft mouse", item.getDescr() );
Expand Down

0 comments on commit a54da75

Please sign in to comment.