From d25f495f10ec9362f8592080e9b3e434c8be4c3b Mon Sep 17 00:00:00 2001 From: Zach Haitz Date: Wed, 9 Sep 2026 07:38:45 -0400 Subject: [PATCH 1/5] fix: type untyped binds by Ruby class instead of defaulting to INT64 Binds that come from raw SQL placeholders (`Arel.sql("name = ?", "abc")` on 7.1+, or `where("name = ?", "abc")` on Rails 8.1+) arrive as bare Ruby values with no attached ActiveModel type. `to_types` only recognised query attributes, Symbols and booleans, and declared everything else INT64, so a String bind was rejected by Spanner with "Expected INT64". `to_params` always serialised such binds through the Integer type as well. Add `untyped_bind_type`, which maps String, true/false, Float, BigDecimal, Time/DateTime and Date to the matching ActiveModel type, and use it from both `to_types` and `to_params` so the declared type and serialised value agree. Unrecognised values keep the previous behaviour (INT64, value sent as-is). Co-Authored-By: Claude Fable 5.1 --- .../spanner/database_statements.rb | 41 +++++++++++----- ...ner_active_record_with_mock_server_test.rb | 49 +++++++++++++++++++ 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/lib/active_record/connection_adapters/spanner/database_statements.rb b/lib/active_record/connection_adapters/spanner/database_statements.rb index 82ac22ee..7bc52a63 100644 --- a/lib/active_record/connection_adapters/spanner/database_statements.rb +++ b/lib/active_record/connection_adapters/spanner/database_statements.rb @@ -378,16 +378,18 @@ def to_types_and_params binds def to_types binds binds.enum_for(:each_with_index).to_h do |bind, i| - type = :INT64 - if bind.respond_to? :type - type = ActiveRecord::Type::Spanner::SpannerActiveRecordConverter - .convert_active_model_type_to_spanner(bind.type) - elsif bind.instance_of? Symbol - # This ensures that for example :environment is sent as the string 'environment' to Cloud Spanner. - type = :STRING - elsif bind.instance_of?(TrueClass) || bind.instance_of?(FalseClass) - type = :BOOL - end + type = if bind.respond_to? :type + ActiveRecord::Type::Spanner::SpannerActiveRecordConverter + .convert_active_model_type_to_spanner(bind.type) + elsif bind.instance_of? Symbol + # This ensures that for example :environment is sent as the string 'environment' to Cloud Spanner. + :STRING + else + # Untyped binds (e.g. from `Arel.sql("name = ?", "abc")`) are bare Ruby values without an + # attached ActiveModel type. Derive the Spanner type from the Ruby class, defaulting to INT64. + ActiveRecord::Type::Spanner::SpannerActiveRecordConverter + .convert_active_model_type_to_spanner(untyped_bind_type(bind)) || :INT64 + end [ # Generates binds for named parameters in the format `@p1, @p2, ...` "p#{i + 1}", type @@ -403,8 +405,8 @@ def to_params binds # This ensures that for example :environment is sent as the string 'environment' to Cloud Spanner. :STRING else - # The Cloud Spanner default type is INT64 if no other type is known. - ActiveModel::Type::Integer + # Untyped bind: pick the serializer that matches the type declared in `to_types`. + untyped_bind_type bind end bind_value = bind.respond_to?(:value) ? bind.value : bind value = ActiveRecord::Type::Spanner::SpannerActiveRecordConverter @@ -414,6 +416,21 @@ def to_params binds end end + # Maps a bare Ruby value (a bind without an attached ActiveModel type) to the ActiveModel type + # that should be used to declare and serialize it. Returns nil for values that are not recognized, + # which keeps the historical behavior: the bind is declared as INT64 and the value is sent as-is. + def untyped_bind_type value + case value + when ::String then ActiveModel::Type::String.new + when true, false then ActiveModel::Type::Boolean.new + when ::Float then ActiveModel::Type::Float.new + when ::BigDecimal then ActiveModel::Type::Decimal.new + # DateTime is a subclass of Date, so it must be matched before Date. + when ::Time, ::DateTime then ActiveRecord::Type::Spanner::Time.new + when ::Date then ActiveModel::Type::Date.new + end + end + # An insert/update/delete statement could use mutations in some specific circumstances. # This method returns an indication whether a specific operation should use mutations instead of DML # based on the operation itself, and the current transaction. 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 897315eb..4d24cdfa 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 @@ -697,6 +697,55 @@ 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_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) From 03eb6ef66bccdbb0581239da830f958078550af4 Mon Sep 17 00:00:00 2001 From: Zach Haitz Date: Mon, 14 Sep 2026 12:05:44 +0000 Subject: [PATCH 2/5] fix: restore CI dependency and timestamp compatibility --- Gemfile | 2 ++ acceptance/cases/migration/change_schema_test.rb | 2 +- acceptance/cases/migration/schema_dumper_test.rb | 2 +- acceptance/cases/type/time_test.rb | 6 +++--- .../connection_adapters/spanner/database_statements.rb | 2 +- lib/active_record/tasks/spanner_database_tasks.rb | 2 +- 6 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Gemfile b/Gemfile index 940d9aac..601720cf 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,8 @@ gemspec ar_version = ENV.fetch("AR_VERSION", "~> 7.1.0") gem "activerecord", ar_version gem "ostruct" +# Rails 7 and docker-api still use JSON options removed in JSON 3. +gem "json", "< 3" gem "minitest", "~> 5.27.0" gem "minitest-rg", "~> 5.4.0" gem "pry", "~> 0.14.2" diff --git a/acceptance/cases/migration/change_schema_test.rb b/acceptance/cases/migration/change_schema_test.rb index ef73f88d..fdf6b0c7 100644 --- a/acceptance/cases/migration/change_schema_test.rb +++ b/acceptance/cases/migration/change_schema_test.rb @@ -300,7 +300,7 @@ def test_add_column_with_timestamp_type column = connection.columns(:testings).find { |c| c.name == "foo" } - assert_equal :time, column.type + assert_equal :datetime, column.type assert_equal "TIMESTAMP", column.sql_type end diff --git a/acceptance/cases/migration/schema_dumper_test.rb b/acceptance/cases/migration/schema_dumper_test.rb index 52f3ae95..88fd7030 100644 --- a/acceptance/cases/migration/schema_dumper_test.rb +++ b/acceptance/cases/migration/schema_dumper_test.rb @@ -76,7 +76,7 @@ def test_dump_schema_contains_commit_timestamp connection = pool_or_connection schema = StringIO.new ActiveRecord::SchemaDumper.dump connection, schema - assert schema.string.include?("t.time \"last_updated\", allow_commit_timestamp: true"), schema.string + assert schema.string.include?("t.datetime \"last_updated\", allow_commit_timestamp: true"), schema.string end def test_dump_schema_contains_virtual_column diff --git a/acceptance/cases/type/time_test.rb b/acceptance/cases/type/time_test.rb index 4ed86950..c22d528d 100644 --- a/acceptance/cases/type/time_test.rb +++ b/acceptance/cases/type/time_test.rb @@ -71,9 +71,9 @@ def test_date_time_with_string_value_with_non_iso_format assert_equal record, TestTypeModel.find_by(start_time: string_value) end - def test_default_year_is_correct - expected_time = ::Time.utc(2000, 1, 1, 10, 30, 0) - record = TestTypeModel.new start_time: { 4 => 10, 5 => 30 } + def test_multiparameter_assignment_preserves_date + expected_time = ::Time.utc(2026, 9, 9, 10, 30, 0) + record = TestTypeModel.new start_time: { 1 => 2026, 2 => 9, 3 => 9, 4 => 10, 5 => 30 } assert_equal expected_time, record.start_time diff --git a/lib/active_record/connection_adapters/spanner/database_statements.rb b/lib/active_record/connection_adapters/spanner/database_statements.rb index 7bc52a63..f2696bda 100644 --- a/lib/active_record/connection_adapters/spanner/database_statements.rb +++ b/lib/active_record/connection_adapters/spanner/database_statements.rb @@ -242,7 +242,7 @@ def write_query? sql end def execute_ddl statements - log [statements].flatten.join(';'), nil do + log [statements].flatten.join(";"), nil do ActiveSupport::Dependencies.interlock.permit_concurrent_loads do @connection.execute_ddl statements end diff --git a/lib/active_record/tasks/spanner_database_tasks.rb b/lib/active_record/tasks/spanner_database_tasks.rb index 3db2cabc..52635672 100644 --- a/lib/active_record/tasks/spanner_database_tasks.rb +++ b/lib/active_record/tasks/spanner_database_tasks.rb @@ -57,7 +57,7 @@ def check_current_protected_environment! db_config, migration_class current = migration_context.current_environment stored = migration_context.last_stored_environment - raise ActiveRecord::ProtectedEnvironmentError.new(stored) if migration_context.protected_environment? + raise ActiveRecord::ProtectedEnvironmentError, stored if migration_context.protected_environment? if stored && stored != current raise ActiveRecord::EnvironmentMismatchError.new(current: current, stored: stored) From 8254e9e7f70c55efa2f44bde1dbc155b36b1e34f Mon Sep 17 00:00:00 2001 From: Zach Haitz Date: Mon, 14 Sep 2026 12:07:21 +0000 Subject: [PATCH 3/5] test: guard bound SQL literal coverage on Rails 7.0 --- .../spanner_active_record_with_mock_server_test.rb | 2 ++ 1 file changed, 2 insertions(+) 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 4d24cdfa..611f4f5d 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 @@ -698,6 +698,8 @@ def test_find_singer_by_last_performance_as_non_iso_string end def test_untyped_binds_from_arel_sql_are_typed_by_ruby_class + skip "Bound SQL literals require Rails version 7.1 or higher" if ActiveRecord.version < Gem::Version.create("7.1.0") + 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) From d38aa21b756a9dd508bee347ff4716b4e72dcbf4 Mon Sep 17 00:00:00 2001 From: Zach Haitz Date: Mon, 14 Sep 2026 12:09:10 +0000 Subject: [PATCH 4/5] test: allow Rails-specific precision in timestamp schema dumps --- acceptance/cases/migration/schema_dumper_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/cases/migration/schema_dumper_test.rb b/acceptance/cases/migration/schema_dumper_test.rb index 88fd7030..d2632be5 100644 --- a/acceptance/cases/migration/schema_dumper_test.rb +++ b/acceptance/cases/migration/schema_dumper_test.rb @@ -76,7 +76,7 @@ def test_dump_schema_contains_commit_timestamp connection = pool_or_connection schema = StringIO.new ActiveRecord::SchemaDumper.dump connection, schema - assert schema.string.include?("t.datetime \"last_updated\", allow_commit_timestamp: true"), schema.string + assert_match(/t\.datetime "last_updated", .*allow_commit_timestamp: true/, schema.string) end def test_dump_schema_contains_virtual_column From 21730682fab3bb1972e8e9366be76f53fd791d45 Mon Sep 17 00:00:00 2001 From: Zach Haitz Date: Mon, 14 Sep 2026 12:09:50 +0000 Subject: [PATCH 5/5] fix: wait for sample emulator to accept requests --- .../snippets/bin/create_emulator_instance.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/examples/snippets/bin/create_emulator_instance.rb b/examples/snippets/bin/create_emulator_instance.rb index 2d927202..e6003579 100644 --- a/examples/snippets/bin/create_emulator_instance.rb +++ b/examples/snippets/bin/create_emulator_instance.rb @@ -7,10 +7,18 @@ require "google/cloud/spanner" spanner = Google::Cloud::Spanner.new project: "test-project", emulator_host: "localhost:9010" -job = spanner.create_instance "test-instance", - name: "Test Instance", - config: "emulator-config", - nodes: 1 +# Container startup completes before the emulator accepts RPCs. +deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + 30 +begin + job = spanner.create_instance "test-instance", + name: "Test Instance", + config: "emulator-config", + nodes: 1 +rescue Google::Cloud::UnavailableError + raise if Process.clock_gettime(Process::CLOCK_MONOTONIC) >= deadline + sleep 1 + retry +end job.wait_until_done! instance = spanner.instance "test-instance"