Skip to content

Conversation

rajucomp
Copy link
Contributor

PR Summary: Fix Resource Leak in performQuery Method

Overview

This PR addresses resource leaks in the performQuery method by implementing proper JDBC resource management using the Consumer pattern.

Changes Made

Core Enhancement:

  • Modified AbstractContainerDatabaseTest.performQuery() method signature:
    • Before: protected ResultSet performQuery(JdbcDatabaseContainer<?> container, String sql) throws SQLException
    • After: protected void performQuery(JdbcDatabaseContainer<?> container, String sql, Consumer<ResultSet> consumer) throws SQLException
  • Implemented try-with-resources pattern to ensure proper cleanup of Connection, Statement, and ResultSet objects
  • Prevents resource leaks by guaranteeing JDBC resources are closed after query execution

Test Updates (20 files):
Updated all database module tests to use the new lambda-based pattern:

  • ClickHouse (2 files)
  • CockroachDB
  • CrateDB
  • Databend
  • DB2
  • MariaDB
  • MS SQL Server
  • MySQL (2 files)
  • OceanBase
  • Oracle (Free & XE)
  • PostgreSQL (3 files including TimescaleDB & CompatibleImage)
  • QuestDB
  • TiDB
  • Timeplus
  • YugabyteDB

Pattern Applied:

// Old pattern (resource leak)
ResultSet resultSet = performQuery(container, "SELECT 1");
int value = resultSet.getInt(1);
assertThat(value).isEqualTo(1);

// New pattern (proper resource management)
performQuery(container, "SELECT 1", resultSet -> {
    Assertions.assertThatNoException().isThrownBy(() -> {
        int value = resultSet.getInt(1);
        assertThat(value).isEqualTo(1);
    });
});

Impact

  • 21 files changed, 744 insertions(+), 251 deletions(-)
  • All tests compile successfully
  • Code formatting (Spotless) applied
  • Checkstyle validation passed

Note

This PR is scoped to the performQuery enhancement only to facilitate easier review. Additional resource leak fixes across the codebase will follow in subsequent PRs.

- Changed performQuery method signature from returning ResultSet to accepting Consumer<ResultSet>
- Implemented proper resource management with try-with-resources for Connection, Statement, and ResultSet
- Updated all test files across database modules to use new lambda-based pattern
- Wrapped assertions in assertThatNoException().isThrownBy() blocks for proper exception handling

This change ensures JDBC resources are properly closed after query execution, preventing resource leaks.
Note: Additional resource leak fixes will follow in subsequent PRs. This PR focuses on the performQuery enhancement to facilitate easier review.
@rajucomp
Copy link
Contributor Author

@eddumelendez Let me know if this is worthy of a review. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant