From a588dd0167a31007e4c7cb22cff2544a5ece2a2c Mon Sep 17 00:00:00 2001 From: Zach Haitz Date: Mon, 14 Sep 2026 16:40:48 +0000 Subject: [PATCH] fix: infer types for untyped query binds * Queries like Singer.where("first_name = ?", "Alice") on Rails 8.1 and similar Arel.sql queries send an untyped string into the adapter, which currently declares it as INT64. * Update to_types_and_params to infer the correct Spanner types for these values. * Use an allowlist to preserve existing behavior for arrays and other unsupported types. --- .../spanner/database_statements.rb | 17 ++++- ...ner_active_record_with_mock_server_test.rb | 73 +++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/lib/active_record/connection_adapters/spanner/database_statements.rb b/lib/active_record/connection_adapters/spanner/database_statements.rb index 622fd470..2ca92c83 100644 --- a/lib/active_record/connection_adapters/spanner/database_statements.rb +++ b/lib/active_record/connection_adapters/spanner/database_statements.rb @@ -342,11 +342,10 @@ def rollback_db_transaction private # Translates binds to Spanner types and params. - def to_types_and_params binds # rubocop:disable Metrics/AbcSize + def to_types_and_params binds return [{}, {}] if binds.empty? converter = ActiveRecord::Type::Spanner::SpannerActiveRecordConverter - integer_type = ActiveModel::Type::Integer types = {} params = {} index = 0 @@ -365,14 +364,24 @@ def to_types_and_params binds # rubocop:disable Metrics/AbcSize types[key] = :BOOL params[key] = bind_value else - types[key] = :INT64 - params[key] = converter.serialize_with_transaction_isolation_level integer_type, bind_value, :dml + types[key] = untyped_bind_type bind_value + params[key] = bind_value end index += 1 end [types, params] end + def untyped_bind_type value + case value + when ::String, ::Float, ::BigDecimal, ::Time, ::Date + Google::Cloud::Spanner::Convert.field_for_object value + else + # Preserve the existing fallback for nil and unsupported values. + :INT64 + end + end + def to_types binds to_types_and_params(binds)[0] end diff --git a/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb b/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb index d3718650..e126dcd8 100644 --- a/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb +++ b/test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb @@ -693,6 +693,79 @@ def test_find_singer_by_last_performance_as_non_iso_string assert_equal timestamp.utc.rfc3339(9), request.params["p1"] end + def test_untyped_binds_from_arel_sql_are_typed_by_ruby_class + select_sql = "SELECT `singers`.* FROM `singers` WHERE first_name = @p1 AND active = @p2 AND weight = @p3 " \ + "AND balance = @p4 AND last_performance = @p5 AND created_at = @p6 AND birth_date = @p7 AND age = @p8" + @mock.put_statement_result select_sql, MockServerTests::create_random_singers_result(1) + + time = ::Time.parse("2021-05-12T10:30:00+02:00") + date_time = ::DateTime.new(2021, 5, 12, 10, 30, 0, "+02:00") + Singer.where( + Arel.sql( + "first_name = ? AND active = ? AND weight = ? AND balance = ? AND last_performance = ? " \ + "AND created_at = ? AND birth_date = ? AND age = ?", + "Alice", true, 1.5, BigDecimal("12.34"), time, date_time, ::Date.new(2021, 5, 12), 42 + ) + ).to_a + + request = @mock.requests.select {|req| req.is_a?(Google::Cloud::Spanner::V1::ExecuteSqlRequest) && req.sql == select_sql }.first + refute_nil request + assert_equal :STRING, request.param_types["p1"].code + assert_equal "Alice", request.params["p1"] + assert_equal :BOOL, request.param_types["p2"].code + assert_equal true, request.params["p2"] + assert_equal :FLOAT64, request.param_types["p3"].code + assert_equal 1.5, request.params["p3"] + assert_equal :NUMERIC, request.param_types["p4"].code + assert_equal "12.34", request.params["p4"] + assert_equal :TIMESTAMP, request.param_types["p5"].code + assert_equal "2021-05-12T08:30:00.000000000Z", request.params["p5"] + assert_equal :TIMESTAMP, request.param_types["p6"].code + assert_equal "2021-05-12T08:30:00.000000000Z", request.params["p6"] + assert_equal :DATE, request.param_types["p7"].code + assert_equal "2021-05-12", request.params["p7"] + assert_equal :INT64, request.param_types["p8"].code + assert_equal "42", request.params["p8"] + end + + def test_untyped_time_with_zone_bind_is_a_timestamp + select_sql = "SELECT `singers`.* FROM `singers` WHERE last_performance = @p1" + @mock.put_statement_result select_sql, MockServerTests::create_random_singers_result(1) + + time = ActiveSupport::TimeZone["America/Los_Angeles"].local(2021, 5, 12, 10, 30, 0) + assert_instance_of ActiveSupport::TimeWithZone, time + Singer.where(Arel.sql("last_performance = ?", time)).to_a + + request = @mock.requests.select { |req| req.is_a?(ExecuteSqlRequest) && req.sql == select_sql }.first + refute_nil request + assert_equal :TIMESTAMP, request.param_types["p1"].code + assert_equal "2021-05-12T17:30:00.000000000Z", request.params["p1"] + end + + def test_untyped_nil_and_unsupported_binds_preserve_the_integer_fallback + binds = [nil, [], Object.new] + types, params = Singer.connection.send(:to_types_and_params, binds) + + assert_equal({ "p1" => :INT64, "p2" => :INT64, "p3" => :INT64 }, types) + binds.each_with_index do |bind, index| + assert_same bind, params["p#{index + 1}"] + end + end + + def test_where_with_positional_string_placeholder + # Before ActiveRecord 8.1, `where("col = ?", value)` inlines the value into the SQL instead of binding it. + skip "Requires Rails version 8.1 or higher" if ActiveRecord.version < Gem::Version.create("8.1.0") + select_sql = "SELECT `singers`.* FROM `singers` WHERE (first_name = @p1)" + @mock.put_statement_result select_sql, MockServerTests::create_random_singers_result(1) + + Singer.where("first_name = ?", "Alice").to_a + + request = @mock.requests.select {|req| req.is_a?(Google::Cloud::Spanner::V1::ExecuteSqlRequest) && req.sql == select_sql }.first + refute_nil request + assert_equal :STRING, request.param_types["p1"].code + assert_equal "Alice", request.params["p1"] + end + def test_create_singer_with_picture insert_sql = "INSERT INTO `singers` (`first_name`, `last_name`, `picture`, `id`) VALUES (@p1, @p2, @p3, @p4)" @mock.put_statement_result insert_sql, StatementResult.new(1)