Permalink
Browse files

HHH-7984 - Oracle callable statement closing

  • Loading branch information...
1 parent 31219e2 commit 5e7b3601a974ecf0a2f2c60aedc6be1931388eef @lukasz-antoniak lukasz-antoniak committed with brmeyer Mar 21, 2013
@@ -370,14 +370,15 @@ public void release(Statement statement) {
}
@Override
- public void register(ResultSet resultSet) {
+ public void register(ResultSet resultSet, Statement statement) {
LOG.tracev( "Registering result set [{0}]", resultSet );
- Statement statement;
- try {
- statement = resultSet.getStatement();
- }
- catch ( SQLException e ) {
- throw exceptionHelper.convert( e, "unable to access statement from resultset" );
+ if ( statement == null ) {
+ try {
+ statement = resultSet.getStatement(); // best guess
+ }
+ catch ( SQLException e ) {
+ throw exceptionHelper.convert( e, "unable to access statement from resultset" );
+ }
}
if ( statement != null ) {
if ( LOG.isEnabled( Level.WARN ) && !xref.containsKey( statement ) ) {
@@ -54,7 +54,7 @@ public ResultSet extract(PreparedStatement statement) {
}
try {
ResultSet rs = statement.executeQuery();
- postExtract( rs );
+ postExtract( rs, statement );
return rs;
}
catch ( SQLException e ) {
@@ -68,7 +68,7 @@ public ResultSet extract(CallableStatement statement) {
// sql logged by StatementPreparerImpl
ResultSet rs = jdbcCoordinator.getLogicalConnection().getJdbcServices()
.getDialect().getResultSet( statement );
- postExtract( rs );
+ postExtract( rs, statement );
return rs;
}
catch ( SQLException e ) {
@@ -82,7 +82,7 @@ public ResultSet extract(Statement statement, String sql) {
.getSqlStatementLogger().logStatement( sql );
try {
ResultSet rs = statement.executeQuery( sql );
- postExtract( rs );
+ postExtract( rs, statement );
return rs;
}
catch ( SQLException e ) {
@@ -100,7 +100,7 @@ public ResultSet execute(PreparedStatement statement) {
}
}
ResultSet rs = statement.getResultSet();
- postExtract( rs );
+ postExtract( rs, statement );
return rs;
}
catch ( SQLException e ) {
@@ -119,7 +119,7 @@ public ResultSet execute(Statement statement, String sql) {
}
}
ResultSet rs = statement.getResultSet();
- postExtract( rs );
+ postExtract( rs, statement );
return rs;
}
catch ( SQLException e ) {
@@ -157,8 +157,8 @@ private final SqlExceptionHelper sqlExceptionHelper() {
.getSqlExceptionHelper();
}
- private void postExtract(ResultSet rs) {
- if ( rs != null ) jdbcCoordinator.register( rs );
+ private void postExtract(ResultSet rs, Statement st) {
+ if ( rs != null ) jdbcCoordinator.register( rs, st );
}
}
@@ -170,10 +170,15 @@
/**
* Register a JDBC result set.
+ * <p/>
+ * Implementation note: Second parameter has been introduced to prevent
+ * multiple registrations of the same statement in case {@link ResultSet#getStatement()}
+ * does not return original {@link Statement} object.
*
* @param resultSet The result set to register.
+ * @param statement Statement from which {@link ResultSet} has been generated.
*/
- public void register(ResultSet resultSet);
+ public void register(ResultSet resultSet, Statement statement);
/**
* Release a previously registered result set.
@@ -952,8 +952,9 @@ public Iterator iterate(QueryParameters queryParameters, EventSource session)
try {
final List<AfterLoadAction> afterLoadActions = new ArrayList<AfterLoadAction>();
- final ResultSet rs = executeQueryStatement( queryParameters, false, afterLoadActions, session );
- final PreparedStatement st = (PreparedStatement) rs.getStatement();
+ final SqlStatementWrapper wrapper = executeQueryStatement( queryParameters, false, afterLoadActions, session );
+ final ResultSet rs = wrapper.getResultSet();
+ final PreparedStatement st = (PreparedStatement) wrapper.getStatement();
HolderInstantiator hi = HolderInstantiator.createClassicHolderInstantiator(holderConstructor, queryParameters.getResultTransformer());
Iterator result = new IteratorImpl( rs, st, session, queryParameters.isReadOnly( session ), returnTypes, getColumnNames(), hi );
@@ -895,8 +895,9 @@ private List doQuery(
final List<AfterLoadAction> afterLoadActions = new ArrayList<AfterLoadAction>();
- final ResultSet rs = executeQueryStatement( queryParameters, false, afterLoadActions, session );
- final Statement st = rs.getStatement();
+ final SqlStatementWrapper wrapper = executeQueryStatement( queryParameters, false, afterLoadActions, session );
+ final ResultSet rs = wrapper.getResultSet();
+ final Statement st = wrapper.getStatement();
// would be great to move all this below here into another method that could also be used
// from the new scrolling stuff.
@@ -1803,15 +1804,15 @@ private ScrollMode getScrollMode(boolean scroll, boolean hasFirstRow, boolean us
* Process query string by applying filters, LIMIT clause, locks and comments if necessary.
* Finally execute SQL statement and advance to the first row.
*/
- protected ResultSet executeQueryStatement(
+ protected SqlStatementWrapper executeQueryStatement(
final QueryParameters queryParameters,
final boolean scroll,
List<AfterLoadAction> afterLoadActions,
final SessionImplementor session) throws SQLException {
return executeQueryStatement( getSQLString(), queryParameters, scroll, afterLoadActions, session );
}
- protected ResultSet executeQueryStatement(
+ protected SqlStatementWrapper executeQueryStatement(
String sqlStatement,
QueryParameters queryParameters,
boolean scroll,
@@ -1832,7 +1833,7 @@ protected ResultSet executeQueryStatement(
sql = preprocessSQL( sql, queryParameters, getFactory().getDialect(), afterLoadActions );
final PreparedStatement st = prepareQueryStatement( sql, queryParameters, limitHandler, scroll, session );
- return getResultSet( st, queryParameters.getRowSelection(), limitHandler, queryParameters.hasAutoDiscoverScalarTypes(), session );
+ return new SqlStatementWrapper( st, getResultSet( st, queryParameters.getRowSelection(), limitHandler, queryParameters.hasAutoDiscoverScalarTypes(), session ) );
}
/**
@@ -2586,8 +2587,9 @@ protected ScrollableResults scroll(
if ( stats ) startTime = System.currentTimeMillis();
try {
- final ResultSet rs = executeQueryStatement( queryParameters, true, Collections.<AfterLoadAction>emptyList(), session );
- final PreparedStatement st = (PreparedStatement) rs.getStatement();
+ final SqlStatementWrapper wrapper = executeQueryStatement( queryParameters, true, Collections.<AfterLoadAction>emptyList(), session );
+ final ResultSet rs = wrapper.getResultSet();
+ final PreparedStatement st = (PreparedStatement) wrapper.getStatement();
if ( stats ) {
getFactory().getStatisticsImplementor().queryExecuted(
@@ -2660,4 +2662,25 @@ public final SessionFactoryImplementor getFactory() {
public String toString() {
return getClass().getName() + '(' + getSQLString() + ')';
}
+
+ /**
+ * Wrapper class for {@link Statement} and associated {@link ResultSet}.
+ */
+ protected static class SqlStatementWrapper {
+ private final Statement statement;
+ private final ResultSet resultSet;
+
+ private SqlStatementWrapper(Statement statement, ResultSet resultSet) {
+ this.resultSet = resultSet;
+ this.statement = statement;
+ }
+
+ public ResultSet getResultSet() {
+ return resultSet;
+ }
+
+ public Statement getStatement() {
+ return statement;
+ }
+ }
}
@@ -253,8 +253,9 @@ private void doTheLoad(String sql, QueryParameters queryParameters, SessionImple
Integer.MAX_VALUE;
final List<AfterLoadAction> afterLoadActions = Collections.emptyList();
- final ResultSet rs = executeQueryStatement( sql, queryParameters, false, afterLoadActions, session );
- final Statement st = rs.getStatement();
+ final SqlStatementWrapper wrapper = executeQueryStatement( sql, queryParameters, false, afterLoadActions, session );
+ final ResultSet rs = wrapper.getResultSet();
+ final Statement st = wrapper.getStatement();
try {
processResultSet( rs, queryParameters, session, true, null, maxRows, afterLoadActions );
}
@@ -255,8 +255,9 @@ private List doTheLoad(String sql, QueryParameters queryParameters, SessionImple
Integer.MAX_VALUE;
final List<AfterLoadAction> afterLoadActions = new ArrayList<AfterLoadAction>();
- final ResultSet rs = executeQueryStatement( sql, queryParameters, false, afterLoadActions, session );
- final Statement st = rs.getStatement();
+ final SqlStatementWrapper wrapper = executeQueryStatement( sql, queryParameters, false, afterLoadActions, session );
+ final ResultSet rs = wrapper.getResultSet();
+ final Statement st = wrapper.getStatement();
try {
return processResultSet( rs, queryParameters, session, false, null, maxRows, afterLoadActions );
}
@@ -510,8 +510,9 @@ public Iterator iterate(
if ( queryParameters.isCallable() ) {
throw new QueryException("iterate() not supported for callable statements");
}
- final ResultSet rs = executeQueryStatement( queryParameters, false, Collections.<AfterLoadAction>emptyList(), session );
- final PreparedStatement st = (PreparedStatement) rs.getStatement();
+ final SqlStatementWrapper wrapper = executeQueryStatement( queryParameters, false, Collections.<AfterLoadAction>emptyList(), session );
+ final ResultSet rs = wrapper.getResultSet();
+ final PreparedStatement st = (PreparedStatement) wrapper.getStatement();
final Iterator result = new IteratorImpl(
rs,
st,
@@ -129,7 +129,7 @@ private void validateColumn(Connection connection, String columnName, int expect
String columnNamePattern = generateFinalNamePattern( meta, columnName );
ResultSet columnInfo = meta.getColumns( null, null, tableNamePattern, columnNamePattern );
- s.getTransactionCoordinator().getJdbcCoordinator().register(columnInfo);
+ s.getTransactionCoordinator().getJdbcCoordinator().register(columnInfo, columnInfo.getStatement());
assertTrue( columnInfo.next() );
int dataType = columnInfo.getInt( "DATA_TYPE" );
s.getTransactionCoordinator().getJdbcCoordinator().release( columnInfo );
@@ -89,6 +89,28 @@ public void testReadResultSetFromRefCursor() {
session.close();
}
+ @Test
+ @TestForIssue( jiraKey = "HHH-7984" )
+ public void testStatementClosing() {
+ Session session = openSession();
+ session.getTransaction().begin();
+ // Reading maximum number of opened cursors requires SYS privileges.
+ // Verify statement closing with JdbcCoordinator#hasRegisteredResources() instead.
+ // BigDecimal maxCursors = (BigDecimal) session.createSQLQuery( "SELECT value FROM v$parameter WHERE name = 'open_cursors'" ).uniqueResult();
+ // for ( int i = 0; i < maxCursors + 10; ++i ) { named_query_execution }
+ Assert.assertEquals(
+ Arrays.asList( new NumValue( 1, "Line 1" ), new NumValue( 2, "Line 2" ) ),
+ session.getNamedQuery( "NumValue.getSomeValues" ).list()
+ );
+ JdbcCoordinator jdbcCoordinator = ( (SessionImplementor) session ).getTransactionCoordinator().getJdbcCoordinator();
+ Assert.assertFalse(
+ "Prepared statement and result set should be released after query execution.",
+ jdbcCoordinator.hasRegisteredResources()
+ );
+ session.getTransaction().commit();
+ session.close();
+ }
+
private void executeStatement(final String sql) {
final Session session = openSession();
session.getTransaction().begin();

4 comments on commit 5e7b360

Contributor

christianbauer replied Mar 26, 2013

This fixes the problem with LOG.unregisteredStatement() in register(ResultSet) but I'm still seeing warnings logged in release(ResultSet). That method still calls resultSet.getStatement() only and if the returned PreparedStatement isn't equal() with the original, the ResultSet won't get removed from xref.

Member

lukasz-antoniak replied Mar 26, 2013

@christianbauer is right. Defect in JIRA affected only Loader class which releases statements, not result sets. release(ResultSet) escaped my notice.

Member

brmeyer replied Mar 26, 2013

@lukasz-antoniak , are you taking a stab at that, or would you like me to? Thanks!

Member

brmeyer replied Mar 26, 2013

I add this in dc193c3

Please sign in to comment.