fix(spi): 9 fixes for connection pool, SQL execution, and context#1924
fix(spi): 9 fixes for connection pool, SQL execution, and context#1924HandSonic wants to merge 8 commits into
Conversation
openai0229
left a comment
There was a problem hiding this comment.
Please split this PR into focused changes. It currently bundles nine unrelated fixes across connection-pool concurrency, transaction state, SQL generation, SQL Server schema handling, equality/hash behavior, and plugin lookup. These areas have different failure modes and need independent tests and rollback boundaries.
In particular, please add concurrency coverage for connection cleanup/queue behavior; preserve and restore the caller's original autoCommit state instead of forcing true; and test commit/rollback behavior around connection failures. The MySQL extent, SQL Server schema, equals/hashCode, and plugin lookup changes also need focused coverage.
The count SQL fix overlaps #1966. Please remove it here and let the smaller #1966 land independently.
buildExt() checked column.getExtent() for emptiness but returned column.getComment() — a copy-paste error from buildComment() above it.
…eName() The second condition duplicated the first, checking databaseName twice. When databaseName was blank but schemaName was not, the schema was skipped.
Empty catch blocks silently swallowed failures when setting catalog or schema, leaving the connection pointing at the wrong database with no indication of the problem.
- Cleanup thread now calls iterator.remove() after closing stale/dead connections, preventing unbounded queue growth - getConnection() returns ConnectInfo to queue when trySetInUse() fails, preventing connection leaks under concurrency
equals() compared {dataSourceId, gmtModified} but hashCode() used
{dataSourceId, consoleId, databaseName}, violating the Java contract.
Objects that are equal must have the same hashCode.
- ResultSet now in try-with-resources to prevent cursor leak - catch block calls connection.rollback() on failure - finally block restores connection.setAutoCommit(true) regardless of outcome Previously, exceptions left the connection in autoCommit=false with no rollback.
ResultSet from stmt.getResultSet() was not explicitly closed. Some JDBC drivers do not auto-close ResultSets when the parent Statement is closed, leading to cursor leaks under repeated use.
|
Updated the PR per review feedback:
The remaining focused changes are:
|
PLUGIN_MAP.get(dbType) returned null for unknown types, causing opaque NPE. Now throws IllegalArgumentException with the unsupported type name and list of registered types.
Summary
Fixes 9 verified bugs in the SPI core layer.
Fixes included:
s.length()→countSql.length()in substring (CRITICAL)getComment()instead ofgetExtent()(CRITICAL)databaseNametwice instead ofschemaName(CRITICAL)setCatalog()/setSchema()now log warningsiterator.remove()after closing dead connectionstrySetInUse()failsequals()fields{dataSourceId, gmtModified}