From 96931d8094ca01fe8281ad4f889e6f2cc6a6885f Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 26 Jan 2022 15:13:03 +0100 Subject: [PATCH] Fix tests and implement handling callable function hint for stored procedures --- .../procedure/internal/ProcedureCallImpl.java | 37 +++++++---- .../internal/ProcedureOutputsImpl.java | 26 ++++++-- .../result/internal/OutputsImpl.java | 4 ++ .../internal/JdbcCallFunctionReturnImpl.java | 2 +- .../orm/test/filter/FilterParameterTests.java | 63 +++++++++++++++++-- .../procedure/OracleStoredProcedureTest.java | 40 ++++++------ 6 files changed, 130 insertions(+), 42 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java index 6cd5508364..8145a3abd3 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java @@ -300,22 +300,26 @@ public class ProcedureCallImpl if ( memento.getHints() != null ) { final Object callableFunction = memento.getHints().get( HINT_CALLABLE_FUNCTION ); if ( callableFunction != null && Boolean.parseBoolean( callableFunction.toString() ) ) { - final List> resultTypes = new ArrayList<>(); - resultSetMapping.visitResultBuilders( - (index, resultBuilder) -> resultTypes.add( resultBuilder.getJavaType() ) - ); - final TypeConfiguration typeConfiguration = getSessionFactory().getTypeConfiguration(); - final BasicType type; - if ( resultTypes.size() != 1 || ( type = typeConfiguration.getBasicTypeForJavaType( resultTypes.get( 0 ) ) ) == null ) { - markAsFunctionCall( Types.REF_CURSOR ); - } - else { - markAsFunctionCall( type.getJdbcType().getJdbcTypeCode() ); - } + applyCallableFunctionHint(); } } } + private void applyCallableFunctionHint() { + final List> resultTypes = new ArrayList<>(); + resultSetMapping.visitResultBuilders( + (index, resultBuilder) -> resultTypes.add( resultBuilder.getJavaType() ) + ); + final TypeConfiguration typeConfiguration = getSessionFactory().getTypeConfiguration(); + final BasicType type; + if ( resultTypes.size() != 1 || ( type = typeConfiguration.getBasicTypeForJavaType( resultTypes.get( 0 ) ) ) == null ) { + markAsFunctionCall( Types.REF_CURSOR ); + } + else { + markAsFunctionCall( type.getJdbcType().getJdbcTypeCode() ); + } + } + @Override public String getProcedureName() { return procedureName; @@ -1062,7 +1066,14 @@ public class ProcedureCallImpl @Override public ProcedureCallImplementor setHint(String hintName, Object value) { - super.setHint( hintName, value ); + if ( HINT_CALLABLE_FUNCTION.equals( hintName ) ) { + if ( value != null && Boolean.parseBoolean( value.toString() ) ) { + applyCallableFunctionHint(); + } + } + else { + super.setHint( hintName, value ); + } return this; } diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java index 0fd96c8471..04d77bb195 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java @@ -8,6 +8,7 @@ package org.hibernate.procedure.internal; import java.sql.CallableStatement; import java.sql.ResultSet; +import java.util.List; import java.util.Map; import org.hibernate.procedure.ParameterMisuseException; @@ -27,7 +28,7 @@ import jakarta.persistence.ParameterMode; * @author Steve Ebersole */ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutputs { - private final ProcedureCallImpl procedureCall; + private final ProcedureCallImpl procedureCall; private final CallableStatement callableStatement; private final Map, JdbcCallParameterRegistration> parameterRegistrations; @@ -35,7 +36,7 @@ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutput private int refCursorParamIndex; ProcedureOutputsImpl( - ProcedureCallImpl procedureCall, + ProcedureCallImpl procedureCall, Map, JdbcCallParameterRegistration> parameterRegistrations, JdbcCallRefCursorExtractor[] refCursorParameters, CallableStatement callableStatement) { @@ -44,6 +45,11 @@ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutput this.callableStatement = callableStatement; this.parameterRegistrations = parameterRegistrations; this.refCursorParameters = refCursorParameters; + if ( procedureCall.getFunctionReturn() != null && procedureCall.getFunctionReturn().getMode() != ParameterMode.REF_CURSOR ) { + // Set to -1, so we can handle the function return as out parameter separately + this.refCursorParamIndex = -1; + } + executeStatement(); } @Override @@ -116,9 +122,19 @@ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutput @Override protected Output buildExtendedReturn() { - final JdbcCallRefCursorExtractor refCursorParam = refCursorParameters[ProcedureOutputsImpl.this.refCursorParamIndex++]; - final ResultSet resultSet = refCursorParam.extractResultSet( callableStatement, procedureCall.getSession() ); - return buildResultSetOutput( () -> extractResults( resultSet ) ); + if ( ProcedureOutputsImpl.this.refCursorParamIndex == -1 ) { + // Handle the function return + ProcedureOutputsImpl.this.refCursorParamIndex = 0; + return buildResultSetOutput( () -> List.of( getOutputParameterValue( procedureCall.getFunctionReturn() ) ) ); + } + else { + final JdbcCallRefCursorExtractor refCursorParam = refCursorParameters[ProcedureOutputsImpl.this.refCursorParamIndex++]; + final ResultSet resultSet = refCursorParam.extractResultSet( + callableStatement, + procedureCall.getSession() + ); + return buildResultSetOutput( () -> extractResults( resultSet ) ); + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java b/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java index cf26252f35..1055da66ee 100644 --- a/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java @@ -65,6 +65,9 @@ public class OutputsImpl implements Outputs { this.context = context; this.jdbcStatement = jdbcStatement; + } + + protected void executeStatement() { try { final boolean isResultSet = jdbcStatement.execute(); currentReturnState = buildCurrentReturnState( isResultSet ); @@ -343,6 +346,7 @@ public class OutputsImpl implements Outputs { else if ( hasExtendedReturns() ) { return buildExtendedReturn(); } +// else if ( procedureCall) throw new NoMoreOutputsException(); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcCallFunctionReturnImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcCallFunctionReturnImpl.java index 37a80cf1c1..1e0cd9204f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcCallFunctionReturnImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcCallFunctionReturnImpl.java @@ -22,7 +22,7 @@ public class JdbcCallFunctionReturnImpl extends JdbcCallParameterRegistrationImp super( null, 1, - ParameterMode.REF_CURSOR, + refCursorExtractor == null ? ParameterMode.OUT : ParameterMode.REF_CURSOR, ormType, null, parameterExtractor, diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/filter/FilterParameterTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/filter/FilterParameterTests.java index ab1750d1a5..ad8ddc4c9e 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/filter/FilterParameterTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/filter/FilterParameterTests.java @@ -18,12 +18,22 @@ import org.hibernate.annotations.Filter; import org.hibernate.annotations.FilterDef; import org.hibernate.annotations.JdbcTypeCode; import org.hibernate.annotations.ParamDef; +import org.hibernate.dialect.DB2Dialect; +import org.hibernate.dialect.DerbyDialect; +import org.hibernate.dialect.H2Dialect; +import org.hibernate.dialect.HSQLDialect; +import org.hibernate.dialect.MariaDBDialect; +import org.hibernate.dialect.MySQLDialect; +import org.hibernate.dialect.OracleDialect; +import org.hibernate.dialect.SQLServerDialect; +import org.hibernate.dialect.SybaseDialect; import org.hibernate.type.NumericBooleanConverter; import org.hibernate.type.YesNoConverter; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.SkipForDialect; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -55,12 +65,32 @@ public class FilterParameterTests { final EntityOne loaded = session.byId( EntityOne.class ).load( 1 ); assertThat( loaded ).isNull(); } ); + } + + @Test + @SkipForDialect(dialectClass = H2Dialect.class, reason = "H2 silently converts a boolean to string types") + @SkipForDialect(dialectClass = HSQLDialect.class, reason = "HSQL silently converts a boolean to string types") + @SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby silently converts a boolean to string types") + @SkipForDialect(dialectClass = DB2Dialect.class, reason = "DB2 silently converts a boolean to string types") + @SkipForDialect(dialectClass = MySQLDialect.class, reason = "MySQL silently converts a boolean to string types") + @SkipForDialect(dialectClass = MariaDBDialect.class, reason = "MariaDB silently converts a boolean to string types") + @SkipForDialect(dialectClass = SybaseDialect.class, matchSubTypes = true, reason = "Sybase silently converts a boolean to string types") + public void testYesNoMismatch(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final EntityOne loaded = session.byId( EntityOne.class ).load( 1 ); + assertThat( loaded ).isNotNull(); + } ); scope.inTransaction( (session) -> { session.enableFilter( "filterYesNoBoolean" ).setParameter( "yesNo", Boolean.FALSE ); - final EntityOne loaded = session.byId( EntityOne.class ).load( 1 ); - assertThat( loaded ).isNull(); + try { + session.byId( EntityOne.class ).load( 1 ); + fail( "Expecting an exception" ); + } + catch (Exception expected) { + System.out.println(expected.getMessage()); + } } ); } @@ -77,16 +107,40 @@ public class FilterParameterTests { final EntityTwo loaded = session.byId( EntityTwo.class ).load( 1 ); assertThat( loaded ).isNull(); } ); + } + + @Test + @SkipForDialect(dialectClass = H2Dialect.class, reason = "H2 silently converts a boolean to integral types") + @SkipForDialect(dialectClass = OracleDialect.class, reason = "Oracle silently converts a boolean to integral types") + @SkipForDialect(dialectClass = HSQLDialect.class, reason = "HSQL silently converts a boolean to integral types") + @SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby silently converts a boolean to integral types") + @SkipForDialect(dialectClass = DB2Dialect.class, reason = "DB2 silently converts a boolean to integral types") + @SkipForDialect(dialectClass = MySQLDialect.class, reason = "MySQL silently converts a boolean to integral types") + @SkipForDialect(dialectClass = MariaDBDialect.class, reason = "MariaDB silently converts a boolean to integral types") + @SkipForDialect(dialectClass = SQLServerDialect.class, reason = "SQL Server silently converts a boolean to integral types") + @SkipForDialect(dialectClass = SybaseDialect.class, matchSubTypes = true, reason = "Sybase silently converts a boolean to integral types") + public void testNumericMismatch(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final EntityTwo loaded = session.byId( EntityTwo.class ).load( 1 ); + assertThat( loaded ).isNotNull(); + } ); scope.inTransaction( (session) -> { session.enableFilter( "filterNumberBoolean" ).setParameter( "zeroOne", Boolean.FALSE ); - final EntityTwo loaded = session.byId( EntityTwo.class ).load( 1 ); - assertThat( loaded ).isNull(); + try { + session.byId( EntityTwo.class ).load( 1 ); + fail( "Expecting an exception" ); + } + catch (Exception expected) { + System.out.println(expected.getMessage()); + } } ); } @Test + @SkipForDialect(dialectClass = MySQLDialect.class, reason = "MySQL silently converts strings to integral types") + @SkipForDialect(dialectClass = MariaDBDialect.class, reason = "MariaDB silently converts strings to integral types") public void testMismatch(SessionFactoryScope scope) { scope.inTransaction( (session) -> { final EntityThree loaded = session.byId( EntityThree.class ).load( 1 ); @@ -101,6 +155,7 @@ public class FilterParameterTests { fail( "Expecting an exception" ); } catch (Exception expected) { + System.out.println(expected.getMessage()); } } ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/procedure/OracleStoredProcedureTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/procedure/OracleStoredProcedureTest.java index fa2dcdfd0b..ecca881bb2 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/procedure/OracleStoredProcedureTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/procedure/OracleStoredProcedureTest.java @@ -69,6 +69,8 @@ import static org.junit.jupiter.api.Assertions.fail; @RequiresDialect(value = OracleDialect.class) public class OracleStoredProcedureTest { + private Person person1; + @Test public void testUnRegisteredParameter(EntityManagerFactoryScope scope) { scope.inTransaction( (em) -> { @@ -97,7 +99,7 @@ public class OracleStoredProcedureTest { query.registerStoredProcedureParameter( 1, Long.class, ParameterMode.IN ); query.registerStoredProcedureParameter( 2, Long.class, ParameterMode.OUT ); - query.setParameter( 1, 1L ); + query.setParameter( 1, person1.getId() ); query.execute(); Long phoneCount = (Long) query.getOutputParameterValue( 2 ); @@ -113,7 +115,7 @@ public class OracleStoredProcedureTest { StoredProcedureQuery query = entityManager.createStoredProcedureQuery( "sp_person_phones" ); query.registerStoredProcedureParameter( 1, Long.class, ParameterMode.IN ); query.registerStoredProcedureParameter( 2, Class.class, ParameterMode.REF_CURSOR ); - query.setParameter( 1, 1L ); + query.setParameter( 1, person1.getId() ); query.execute(); List postComments = query.getResultList(); @@ -134,7 +136,7 @@ public class OracleStoredProcedureTest { Long.class, ParameterMode.IN ); - call.setParameter( inParam, 1L ); + call.setParameter( inParam, person1.getId() ); call.registerParameter( 2, Class.class, ParameterMode.REF_CURSOR ); Output output = call.getOutputs().getCurrent(); @@ -150,7 +152,7 @@ public class OracleStoredProcedureTest { entityManager -> { BigDecimal phoneCount = (BigDecimal) entityManager .createNativeQuery( "SELECT fn_count_phones(:personId) FROM DUAL" ) - .setParameter( "personId", 1 ) + .setParameter( "personId", person1.getId() ) .getSingleResult(); assertEquals( BigDecimal.valueOf( 2 ), phoneCount ); } @@ -163,7 +165,7 @@ public class OracleStoredProcedureTest { entityManager -> { List postAndComments = entityManager .createNamedStoredProcedureQuery( "personAndPhonesFunction" ) - .setParameter( 1, 1L ) + .setParameter( 1, person1.getId() ) .getResultList(); Object[] postAndComment = postAndComments.get( 0 ); Person person = (Person) postAndComment[0]; @@ -188,7 +190,7 @@ public class OracleStoredProcedureTest { //OracleTypes.CURSOR function.registerOutParameter( 1, -10 ); } - function.setInt( 2, 1 ); + function.setLong( 2, person1.getId() ); function.execute(); try (ResultSet resultSet = (ResultSet) function.getObject( 1 );) { while ( resultSet.next() ) { @@ -411,18 +413,18 @@ public class OracleStoredProcedureTest { ); statement.execute( - "create or replace function find_char(" + - " search in char, " + - " string in varchar," + - " start in integer default 0) " + - "return integer " + - "as " + - " position integer; " + - "begin " + - " select instr( search, string, start ) into position " + - " from dual; " + - " return position; " + - "end;" + "CREATE OR REPLACE FUNCTION find_char(" + + " search_char IN CHAR, " + + " string IN VARCHAR," + + " start_idx IN NUMBER DEFAULT 1) " + + "RETURN NUMBER " + + "IS " + + " pos NUMBER; " + + "BEGIN " + + " SELECT INSTR( string, search_char, start_idx ) INTO pos " + + " FROM dual; " + + " RETURN pos; " + + "END;" ); } catch (SQLException e) { @@ -432,7 +434,7 @@ public class OracleStoredProcedureTest { } ) ); scope.inTransaction( (entityManager) -> { - Person person1 = new Person( "John Doe" ); + person1 = new Person( "John Doe" ); person1.setNickName( "JD" ); person1.setAddress( "Earth" ); person1.setCreatedOn( Timestamp.from( LocalDateTime.of( 2000, 1, 1, 0, 0, 0 )