Skip to content

Commit 5bd86de

Browse files
skunkworkerclaude
andcommitted
Fix PG connection leak on failed establishment and add SQL warnings support
1.1 Connection leak: PostgreSQLRubyJdbcConnection#newConnection opened a physical backend via super.newConnection() but then ran unwrap()/addDataType() without cleanup. Any failure there leaked the open server-side connection, since the caller only saw the exception and never got a handle to close it. Wrap the post-connect setup so the connection is closed (suppressing close errors) before re-raising, mirroring the native adapter discarding a connection it could not fully establish. 1.2 SQL warnings (AR 7.2 db_warnings_action): capture statement.getWarnings() in RubyJdbcConnection#execute before the statement closes and expose them via #last_warnings. PostgreSQLRubyJdbcConnection#newWarning maps PSQLWarning's ServerErrorMessage to [message, sqlstate, severity]. The PG raw_execute override dispatches them through handle_warnings to ActiveRecord .db_warnings_action and normalises the Integer update-count return to [] for PG::Result API parity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent dbe2bb7 commit 5bd86de

3 files changed

Lines changed: 139 additions & 11 deletions

File tree

lib/arjdbc/postgresql/database_statements.rb

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,46 @@ def build_explain_clause(options = [])
1515

1616
"EXPLAIN (#{options.join(", ").upcase})"
1717
end
18+
19+
# Overridden to surface any SQL warnings (PostgreSQL NOTICE / RAISE
20+
# WARNING messages) emitted while running +sql+ to the configured
21+
# +ActiveRecord.db_warnings_action+, matching the native PostgreSQL
22+
# adapter. The warnings themselves are collected on the Java side during
23+
# #execute and read back here via #last_warnings.
24+
def raw_execute(sql, name, async: false, allow_retry: false, materialize_transactions: true)
25+
log(sql, name, async: async) do
26+
with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn|
27+
result = conn.execute(sql)
28+
verified!
29+
handle_warnings(sql)
30+
# The native adapter returns a result object (PG::Result) whose
31+
# #to_a is []; statements without a result set come back as an
32+
# update count here, so normalise to an array for API parity.
33+
result.is_a?(Integer) ? [] : result
34+
end
35+
end
36+
end
37+
38+
private
39+
40+
# Dispatches SQL warnings collected by the most recent #execute to
41+
# +ActiveRecord.db_warnings_action+ (mirrors the native adapter).
42+
def handle_warnings(sql)
43+
return if ActiveRecord.db_warnings_action.nil?
44+
45+
@raw_connection.last_warnings.each do |message, code, level|
46+
warning = ActiveRecord::SQLWarning.new(message, code, level, sql, @pool)
47+
next if warning_ignored?(warning)
48+
49+
ActiveRecord.db_warnings_action.call(warning)
50+
end
51+
end
52+
53+
# Only WARNING and above are treated as SQL warnings; NOTICE/INFO/DEBUG/LOG
54+
# level messages are ignored, as in the native PostgreSQL adapter.
55+
def warning_ignored?(warning)
56+
["WARNING", "ERROR", "FATAL", "PANIC"].exclude?(warning.level) || super
57+
end
1858
end
1959
end
2060
end

src/java/arjdbc/jdbc/RubyJdbcConnection.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.sql.ResultSet;
4141
import java.sql.ResultSetMetaData;
4242
import java.sql.SQLException;
43+
import java.sql.SQLWarning;
4344
import java.sql.SQLXML;
4445
import java.sql.Statement;
4546
import java.sql.Date;
@@ -133,6 +134,10 @@ public class RubyJdbcConnection extends RubyObject {
133134
private boolean configureConnection = true; // final once initialized
134135
private int fetchSize = 0; // 0 = JDBC default
135136

137+
// SQL warnings collected by the most recent #execute call, exposed to the
138+
// adapter via #last_warnings so it can honour ActiveRecord.db_warnings_action.
139+
private transient IRubyObject lastWarnings;
140+
136141
protected RubyJdbcConnection(Ruby runtime, RubyClass metaClass) {
137142
super(runtime, metaClass);
138143
attributeClass = runtime.getModule("ActiveModel").getClass("Attribute");
@@ -807,6 +812,10 @@ public IRubyObject execute(final ThreadContext context, final IRubyObject sql) {
807812
updateCount = statement.getUpdateCount();
808813
}
809814

815+
// Capture any SQL warnings (e.g. PostgreSQL RAISE WARNING / NOTICE)
816+
// raised while executing, before the statement is closed below.
817+
lastWarnings = mapWarnings(context, statement.getWarnings());
818+
810819
return result;
811820

812821
} catch (final SQLException e) {
@@ -818,6 +827,42 @@ public IRubyObject execute(final ThreadContext context, final IRubyObject sql) {
818827
});
819828
}
820829

