Skip to content

VELOCITY-989 - Review data sources resource loading#62

Open
arkanovicz wants to merge 4 commits intomasterfrom
velocity-989
Open

VELOCITY-989 - Review data sources resource loading#62
arkanovicz wants to merge 4 commits intomasterfrom
velocity-989

Conversation

@arkanovicz
Copy link
Copy Markdown
Contributor

Address VELOCITY-989 in a different way (hopefully cleaner) than PR 57.

Mainly:

  • fixed potential leaks on failures
  • introduces a prepared statements holder to avoid relying on statement.getConnection() (which behaves inconsistently across different jdbc pools)
  • deprecates old DatabaseObjectsFactory, but makes it functional by use of a temporary class proxy

…ningReader

Calling ResultSet.getStatement() after ResultSet.close() is driver-dependent:
DBCP2 returns null and the resulting NPE was silently swallowed, leaking the
prepared statement and its connection. Fetch the statement reference before
closing the result set.
…seObjectsFactory

The CachingDatabaseObjectsFactory class was removed in 3484d89. Its javadoc
@link was still dangling in DataSourceResourceLoader, along with an orphaned
statements_pool_max_size config line in the example snippet. Restate the SPI
contract: pooling and statement caching belong to the DataSource / JDBC driver,
not to Velocity.
@arkanovicz arkanovicz requested a review from michael-o April 22, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants