HHH-8400 - Positional parameters defined on NamedStoredProcedureQuery not applied

hibernate-core/hibernate-core.gradle

@@ -33,6 +33,7 @@ dependencies {
     testRuntime( 'jaxen:jaxen:1.1' )
     testRuntime( libraries.javassist )
     testRuntime( libraries.unified_el )
+    testRuntime( libraries.derby ) // for testing stored procedure support
 }
 
 def pomName() {

hibernate-core/src/main/java/org/hibernate/cfg/annotations/NamedProcedureCallDefinition.java

@@ -173,7 +173,8 @@ public class NamedProcedureCallDefinition {
 						: ParameterStrategy.POSITIONAL;
 				parameterDefinitions = new ParameterDefinition[ parameters.length ];
 				for ( int i = 0; i < parameters.length; i++ ) {
-					parameterDefinitions[i] = new ParameterDefinition( i, parameters[i] );
+					// i+1 for the position because the apis say the numbers are 1-based, not zero
+					parameterDefinitions[i] = new ParameterDefinition( i+1, parameters[i] );
 				}
 			}
 		}

hibernate-core/src/main/java/org/hibernate/result/internal/ResultImpl.java

@@ -31,9 +31,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.jboss.logging.Logger;
+
 import org.hibernate.JDBCException;
 import org.hibernate.engine.spi.QueryParameters;
 import org.hibernate.engine.spi.SessionImplementor;
+import org.hibernate.internal.CoreLogging;
 import org.hibernate.loader.custom.CustomLoader;
 import org.hibernate.loader.custom.CustomQuery;
 import org.hibernate.loader.custom.sql.SQLQueryReturnProcessor;
@@ -49,6 +52,8 @@ import org.hibernate.result.spi.ResultContext;
  * @author Steve Ebersole
  */
 public class ResultImpl implements Result {
+	private static final Logger log = CoreLogging.logger( ResultImpl.class );
+
 	private final ResultContext context;
 	private final PreparedStatement jdbcStatement;
 	private final CustomLoaderExtension loader;
@@ -188,6 +193,15 @@ public class ResultImpl implements Result {
 		}
 
 		protected Return buildReturn() {
+			if ( log.isDebugEnabled() ) {
+				log.debugf(
+						"Building Return [isResultSet=%s, updateCount=%s, extendedReturn=%s",
+						isResultSet(),
+						getUpdateCount(),
+						hasExtendedReturns( currentReturnState )
+				);
+			}
+
 			if ( isResultSet() ) {
 				return new ResultSetReturnImpl( extractCurrentResults() );
 			}

hibernate-entitymanager/hibernate-entitymanager.gradle

@@ -22,6 +22,9 @@ dependencies {
     testRuntime( libraries.validator )
     testRuntime( "org.jboss.spec.javax.ejb:jboss-ejb-api_3.2_spec:1.0.0.Alpha2" )
 //    testRuntime( "org.glassfish.web:el-impl:2.1.2-b04" )
+
+    // for testing stored procedure support
+    testRuntime( libraries.derby )
 }
 
 def pomName() {

hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java

@@ -40,7 +40,9 @@ import java.util.List;
 import org.hibernate.CacheMode;
 import org.hibernate.FlushMode;
 import org.hibernate.LockMode;
+import org.hibernate.procedure.ParameterRegistration;
 import org.hibernate.procedure.ProcedureCall;
+import org.hibernate.procedure.ProcedureCallMemento;
 import org.hibernate.procedure.ProcedureResult;
 import org.hibernate.result.ResultSetReturn;
 import org.hibernate.result.Return;
@@ -60,6 +62,21 @@ public class StoredProcedureQueryImpl extends BaseQueryImpl implements StoredPro
 		this.procedureCall = procedureCall;
 	}
 
+	/**
+	 * This form is used to build a StoredProcedureQueryImpl from a memento (usually from a NamedStoredProcedureQuery).
+	 *
+	 * @param memento The memento
+	 * @param entityManager The EntityManager
+	 */
+	@SuppressWarnings("unchecked")
+	public StoredProcedureQueryImpl(ProcedureCallMemento memento, HibernateEntityManagerImplementor entityManager) {
+		super( entityManager );
+		this.procedureCall = memento.makeProcedureCall( entityManager.getSession() );
+		for ( org.hibernate.procedure.ParameterRegistration nativeParamReg : procedureCall.getRegisteredParameters() ) {
+			registerParameter( new ParameterRegistrationImpl( nativeParamReg ) );
+		}
+	}
+
 	@Override
 	protected boolean applyTimeoutHint(int timeout) {
 		procedureCall.setTimeout( timeout );

hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/AbstractEntityManagerImpl.java

@@ -838,8 +838,7 @@ public abstract class AbstractEntityManagerImpl implements HibernateEntityManage
 		if ( memento == null ) {
 			throw new IllegalArgumentException( "No @NamedStoredProcedureQuery was found with that name : " + name );
 		}
-		final ProcedureCall procedureCall = memento.makeProcedureCall( internalGetSession() );
+		final StoredProcedureQueryImpl jpaImpl = new StoredProcedureQueryImpl( memento, this );
-		final StoredProcedureQueryImpl jpaImpl = new StoredProcedureQueryImpl( procedureCall, this );
 		// apply hints
 		if ( memento.getHintsMap() != null ) {
 			for ( Map.Entry<String,Object> hintEntry : memento.getHintsMap().entrySet() ) {

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/procedure/JpaTckUsageTest.java

@@ -24,20 +24,38 @@
 package org.hibernate.jpa.test.procedure;
 
 import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
 import javax.persistence.StoredProcedureQuery;
 import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
-import org.hibernate.dialect.H2Dialect;
+import org.h2.tools.SimpleResultSet;
+
+import org.hibernate.dialect.DerbyTenSevenDialect;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.jpa.AvailableSettings;
+import org.hibernate.jpa.HibernateEntityManagerFactory;
+import org.hibernate.jpa.boot.spi.Bootstrap;
+import org.hibernate.jpa.boot.spi.PersistenceUnitDescriptor;
+import org.hibernate.jpa.internal.EntityManagerFactoryImpl;
 import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
 
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import org.hibernate.testing.FailureExpected;
-import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
 
 import static org.hibernate.testing.junit4.ExtraAssertions.assertTyping;
 import static org.junit.Assert.assertEquals;
@@ -48,108 +66,203 @@ import static org.junit.Assert.assertTrue;
  *
  * @author Steve Ebersole
  */
-@RequiresDialect( H2Dialect.class )
+public class JpaTckUsageTest extends BaseUnitTestCase {
-public class JpaTckUsageTest extends BaseEntityManagerFunctionalTestCase {
 
 	@Test
 	public void testMultipleGetUpdateCountCalls() {
-		EntityManager em = getOrCreateEntityManager();
+		EntityManager em = entityManagerFactory.createEntityManager();
 		em.getTransaction().begin();
 
-		StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser" );
+		try {
-		// this is what the TCK attempts to do, don't shoot the messenger...
+			StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser" );
-		query.getUpdateCount();
+			// this is what the TCK attempts to do, don't shoot the messenger...
-		// yep, twice
+			query.getUpdateCount();
-		query.getUpdateCount();
+			// yep, twice
-
+			query.getUpdateCount();
-		em.getTransaction().commit();
+		}
-		em.close();
+		finally {
+			em.getTransaction().commit();
+			em.close();
+		}
 	}
 
 	@Test
 	public void testBasicScalarResults() {
-		EntityManager em = getOrCreateEntityManager();
+		EntityManager em = entityManagerFactory.createEntityManager();
 		em.getTransaction().begin();
 
-		StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser" );
+		try {
-		boolean isResult = query.execute();
+			StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser" );
-		assertTrue( isResult );
+			boolean isResult = query.execute();
-		int updateCount = query.getUpdateCount();
+			assertTrue( isResult );
+			int updateCount = query.getUpdateCount();
 
-		boolean results = false;
+			boolean results = false;
-		do {
+			do {
-			List list = query.getResultList();
+				List list = query.getResultList();
-			assertEquals( 1, list.size() );
+				assertEquals( 1, list.size() );
 
-			results = query.hasMoreResults();
+				results = query.hasMoreResults();
-			// and it only sets the updateCount once lol
+				// and it only sets the updateCount once lol
-		} while ( results || updateCount != -1);
+			} while ( results || updateCount != -1);
-
+		}
-		em.getTransaction().commit();
+		finally {
-		em.close();
+			em.getTransaction().commit();
+			em.close();
+		}
 	}
 
 	@Test
 	public void testResultClassHandling() {
-		EntityManager em = getOrCreateEntityManager();
+		EntityManager em = entityManagerFactory.createEntityManager();
 		em.getTransaction().begin();
 
-		StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser", User.class );
+		try {
-		boolean isResult = query.execute();
+			StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser", User.class );
-		assertTrue( isResult );
+			boolean isResult = query.execute();
-		int updateCount = query.getUpdateCount();
+			assertTrue( isResult );
+			int updateCount = query.getUpdateCount();
 
-		boolean results = false;
+			boolean results = false;
-		do {
+			do {
-			List list = query.getResultList();
+				List list = query.getResultList();
-			assertEquals( 1, list.size() );
+				assertEquals( 1, list.size() );
-			assertTyping( User.class, list.get( 0 ) );
+				assertTyping( User.class, list.get( 0 ) );
 
-			results = query.hasMoreResults();
+				results = query.hasMoreResults();
-			// and it only sets the updateCount once lol
+				// and it only sets the updateCount once lol
-		} while ( results || updateCount != -1);
+			} while ( results || updateCount != -1);
-
+		}
-		em.getTransaction().commit();
+		finally {
-		em.close();
+			em.getTransaction().commit();
+			em.close();
+		}
 	}
 
+	@Test
+	public void testSettingInParamDefinedOnNamedStoredProcedureQuery() {
+		EntityManager em = entityManagerFactory.createEntityManager();
+		em.getTransaction().begin();
+		try {
+			StoredProcedureQuery query = em.createNamedStoredProcedureQuery( "positional-param" );
+			query.setParameter( 1, 1 );
+		}
+		finally {
+			em.getTransaction().commit();
+			em.close();
+		}
+	}
 
-	@Override
+	@Test
-	protected Class<?>[] getAnnotatedClasses() {
+	@FailureExpected( jiraKey = "HHH-8395", message = "Out of the frying pan into the fire: https://issues.apache.org/jira/browse/DERBY-211" )
-		return new Class[] { User.class };
+	public void testExecuteUpdate() {
+		EntityManager em = entityManagerFactory.createEntityManager();
+		em.getTransaction().begin();
+
+		try {
+			StoredProcedureQuery query = em.createStoredProcedureQuery( "deleteAllUsers" );
+			int count = query.executeUpdate();
+			// this fails because the Derby EmbeddedDriver is returning zero here rather than the actual updateCount :(
+			// https://issues.apache.org/jira/browse/DERBY-211
+			assertEquals( 1, count );
+		}
+		finally {
+			em.getTransaction().commit();
+			em.close();
+		}
 	}
 
 	// todo : look at ways to allow "Auxiliary DB Objects" to the db via EMF bootstrapping.
 
-	public static final String CREATE_CMD = "CREATE ALIAS findOneUser AS $$\n" +
+//	public static final String findOneUser_CREATE_CMD = "CREATE ALIAS findOneUser AS $$\n" +
-			"import org.h2.tools.SimpleResultSet;\n" +
+//			"import org.h2.tools.SimpleResultSet;\n" +
-			"import java.sql.*;\n" +
+//			"import java.sql.*;\n" +
-			"@CODE\n" +
+//			"@CODE\n" +
-			"ResultSet findOneUser() {\n" +
+//			"ResultSet findOneUser() {\n" +
-			"    SimpleResultSet rs = new SimpleResultSet();\n" +
+//			"    SimpleResultSet rs = new SimpleResultSet();\n" +
-			"    rs.addColumn(\"ID\", Types.INTEGER, 10, 0);\n" +
+//			"    rs.addColumn(\"ID\", Types.INTEGER, 10, 0);\n" +
-			"    rs.addColumn(\"NAME\", Types.VARCHAR, 255, 0);\n" +
+//			"    rs.addColumn(\"NAME\", Types.VARCHAR, 255, 0);\n" +
-			"    rs.addRow(1, \"Steve\");\n" +
+//			"    rs.addRow(1, \"Steve\");\n" +
-			"    return rs;\n" +
+//			"    return rs;\n" +
-			"}\n" +
+//			"}\n" +
-			"$$";
+//			"$$";
-	public static final String DROP_CMD = "DROP ALIAS findOneUser IF EXISTS";
+//	public static final String findOneUser_DROP_CMD = "DROP ALIAS findOneUser IF EXISTS";
+//
+//	public static final String deleteAllUsers_CREATE_CMD = "CREATE ALIAS deleteAllUsers AS $$\n" +
+//			"@CODE\n" +
+//			"int deleteAllUsers() {\n" +
+//			"    return 156;" +
+//			"}\n" +
+//			"$$";
+//	public static final String deleteAllUsers_DROP_CMD = "DROP ALIAS deleteAllUsers IF EXISTS";
 
-	@Override
+	HibernateEntityManagerFactory entityManagerFactory;
-	protected void afterEntityManagerFactoryBuilt() {
+
-		execute( CREATE_CMD );
+	@Before
+	public void startUp() {
+		// create the EMF
+		entityManagerFactory = (EntityManagerFactoryImpl) Bootstrap.getEntityManagerFactoryBuilder(
+				buildPersistenceUnitDescriptor(),
+				buildSettingsMap()
+		).build();
+
+		// create the procedures
+		createTestUser( entityManagerFactory );
+		createProcedures( entityManagerFactory );
 	}
 
-	private void execute(String sql) {
+	private PersistenceUnitDescriptor buildPersistenceUnitDescriptor() {
-		System.out.println( "Executing SQL : " + sql );
+		return new BaseEntityManagerFunctionalTestCase.TestingPersistenceUnitDescriptorImpl( getClass().getSimpleName() );
-		final SessionFactoryImplementor sf = entityManagerFactory().unwrap( SessionFactoryImplementor.class );
+	}
+
+	@SuppressWarnings("unchecked")
+	private Map buildSettingsMap() {
+		Map settings = new HashMap();
+
+		settings.put( AvailableSettings.LOADED_CLASSES, Collections.singletonList( User.class ) );
+
+		settings.put( org.hibernate.cfg.AvailableSettings.DIALECT, DerbyTenSevenDialect.class );
+		settings.put( org.hibernate.cfg.AvailableSettings.DRIVER,  org.apache.derby.jdbc.EmbeddedDriver.class.getName() );
+		settings.put( org.hibernate.cfg.AvailableSettings.URL, "jdbc:derby:hibernate-orm-testing;create=true" );
+		settings.put( org.hibernate.cfg.AvailableSettings.USER, "" );
+
+		settings.put( org.hibernate.cfg.AvailableSettings.HBM2DDL_AUTO, "create-drop" );
+		settings.put( org.hibernate.cfg.AvailableSettings.USE_NEW_ID_GENERATOR_MAPPINGS, "true" );
+		settings.put( org.hibernate.cfg.AvailableSettings.DIALECT, DerbyTenSevenDialect.class.getName() );
+		return settings;
+	}
+
+	@After
+	public void tearDown() {
+		if ( entityManagerFactory == null ) {
+			return;
+		}
+
+		deleteTestUser( entityManagerFactory );
+		dropProcedures( entityManagerFactory );
+
+		entityManagerFactory.close();
+	}
+
+	private void createProcedures(HibernateEntityManagerFactory emf) {
+		final SessionFactoryImplementor sf = emf.unwrap( SessionFactoryImplementor.class );
 		final Connection conn;
 		try {
 			conn = sf.getConnectionProvider().getConnection();
+			conn.setAutoCommit( false );
 
 			try {
 				Statement statement = conn.createStatement();
-				statement.execute( sql );
+
+				// drop them, just to be sure
+				try {
+					dropProcedures( statement );
+				}
+				catch (SQLException ignore) {
+				}
+
+				createProcedureFindOneUser( statement );
+				createProcedureDeleteAllUsers( statement );
 				try {
 					statement.close();
 				}
@@ -157,6 +270,13 @@ public class JpaTckUsageTest extends BaseEntityManagerFunctionalTestCase {
 				}
 			}
 			finally {
+				try {
+					conn.commit();
+				}
+				catch (SQLException e) {
+					System.out.println( "Unable to commit transaction after creating creating procedures");
+				}
+
 				try {
 					sf.getConnectionProvider().closeConnection( conn );
 				}
@@ -165,13 +285,106 @@ public class JpaTckUsageTest extends BaseEntityManagerFunctionalTestCase {
 			}
 		}
 		catch (SQLException e) {
-			throw new RuntimeException( "Unable to execute SQL : " + sql, e );
+			throw new RuntimeException( "Unable to create stored procedures", e );
 		}
 	}
 
-	@Override
+	private void dropProcedures(Statement statement) throws SQLException {
-	public void releaseResources() {
+		statement.execute( "DROP PROCEDURE findOneUser" );
-		execute( DROP_CMD );
+		statement.execute( "DROP PROCEDURE deleteAllUsers" );
-		super.releaseResources();
+	}
+
+	private void createProcedureFindOneUser(Statement statement) throws SQLException {
+		statement.execute(
+				"CREATE PROCEDURE findOneUser() " +
+						"language java " +
+						"dynamic result sets 1 " +
+						"external name 'org.hibernate.jpa.test.procedure.JpaTckUsageTest.findOneUser' " +
+						"parameter style java"
+		);
+	}
+
+	private void createProcedureDeleteAllUsers(Statement statement) throws SQLException {
+		statement.execute(
+				"CREATE PROCEDURE deleteAllUsers() " +
+						"language java " +
+						"external name 'org.hibernate.jpa.test.procedure.JpaTckUsageTest.deleteAllUsers' " +
+						"parameter style java"
+		);
+	}
+
+	public static void findOneUser(ResultSet[] results) throws SQLException {
+		Connection conn = DriverManager.getConnection( "jdbc:default:connection" );
+		PreparedStatement ps = conn.prepareStatement( "select id, name from t_user where name=?" );
+		ps.setString( 1, "steve" );
+		results[0] = ps.executeQuery();
+		conn.close();
+	}
+
+	public static void deleteAllUsers() throws SQLException {
+		// afaict the only way to return update counts here is to actually perform some DML
+		Connection conn = DriverManager.getConnection( "jdbc:default:connection" );
+		System.out.println( "Preparing delete all" );
+		PreparedStatement ps = conn.prepareStatement( "delete from t_user" );
+		System.out.println( "Executing delete all" );
+		int count = ps.executeUpdate();
+		System.out.println( "Count : " + count );
+		System.out.println( "Closing resources" );
+		ps.close();
+		conn.close();
+	}
+
+	private void createTestUser(HibernateEntityManagerFactory entityManagerFactory) {
+		EntityManager em = entityManagerFactory.createEntityManager();
+		em.getTransaction().begin();
+
+		em.persist( new User( 1, "steve" ) );
+		em.getTransaction().commit();
+		em.close();
+	}
+
+	private void deleteTestUser(HibernateEntityManagerFactory entityManagerFactory) {
+		EntityManager em = entityManagerFactory.createEntityManager();
+		em.getTransaction().begin();
+		em.createQuery( "delete from User" ).executeUpdate();
+		em.getTransaction().commit();
+		em.close();
+	}
+
+	private void dropProcedures(HibernateEntityManagerFactory emf) {
+
+		final SessionFactoryImplementor sf = emf.unwrap( SessionFactoryImplementor.class );
+		final Connection conn;
+		try {
+			conn = sf.getConnectionProvider().getConnection();
+			conn.setAutoCommit( false );
+
+			try {
+				Statement statement = conn.createStatement();
+				dropProcedures( statement );
+				try {
+					statement.close();
+				}
+				catch (SQLException ignore) {
+				}
+			}
+			finally {
+				try {
+					conn.commit();
+				}
+				catch (SQLException e) {
+					System.out.println( "Unable to commit transaction after creating dropping procedures");
+				}
+
+				try {
+					sf.getConnectionProvider().closeConnection( conn );
+				}
+				catch (SQLException ignore) {
+				}
+			}
+		}
+		catch (SQLException e) {
+			throw new RuntimeException( "Unable to drop stored procedures", e );
+		}
 	}
 }

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/procedure/User.java

@@ -9,6 +9,7 @@ import javax.persistence.NamedStoredProcedureQuery;
 import javax.persistence.ParameterMode;
 import javax.persistence.SqlResultSetMapping;
 import javax.persistence.StoredProcedureParameter;
+import javax.persistence.Table;
 
 /**
  * @author Strong Liu <stliu@hibernate.org>
@@ -43,6 +44,13 @@ import javax.persistence.StoredProcedureParameter;
 						},
 						resultSetMappings = { "srms" }
 
+				),
+				@NamedStoredProcedureQuery(
+						name = "positional-param",
+						procedureName = "positionalParameterTesting",
+						parameters = {
+								@StoredProcedureParameter( mode = ParameterMode.IN, type = Integer.class )
+						}
 				)
 		}
 )
@@ -54,11 +62,20 @@ import javax.persistence.StoredProcedureParameter;
 				})
 		}
 )
+@Table( name = "T_USER" )
 public class User {
 	@Id
 	private int id;
 	private String name;
 
+	public User() {
+	}
+
+	public User(int id, String name) {
+		this.id = id;
+		this.name = name;
+	}
+
 	public int getId() {
 		return id;
 	}

hibernate-entitymanager/src/test/resources/hibernate.properties

@@ -21,9 +21,15 @@
 # 51 Franklin Street, Fifth Floor
 # Boston, MA  02110-1301  USA
 #
-hibernate.dialect org.hibernate.dialect.H2Dialect
+
-hibernate.connection.driver_class org.h2.Driver
+#hibernate.dialect org.hibernate.dialect.H2Dialect
-hibernate.connection.url jdbc:h2:mem:db1;DB_CLOSE_DELAY=-1;MVCC=TRUE;LOCK_TIMEOUT=10000
+#hibernate.connection.driver_class org.h2.Driver
+#hibernate.connection.url jdbc:h2:mem:db1;DB_CLOSE_DELAY=-1;MVCC=TRUE;LOCK_TIMEOUT=10000
+#hibernate.connection.username sa
+
+hibernate.dialect org.hibernate.dialect.DerbyTenSevenDialect
+hibernate.connection.driver_class org.apache.derby.jdbc.EmbeddedDriver
+hibernate.connection.url jdbc:derby:memory:hibernate-orm-testing;create=true
 hibernate.connection.username sa
 
 hibernate.connection.pool_size 5

libraries.gradle

@@ -92,6 +92,7 @@ ext {
             shrinkwrap:      'org.jboss.shrinkwrap:shrinkwrap-impl-base:1.0.0-beta-6',
             validator:       'org.hibernate:hibernate-validator:5.0.1.Final',
             h2:              "com.h2database:h2:${h2Version}",
+            derby:           "org.apache.derby:derby:10.9.1.0",
             jboss_jta:       "org.jboss.jbossts:jbossjta:4.16.4.Final",
             xapool:          "com.experlog:xapool:1.5.0",
             mockito:         'org.mockito:mockito-core:1.9.0',