Skip to content

Commit 07f9fef

Browse files
BuonOmorafiss
authored andcommitted
fix: Refactor new connection
The old method was calling `AbstractAdapter#initialize` with a connection object, rather than a hash. This was then setting `@unconfigured_connection`, which in turns forced to re-run `#configure_connection` when a first query was made. This is usually OK (although unnecessary) except if `use_follower_reads_for_type_introspection` is used, since it sets AOST for a single query, which fails within a transaction. Fixes #320
1 parent 97ce388 commit 07f9fef

File tree

2 files changed

+23
-27
lines changed

2 files changed

+23
-27
lines changed

Gemfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ group :development, :test do
5555
gem "debug"
5656
gem "minitest-excludes", "~> 2.0.1"
5757
gem "minitest-github_action_reporter", github: "BuonOmo/minitest-github_action_reporter", require: "minitest/github_action_reporter_plugin"
58+
gem "ostruct", "~> 0.6"
5859

5960
# Gems used for tests meta-programming.
6061
gem "parser"

lib/active_record/connection_adapters/cockroachdb_adapter.rb

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,35 +32,15 @@
3232
ActiveRecord::ConnectionAdapters::CockroachDB.initial_setup
3333

3434
module ActiveRecord
35+
# TODO: once in rails 7.2, remove this and replace with a `#register` call.
36+
# See: https://github.com/rails/rails/commit/22a26d7f74ea8f0d5f7c4169531ae38441cfd5e5#diff-2468c670eb10c24bd2823e42708489a336d6f21c6efc7e3c4a574166fa77bb22
3537
module ConnectionHandling
38+
def cockroachdb_adapter_class
39+
ConnectionAdapters::CockroachDBAdapter
40+
end
41+
3642
def cockroachdb_connection(config)
37-
# This is copied from the PostgreSQL adapter.
38-
conn_params = config.symbolize_keys.compact
39-
40-
# Map ActiveRecords param names to PGs.
41-
conn_params[:user] = conn_params.delete(:username) if conn_params[:username]
42-
conn_params[:dbname] = conn_params.delete(:database) if conn_params[:database]
43-
44-
# Forward only valid config params to PG::Connection.connect.
45-
valid_conn_param_keys = PG::Connection.conndefaults_hash.keys + [:requiressl]
46-
conn_params.slice!(*valid_conn_param_keys)
47-
48-
ConnectionAdapters::CockroachDBAdapter.new(
49-
ConnectionAdapters::CockroachDBAdapter.new_client(conn_params),
50-
logger,
51-
conn_params,
52-
config
53-
)
54-
# This rescue flow appears in new_client, but it is needed here as well
55-
# since Cockroach will sometimes not raise until a query is made.
56-
rescue ActiveRecord::StatementInvalid => error
57-
no_db_err_check1 = conn_params && conn_params[:dbname] && error.cause.message.include?(conn_params[:dbname])
58-
no_db_err_check2 = conn_params && conn_params[:dbname] && error.cause.message.include?("pg_type")
59-
if no_db_err_check1 || no_db_err_check2
60-
raise ActiveRecord::NoDatabaseError
61-
else
62-
raise ActiveRecord::ConnectionNotEstablished, error.message
63-
end
43+
cockroachdb_adapter_class.new(config)
6444
end
6545
end
6646
end
@@ -255,6 +235,21 @@ def self.database_exists?(config)
255235
false
256236
end
257237

238+
def initialize(...)
239+
super
240+
241+
# This rescue flow appears in new_client, but it is needed here as well
242+
# since Cockroach will sometimes not raise until a query is made.
243+
rescue ActiveRecord::StatementInvalid => error
244+
no_db_err_check1 = @connection_parameters && @connection_parameters[:dbname] && error.cause.message.include?(@connection_parameters[:dbname])
245+
no_db_err_check2 = @connection_parameters && @connection_parameters[:dbname] && error.cause.message.include?("pg_type")
246+
if no_db_err_check1 || no_db_err_check2
247+
raise ActiveRecord::NoDatabaseError
248+
else
249+
raise ActiveRecord::ConnectionNotEstablished, error.message
250+
end
251+
end
252+
258253
# override
259254
# The PostgreSQLAdapter uses syntax for an anonymous function
260255
# (DO $$) that CockroachDB does not support.

0 commit comments

Comments
 (0)