From 10797241253edea4c54d5ed1e16779070f26a327 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 26 Nov 2024 09:47:56 +0100 Subject: [PATCH] More Mysql2 adapter fixes to support prepared statements Followup: https://github.com/rails/rails/pull/53702 - Changed Mysql2 type_cast method to also coerce raw Time values. - Cleaned up some tests. - Improved the logic around when to skip using a prepared statement. --- .../connection_adapters/mysql/quoting.rb | 8 ++++- .../mysql2/database_statements.rb | 5 +-- .../cases/adapters/postgresql/quoting_test.rb | 6 ++-- .../test/cases/bind_parameter_test.rb | 7 ++-- activerecord/test/cases/explain_test.rb | 32 +++++++++---------- activerecord/test/cases/finder_test.rb | 4 +-- activerecord/test/cases/insert_all_test.rb | 2 +- activerecord/test/cases/test_case.rb | 2 +- 8 files changed, 36 insertions(+), 30 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb index 15e3d7984d033..70d99d2880edf 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/quoting.rb @@ -102,7 +102,13 @@ def type_cast(value) # :nodoc: else value.getlocal end - when Date, Time + when Time + if default_timezone == :utc + value.utc? ? value : value.getutc + else + value.utc? ? value.getlocal : value + end + when Date value else super diff --git a/activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb index 810257aa84ded..0f2edeaa0e141 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb @@ -48,7 +48,7 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif # made since we established the connection raw_connection.query_options[:database_timezone] = default_timezone - result = if !prepared_statements || binds.nil? || binds.empty? + result = if binds.nil? || binds.empty? ActiveSupport::Dependencies.interlock.permit_concurrent_loads do result = raw_connection.query(sql) @affected_rows_before_warnings = raw_connection.affected_rows @@ -74,14 +74,15 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif else stmt.close end + raise end + verified! result end - notification_payload[:affected_rows] = @affected_rows_before_warnings notification_payload[:row_count] = result&.size || 0 diff --git a/activerecord/test/cases/adapters/postgresql/quoting_test.rb b/activerecord/test/cases/adapters/postgresql/quoting_test.rb index acf4fe97db8de..c4f2362271e20 100644 --- a/activerecord/test/cases/adapters/postgresql/quoting_test.rb +++ b/activerecord/test/cases/adapters/postgresql/quoting_test.rb @@ -29,15 +29,15 @@ def test_quote_float_infinity assert_equal "'Infinity'", @conn.quote(infinity) end - def test_cast_bound_integer + def test_quote_integer assert_equal "42", @conn.quote(42) end - def test_cast_bound_big_decimal + def test_quote_big_decimal assert_equal "4.2", @conn.quote(BigDecimal("4.2")) end - def test_cast_bound_rational + def test_quote_rational assert_equal "3/4", @conn.quote(Rational(3, 4)) end diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index ef98644b1e135..d53759b8b7e3e 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -238,10 +238,11 @@ def assert_bind_params_to_sql # # SELECT `authors`.* FROM `authors` WHERE `authors`.`id` IN (1, 2, 3) # - if current_adapter?(:Mysql2Adapter) - params = bind_params((1..3).map(&:to_s)) + params = if current_adapter?(:Mysql2Adapter, :TrilogyAdapter) + # With MySQL integers are casted as string for security. + bind_params((1..3).map(&:to_s)) else - params = bind_params(1..3) + bind_params(1..3) end sql = "SELECT #{table}.* FROM #{table} WHERE #{pk} IN (#{params})" diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb index af0f41b2d84ff..b16dd7e7b9065 100644 --- a/activerecord/test/cases/explain_test.rb +++ b/activerecord/test/cases/explain_test.rb @@ -89,24 +89,22 @@ def test_relation_explain_with_sum assert_match(expected_query, message) end - unless current_adapter?(:Mysql2Adapter) && ActiveRecord::Base.lease_connection.prepared_statements - def test_relation_explain_with_first - expected_query = capture_sql { - Car.all.first - }.first - message = Car.all.explain.first - assert_match(/^EXPLAIN/, message) - assert_match(expected_query, message) - end + def test_relation_explain_with_first + expected_query = capture_sql { + Car.all.first + }.first + message = Car.all.explain.first + assert_match(/^EXPLAIN/, message) + assert_match(expected_query.sub(/LIMIT.*/, ""), message) + end - def test_relation_explain_with_last - expected_query = capture_sql { - Car.all.last - }.first - message = Car.all.explain.last - assert_match(/^EXPLAIN/, message) - assert_match(expected_query, message) - end + def test_relation_explain_with_last + expected_query = capture_sql { + Car.all.last + }.first + message = Car.all.explain.last + assert_match(/^EXPLAIN/, message) + assert_match(expected_query.sub(/LIMIT.*/, ""), message) end def test_relation_explain_with_pluck diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index eb9c4aff6a664..3964308bbb6e9 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1406,7 +1406,7 @@ def test_condition_utc_time_interpolation_with_default_timezone_local with_env_tz "America/New_York" do with_timezone_config default: :local do topic = Topic.first - assert_equal topic, Topic.where(["written_on = ?", topic.written_on]).first + assert_equal topic, Topic.where(["written_on = ?", topic.written_on.getutc]).first end end end @@ -1424,7 +1424,7 @@ def test_condition_local_time_interpolation_with_default_timezone_utc with_env_tz "America/New_York" do with_timezone_config default: :utc do topic = Topic.first - assert_equal topic, Topic.where(["written_on = ?", topic.written_on]).first + assert_equal topic, Topic.where(["written_on = ?", topic.written_on.getlocal]).first end end end diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index cdab4fe1ae422..e526364d8b1f6 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -787,7 +787,7 @@ def test_upsert_all_updates_using_provided_sql assert_equal "written", Book.find(2).status end - if current_adapter?(:Mysql2Adapter) || current_adapter?(:TrilogyAdapter) + if current_adapter?(:Mysql2Adapter, :TrilogyAdapter) def test_upsert_all_updates_using_values_function_on_duplicate_raw_sql skip unless supports_insert_on_duplicate_update? diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index 0e2491b83b494..c638f993504ed 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -308,7 +308,7 @@ def self.run(*args) class AbstractMysqlTestCase < TestCase def self.run(*args) - super if current_adapter?(:Mysql2Adapter) || current_adapter?(:TrilogyAdapter) + super if current_adapter?(:Mysql2Adapter, :TrilogyAdapter) end end