HHH-8411 - StoredProcedureQuery : getResultList and hasMoreResults() mot implemented properly

hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureResultImpl.java

@@ -72,36 +72,6 @@ protected CurrentReturnState buildCurrentReturnDescriptor(boolean isResultSet, i
 		return new ProcedureCurrentReturnState( isResultSet, updateCount, refCursorParamIndex );
 	}
 
-	protected boolean hasMoreReturns(CurrentReturnState descriptor) {
-		return super.hasMoreReturns( descriptor )
-				|| ( (ProcedureCurrentReturnState) descriptor ).refCursorParamIndex < refCursorParameters.length;
-	}
-
-	@Override
-	protected boolean hasExtendedReturns(CurrentReturnState currentReturnState) {
-		return ProcedureCurrentReturnState.class.isInstance( currentReturnState )
-				&& ( (ProcedureCurrentReturnState) currentReturnState ).refCursorParamIndex < refCursorParameters.length;
-	}
-
-	@Override
-	protected Return buildExtendedReturn(CurrentReturnState returnDescriptor) {
-		this.refCursorParamIndex++;
-		final int refCursorParamIndex = ( (ProcedureCurrentReturnState) returnDescriptor ).refCursorParamIndex;
-		final ParameterRegistrationImplementor refCursorParam = refCursorParameters[refCursorParamIndex];
-		ResultSet resultSet;
-		if ( refCursorParam.getName() != null ) {
-			resultSet = procedureCall.getSession().getFactory().getServiceRegistry()
-					.getService( RefCursorSupport.class )
-					.getResultSet( callableStatement, refCursorParam.getName() );
-		}
-		else {
-			resultSet = procedureCall.getSession().getFactory().getServiceRegistry()
-					.getService( RefCursorSupport.class )
-					.getResultSet( callableStatement, refCursorParam.getPosition() );
-		}
-		return new ResultSetReturnImpl( extractResults( resultSet ) );
-	}
-
 	protected class ProcedureCurrentReturnState extends CurrentReturnState {
 		private final int refCursorParamIndex;
 
@@ -109,6 +79,35 @@ private ProcedureCurrentReturnState(boolean isResultSet, int updateCount, int re
 			super( isResultSet, updateCount );
 			this.refCursorParamIndex = refCursorParamIndex;
 		}
+
+		@Override
+		public boolean indicatesMoreReturns() {
+			return super.indicatesMoreReturns()
+					|| ProcedureResultImpl.this.refCursorParamIndex < ProcedureResultImpl.this.refCursorParameters.length;
+		}
+
+		@Override
+		protected boolean hasExtendedReturns() {
+			return refCursorParamIndex < refCursorParameters.length;
+		}
+
+		@Override
+		protected Return buildExtendedReturn() {
+			ProcedureResultImpl.this.refCursorParamIndex++;
+			final ParameterRegistrationImplementor refCursorParam = ProcedureResultImpl.this.refCursorParameters[refCursorParamIndex];
+			ResultSet resultSet;
+			if ( refCursorParam.getName() != null ) {
+				resultSet = ProcedureResultImpl.this.procedureCall.getSession().getFactory().getServiceRegistry()
+						.getService( RefCursorSupport.class )
+						.getResultSet( ProcedureResultImpl.this.callableStatement, refCursorParam.getName() );
+			}
+			else {
+				resultSet = ProcedureResultImpl.this.procedureCall.getSession().getFactory().getServiceRegistry()
+						.getService( RefCursorSupport.class )
+						.getResultSet( ProcedureResultImpl.this.callableStatement, refCursorParam.getPosition() );
+			}
+			return new ResultSetReturnImpl( extractResults( resultSet ) );
+		}
 	}
 
 }

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

@@ -113,11 +113,7 @@ public boolean hasMoreReturns() {
 			throw convert( e, "Error calling CallableStatement.getMoreResults" );
 		}
 
-		return hasMoreReturns( currentReturnState );
+		return currentReturnState != null && currentReturnState.indicatesMoreReturns();
-	}
-
-	protected boolean hasMoreReturns(CurrentReturnState descriptor) {
-		return descriptor != null && ( descriptor.isResultSet() || descriptor.getUpdateCount() >= 0 );
 	}
 
 	@Override