830+
/**
831+
* @return the SQL warnings collected by the most recent {@link #execute}
832+
* as an Array of <code>[message, sql_state, level]</code> arrays.
833+
*/
834+
@JRubyMethod(name = "last_warnings")
835+
public IRubyObject last_warnings(final ThreadContext context) {
836+
return lastWarnings == null ? context.runtime.newEmptyArray() : lastWarnings;
837+
}
838+
839+
/**
840+
* Maps a chain of {@link SQLWarning}s to a Ruby Array of warning tuples.
841+
* @param warning the head of the warning chain (may be null)
842+
* @return a (possibly empty) Ruby Array of <code>[message, sql_state, level]</code>
843+
*/
844+
protected IRubyObject mapWarnings(final ThreadContext context, SQLWarning warning) {
845+
final RubyArray warnings = context.runtime.newArray();
846+
while (warning != null) {
847+
warnings.append(newWarning(context, warning));
848+
warning = warning.getNextWarning();
849+
}
850+
return warnings;
851+
}
852+
853+
/**
854+
* Builds a single warning tuple <code>[message, sql_state, level]</code>.
855+
* The generic JDBC API does not expose a severity level, so it is left nil;
856+
* adapters with richer driver support (e.g. PostgreSQL) may override this.
857+
*/
858+
protected IRubyObject newWarning(final ThreadContext context, final SQLWarning warning) {
859+
final Ruby runtime = context.runtime;
860+
final IRubyObject message = RubyString.newUnicodeString(runtime, warning.getMessage());
861+
final String sqlState = warning.getSQLState();
862+
final IRubyObject code = sqlState == null ? context.nil : RubyString.newUnicodeString(runtime, sqlState);
863+
return runtime.newArray(message, code, context.nil);
864+
}
865+
821866
protected Statement createStatement(final ThreadContext context, final Connection connection)
822867
throws SQLException {
823868
final Statement statement = connection.createStatement();

src/java/arjdbc/postgresql/PostgreSQLRubyJdbcConnection.java

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@
6666
import org.postgresql.geometric.PGpolygon;
6767
import org.postgresql.util.PGInterval;
6868
import org.postgresql.util.PGobject;
69+
import org.postgresql.util.PSQLWarning;
70+
import org.postgresql.util.ServerErrorMessage;
6971

7072
/**
7173
*
@@ -241,19 +243,35 @@ protected Connection newConnection() throws RaiseException, SQLException {
241243
}
242244
throw ex;
243245
}
244-
final PGConnection pgConnection;
245-
if ( connection instanceof PGConnection ) {
246-
pgConnection = (PGConnection) connection;
246+
// The physical connection is open now; if any of the post-connect
247+
// setup below fails we must close it, otherwise the server-side backend
248+
// leaks (the caller only sees the exception and never gets a handle to
249+
// close). This mirrors the native adapter discarding a connection that
250+
// could not be fully established.
251+
try {
252+
final PGConnection pgConnection;
253+
if ( connection instanceof PGConnection ) {
254+
pgConnection = (PGConnection) connection;
255+
}
256+
else {
257+
pgConnection = connection.unwrap(PGConnection.class);
258+
}
259+
pgConnection.addDataType("daterange", DateRangeType.class);
260+
pgConnection.addDataType("tsrange", TsRangeType.class);
261+
pgConnection.addDataType("tstzrange", TstzRangeType.class);
262+
pgConnection.addDataType("int4range", Int4RangeType.class);
263+
pgConnection.addDataType("int8range", Int8RangeType.class);
264+
pgConnection.addDataType("numrange", NumRangeType.class);
247265
}
248-
else {
249-
pgConnection = connection.unwrap(PGConnection.class);
266+
catch (SQLException|RuntimeException ex) {
267+
try {
268+
connection.close();
269+
}
270+
catch (SQLException closeError) {
271+
ex.addSuppressed(closeError);
272+
}
273+
throw ex;
250274
}
251-
pgConnection.addDataType("daterange", DateRangeType.class);
252-
pgConnection.addDataType("tsrange", TsRangeType.class);
253-
pgConnection.addDataType("tstzrange", TstzRangeType.class);
254-
pgConnection.addDataType("int4range", Int4RangeType.class);
255-
pgConnection.addDataType("int8range", Int8RangeType.class);
256-
pgConnection.addDataType("numrange", NumRangeType.class);
257275
return connection;
258276
}
259277

@@ -277,6 +295,31 @@ protected IRubyObject mapQueryResult(final ThreadContext context, final Connecti
277295
return mapExecuteResult(context, connection, resultSet).toARResult(context);
278296
}
279297

298+
/**
299+
* Builds a warning tuple <code>[message, sql_state, level]</code> for a
300+
* PostgreSQL server message (e.g. <code>RAISE WARNING</code> / NOTICE).
301+
* The driver wraps these as {@link PSQLWarning}, which carries the original
302+
* {@link ServerErrorMessage} with the clean primary message, SQLSTATE and
303+
* severity ("WARNING", "NOTICE", ...) that ActiveRecord's db_warnings
304+
* handling needs.
305+
*/
306+
@Override
307+
protected IRubyObject newWarning(final ThreadContext context, final SQLWarning warning) {
308+
if (warning instanceof PSQLWarning) {
309+
final ServerErrorMessage serverError = ((PSQLWarning) warning).getServerErrorMessage();
310+
if (serverError != null) {
311+
final Ruby runtime = context.runtime;
312+
final IRubyObject message = RubyString.newUnicodeString(runtime, serverError.getMessage());
313+
final String sqlState = serverError.getSQLState();
314+
final IRubyObject code = sqlState == null ? context.nil : RubyString.newUnicodeString(runtime, sqlState);
315+
final String severity = serverError.getSeverity();
316+
final IRubyObject level = severity == null ? context.nil : RubyString.newUnicodeString(runtime, severity);
317+
return runtime.newArray(message, code, level);
318+
}
319+
}
320+
return super.newWarning(context, warning);
321+
}
322+
280323
@Override
281324
protected void setArrayParameter(final ThreadContext context,
282325
final Connection connection, final PreparedStatement statement,

0 commit comments

Comments
 (0)