@@ -129,10 +125,6 @@ public Return getNextReturn() {
 		return getCurrentReturn();
 	}
 
-	protected boolean hasExtendedReturns(CurrentReturnState currentReturnState) {
-		return false;
-	}
-
 	private List extractCurrentResults() {
 		try {
 			return extractResults( jdbcStatement.getResultSet() );
@@ -151,10 +143,6 @@ protected List extractResults(ResultSet resultSet) {
 		}
 	}
 
-	protected Return buildExtendedReturn(CurrentReturnState copyReturnDescriptor) {
-		throw new NoMoreReturnsException();
-	}
-
 	protected JDBCException convert(SQLException e, String message) {
 		return context.getSession().getFactory().getSQLExceptionHelper().convert(
 				e,
@@ -177,6 +165,10 @@ protected CurrentReturnState(boolean isResultSet, int updateCount) {
 			this.updateCount = updateCount;
 		}
 
+		public boolean indicatesMoreReturns() {
+			return isResultSet() || getUpdateCount() >= 0;
+		}
+
 		public boolean isResultSet() {
 			return isResultSet;
 		}
@@ -198,9 +190,18 @@ protected Return buildReturn() {
 						"Building Return [isResultSet=%s, updateCount=%s, extendedReturn=%s",
 						isResultSet(),
 						getUpdateCount(),
-						hasExtendedReturns( currentReturnState )
+						hasExtendedReturns()
 				);
 			}
+			// todo : temporary for tck testing...
+			System.out.println(
+					String.format(
+							"Building Return [isResultSet=%s, updateCount=%s, extendedReturn=%s",
+							isResultSet(),
+							getUpdateCount(),
+							hasExtendedReturns()
+					)
+			);
 
 			if ( isResultSet() ) {
 				return new ResultSetReturnImpl( extractCurrentResults() );
@@ -208,12 +209,22 @@ protected Return buildReturn() {
 			else if ( getUpdateCount() >= 0 ) {
 				return new UpdateCountReturnImpl( updateCount );
 			}
-			else if ( hasExtendedReturns( currentReturnState ) ) {
+			else if ( hasExtendedReturns() ) {
-				return buildExtendedReturn( currentReturnState );
+				return buildExtendedReturn();
 			}
 
 			throw new NoMoreReturnsException();
 		}
+
+		// hooks for stored procedure (out param) processing ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+		protected boolean hasExtendedReturns() {
+			return false;
+		}
+
+		protected Return buildExtendedReturn() {
+			throw new IllegalStateException( "State does not define extended returns" );
+		}
 	}
 
 	protected static class ResultSetReturnImpl implements ResultSetReturn {

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

@@ -33,6 +33,7 @@
 import javax.persistence.Query;
 import javax.persistence.StoredProcedureQuery;
 import javax.persistence.TemporalType;
+import javax.persistence.TransactionRequiredException;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.List;
@@ -44,6 +45,7 @@
 import org.hibernate.procedure.ProcedureCall;
 import org.hibernate.procedure.ProcedureCallMemento;
 import org.hibernate.procedure.ProcedureResult;
+import org.hibernate.result.NoMoreReturnsException;
 import org.hibernate.result.ResultSetReturn;
 import org.hibernate.result.Return;
 import org.hibernate.result.UpdateCountReturn;
@@ -217,46 +219,63 @@ public Object getOutputParameterValue(String parameterName) {
 
 	@Override
 	public boolean execute() {
-		final Return rtn = outputs().getCurrentReturn();
+		try {
-		return rtn != null && ResultSetReturn.class.isInstance( rtn );
+			final Return rtn = outputs().getCurrentReturn();
+			return rtn != null && ResultSetReturn.class.isInstance( rtn );
+		}
+		catch (NoMoreReturnsException e) {
+			return false;
+		}
 	}
 
 	@Override
 	public int executeUpdate() {
+		if ( ! entityManager().isTransactionInProgress() ) {
+			throw new TransactionRequiredException( "javax.persistence.Query.executeUpdate requires active transaction" );
+		}
 		return getUpdateCount();
 	}
 
 	@Override
 	public boolean hasMoreResults() {
-		return outputs().hasMoreReturns();
+		return outputs().hasMoreReturns() && ResultSetReturn.class.isInstance( outputs().getCurrentReturn() );
 	}
 
 	@Override
 	public int getUpdateCount() {
-		final Return rtn = outputs().getCurrentReturn();
+		try {
-		if ( rtn == null ) {
+			final Return rtn = outputs().getCurrentReturn();
-			return -1;
+			if ( rtn == null ) {
+				return -1;
+			}
+			else if ( UpdateCountReturn.class.isInstance( rtn ) ) {
+				return ( (UpdateCountReturn) rtn ).getUpdateCount();
+			}
+			else {
+				return -1;
+			}
 		}
-		else if ( UpdateCountReturn.class.isInstance( rtn ) ) {
+		catch (NoMoreReturnsException e) {
-			return ( (UpdateCountReturn) rtn ).getUpdateCount();
-		}
-		else {
 			return -1;
 		}
 	}
 
 	@Override
 	public List getResultList() {
-		final Return rtn = outputs().getCurrentReturn();
+		try {
-		if ( ! ResultSetReturn.class.isInstance( rtn ) ) {
+			final Return rtn = outputs().getCurrentReturn();
-			throw new IllegalStateException( "Current CallableStatement was not a ResultSet, but getResultList was called" );
+			if ( ! ResultSetReturn.class.isInstance( rtn ) ) {
-		}
+				throw new IllegalStateException( "Current CallableStatement return was not a ResultSet, but getResultList was called" );
+			}
 
-		if ( outputs().hasMoreReturns() ) {
+			return ( (ResultSetReturn) rtn ).getResultList();
-			outputs().getNextReturn();
+		}
+		catch (NoMoreReturnsException e) {
+			// todo : the spec is completely silent on these type of edge-case scenarios.
+			// Essentially here we'd have a case where there are no more results (ResultSets nor updateCount) but
+			// getResultList was called.
+			return null;
 		}
-
-		return ( (ResultSetReturn) rtn ).getResultList();
 	}
 
 	@Override
@@ -305,17 +324,14 @@ else if ( BaseQueryImpl.class.isAssignableFrom( cls ) ) {
 		);
 	}
 
-
-	// ugh ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
 	@Override
 	public Query setLockMode(LockModeType lockMode) {
-		return null;
+		throw new IllegalStateException( "javax.persistence.Query.setLockMode not valid on javax.persistence.StoredProcedureQuery" );
 	}
 
 	@Override
 	public LockModeType getLockMode() {
-		return null;
+		throw new IllegalStateException( "javax.persistence.Query.getLockMode not valid on javax.persistence.StoredProcedureQuery" );
 	}
 
 

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

@@ -755,16 +755,20 @@ public <T> T getParameterValue(Parameter<T> param) {
 		checkOpen( false );
 
 		final ParameterRegistration<T> registration = findParameterRegistration( param );
-		if ( registration != null ) {
+		if ( registration == null ) {
-			if ( ! registration.isBindable() ) {
+			throw new IllegalArgumentException( "Passed parameter [" + param + "] is not a (registered) parameter of this query" );
-				throw new IllegalArgumentException( "Passed parameter [" + param + "] is not bindable" );
-			}
-			final ParameterBind<T> bind = registration.getBind();
-			if ( bind != null ) {
-				return bind.getValue();
-			}
 		}
-		throw new IllegalStateException( "Parameter [" + param + "] has not yet been bound" );
+
+		if ( ! registration.isBindable() ) {
+			throw new IllegalStateException( "Passed parameter [" + param + "] is not bindable" );
+		}
+
+		final ParameterBind<T> bind = registration.getBind();
+		if ( bind == null ) {
+			throw new IllegalStateException( "Parameter [" + param + "] has not yet been bound" );
+		}
+
+		return bind.getValue();
 	}
 
 	@Override