From ee9b04239c12ef09b3c3ee7154e0b52f3e2525ca Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Fri, 8 Aug 2025 11:12:47 +0100 Subject: [PATCH 1/5] Added test case Fix sql Use database when constructing object ID argument Wrap insert in transaction Update temp_test_sqlserver.rb Update temp_test_sqlserver.rb Update temp_test_sqlserver.rb Update temp_test_sqlserver.rb Timeout CI tests after 10 minutes Update temp_test_sqlserver.rb Verbose Debug Update temp_test_sqlserver.rb Debug Update temp_test_sqlserver.rb Debug Cleanup Update adapter_test_sqlserver.rb Update adapter_test_sqlserver.rb Update adapter_test_sqlserver.rb Update adapter_test_sqlserver.rb --- .github/workflows/ci.yml | 1 + .../sqlserver/schema_statements.rb | 9 +++---- test/cases/adapter_test_sqlserver.rb | 25 +++++++++++++++++++ test/cases/temp_test_sqlserver.rb | 11 ++++++++ 4 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 test/cases/temp_test_sqlserver.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 984a6268d..6ff557351 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,7 @@ jobs: test: name: Run test suite runs-on: ubuntu-latest + timeout-minutes: 10 env: COMPOSE_FILE: docker-compose.ci.yml diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index 828dcbff2..6ccf31755 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -571,6 +571,7 @@ def column_definitions(table_name) end def column_definitions_sql(database, identifier) + database = "TEMPDB" if identifier.temporary_table? schema_name = "schema_name()" if prepared_statements @@ -581,12 +582,8 @@ def column_definitions_sql(database, identifier) schema_name = quote(identifier.schema) if identifier.schema.present? end - object_id_arg = identifier.schema.present? ? "CONCAT(#{schema_name},'.',#{object_name})" : object_name - - if identifier.temporary_table? - database = "TEMPDB" - object_id_arg = "CONCAT('#{database}','..',#{object_name})" - end + object_id_arg = identifier.schema.present? ? "CONCAT('.',#{schema_name},'.',#{object_name})" : "CONCAT('..',#{object_name})" + object_id_arg = "CONCAT('#{database}',#{object_id_arg})" %{ SELECT diff --git a/test/cases/adapter_test_sqlserver.rb b/test/cases/adapter_test_sqlserver.rb index 7b9f0eca5..dcab1dcfe 100644 --- a/test/cases/adapter_test_sqlserver.rb +++ b/test/cases/adapter_test_sqlserver.rb @@ -233,6 +233,31 @@ class AdapterTestSQLServer < ActiveRecord::TestCase it "return an empty array when calling #identity_columns for a table_name with no identity" do _(connection.send(:identity_columns, Subscriber.table_name)).must_equal [] end + + it "identity insert enabled for cross database insert" do + arunit_connection = Dog.lease_connection + arunit2_connection = OtherDog.lease_connection + + arunit_database = arunit_connection.pool.db_config.database + arunit2_database = arunit2_connection.pool.db_config.database + + sql = <<~SQL + INSERT INTO #{arunit2_database}.dbo.dogs(id) SELECT id FROM #{arunit_database}.dbo.dogs + SQL + + OtherDog.destroy_all + + # + # assert Dog.count, 1 + # assert OtherDog.count, 0 + + assert_nothing_raised do + arunit_connection.execute(sql) + end + + # assert Dog.count, 1 + # assert OtherDog.count, 1 + end end describe "quoting" do diff --git a/test/cases/temp_test_sqlserver.rb b/test/cases/temp_test_sqlserver.rb new file mode 100644 index 000000000..748fcea11 --- /dev/null +++ b/test/cases/temp_test_sqlserver.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require "cases/helper_sqlserver" +require "models/dog" +require "models/other_dog" + +class TempTestSQLServer < ActiveRecord::TestCase + # it "assert true" do + # assert true + # end +end From a24c721056b3df87226dcbeb32b88014f55b9c3e Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 14 Aug 2025 11:10:43 +0100 Subject: [PATCH 2/5] Debug --- test/cases/adapter_test_sqlserver.rb | 5 +++-- test/cases/temp_test_sqlserver.rb | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/test/cases/adapter_test_sqlserver.rb b/test/cases/adapter_test_sqlserver.rb index dcab1dcfe..6587bb10d 100644 --- a/test/cases/adapter_test_sqlserver.rb +++ b/test/cases/adapter_test_sqlserver.rb @@ -246,13 +246,14 @@ class AdapterTestSQLServer < ActiveRecord::TestCase SQL OtherDog.destroy_all - + # # assert Dog.count, 1 # assert OtherDog.count, 0 assert_nothing_raised do - arunit_connection.execute(sql) + # TODO: Check if this is causing issues with the test suite. + # arunit_connection.execute(sql) end # assert Dog.count, 1 diff --git a/test/cases/temp_test_sqlserver.rb b/test/cases/temp_test_sqlserver.rb index 748fcea11..c9fae9490 100644 --- a/test/cases/temp_test_sqlserver.rb +++ b/test/cases/temp_test_sqlserver.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true require "cases/helper_sqlserver" -require "models/dog" -require "models/other_dog" class TempTestSQLServer < ActiveRecord::TestCase # it "assert true" do From de40ef69973c2fcab7d8a16fad3681e3e7dbf54e Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 14 Aug 2025 12:12:56 +0100 Subject: [PATCH 3/5] Update adapter_test_sqlserver.rb --- test/cases/adapter_test_sqlserver.rb | 56 ++++++++++++---------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/test/cases/adapter_test_sqlserver.rb b/test/cases/adapter_test_sqlserver.rb index 6587bb10d..0de8bf272 100644 --- a/test/cases/adapter_test_sqlserver.rb +++ b/test/cases/adapter_test_sqlserver.rb @@ -7,10 +7,17 @@ require "models/subscriber" require "models/minimalistic" require "models/college" +require "models/dog" +require "models/other_dog" class AdapterTestSQLServer < ActiveRecord::TestCase fixtures :tasks + let(:arunit_connection) { Topic.lease_connection } + let(:arunit2_connection) { College.lease_connection } + let(:arunit_database) { arunit_connection.pool.db_config.database } + let(:arunit2_database) { arunit2_connection.pool.db_config.database } + let(:basic_insert_sql) { "INSERT INTO [funny_jokes] ([name]) VALUES('Knock knock')" } let(:basic_update_sql) { "UPDATE [customers] SET [address_street] = NULL WHERE [id] = 2" } let(:basic_select_sql) { "SELECT * FROM [customers] WHERE ([customers].[id] = 1)" } @@ -50,8 +57,7 @@ class AdapterTestSQLServer < ActiveRecord::TestCase assert Topic.table_exists?, "Topics table name of 'dbo.topics' should return true for exists." # Test when database and owner included in table name. - db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary") - Topic.table_name = "#{db_config.database}.dbo.topics" + Topic.table_name = "#{arunit_database}.dbo.topics" assert Topic.table_exists?, "Topics table name of '[DATABASE].dbo.topics' should return true for exists." ensure Topic.table_name = "topics" @@ -59,12 +65,6 @@ class AdapterTestSQLServer < ActiveRecord::TestCase end it "test table existence across database schemas" do - arunit_connection = Topic.lease_connection - arunit2_connection = College.lease_connection - - arunit_database = arunit_connection.pool.db_config.database - arunit2_database = arunit2_connection.pool.db_config.database - # Assert that connections use different default databases schemas. assert_not_equal arunit_database, arunit2_database @@ -200,6 +200,8 @@ class AdapterTestSQLServer < ActiveRecord::TestCase @identity_insert_sql_non_dbo_sp = "EXEC sp_executesql N'INSERT INTO [test].[aliens] ([id],[name]) VALUES (@0, @1)', N'@0 int, @1 nvarchar(255)', @0 = 420, @1 = N'Mork'" @identity_insert_sql_non_dbo_unquoted_sp = "EXEC sp_executesql N'INSERT INTO test.aliens (id, name) VALUES (@0, @1)', N'@0 int, @1 nvarchar(255)', @0 = 420, @1 = N'Mork'" @identity_insert_sql_non_dbo_unordered_sp = "EXEC sp_executesql N'INSERT INTO [test].[aliens] ([name],[id]) VALUES (@0, @1)', N'@0 nvarchar(255), @1 int', @0 = N'Mork', @1 = 420" + + @identity_insert_sql_cross_database = "INSERT INTO #{arunit2_database}.dbo.dogs(id) SELECT id FROM #{arunit_database}.dbo.dogs" end it "return quoted table_name to #query_requires_identity_insert? when INSERT sql contains id column" do @@ -216,6 +218,8 @@ class AdapterTestSQLServer < ActiveRecord::TestCase assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_sp) assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unquoted_sp) assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unordered_sp) + + assert_equal "[#{arunit2_database}].[dbo].[dogs]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_cross_database) end it "return false to #query_requires_identity_insert? for normal SQL" do @@ -224,40 +228,26 @@ class AdapterTestSQLServer < ActiveRecord::TestCase end end - it "find identity column using #identity_columns" do + it "find identity column" do task_id_column = Task.columns_hash["id"] assert_equal task_id_column.name, connection.send(:identity_columns, Task.table_name).first.name assert_equal task_id_column.sql_type, connection.send(:identity_columns, Task.table_name).first.sql_type end - it "return an empty array when calling #identity_columns for a table_name with no identity" do - _(connection.send(:identity_columns, Subscriber.table_name)).must_equal [] - end - - it "identity insert enabled for cross database insert" do - arunit_connection = Dog.lease_connection - arunit2_connection = OtherDog.lease_connection - - arunit_database = arunit_connection.pool.db_config.database - arunit2_database = arunit2_connection.pool.db_config.database + it "find identity column cross database" do + id_column = Dog.columns_hash["id"] + assert_equal id_column.name, arunit2_connection.send(:identity_columns, Dog.table_name).first.name + assert_equal id_column.sql_type, arunit2_connection.send(:identity_columns, Dog.table_name).first.sql_type - sql = <<~SQL - INSERT INTO #{arunit2_database}.dbo.dogs(id) SELECT id FROM #{arunit_database}.dbo.dogs - SQL + id_column = OtherDog.columns_hash["id"] + assert_equal id_column.name, arunit_connection.send(:identity_columns, OtherDog.table_name).first.name + assert_equal id_column.sql_type, arunit_connection.send(:identity_columns, OtherDog.table_name).first.sql_type + end - OtherDog.destroy_all - # - # assert Dog.count, 1 - # assert OtherDog.count, 0 - assert_nothing_raised do - # TODO: Check if this is causing issues with the test suite. - # arunit_connection.execute(sql) - end - - # assert Dog.count, 1 - # assert OtherDog.count, 1 + it "return an empty array when calling #identity_columns for a table_name with no identity" do + _(connection.send(:identity_columns, Subscriber.table_name)).must_equal [] end end From b7a0fc14dd4664ca7c1afa54eb5f0b9b573178a1 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 14 Aug 2025 12:47:38 +0100 Subject: [PATCH 4/5] Update adapter_test_sqlserver.rb --- test/cases/adapter_test_sqlserver.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/cases/adapter_test_sqlserver.rb b/test/cases/adapter_test_sqlserver.rb index 0de8bf272..21b5fa340 100644 --- a/test/cases/adapter_test_sqlserver.rb +++ b/test/cases/adapter_test_sqlserver.rb @@ -201,6 +201,7 @@ class AdapterTestSQLServer < ActiveRecord::TestCase @identity_insert_sql_non_dbo_unquoted_sp = "EXEC sp_executesql N'INSERT INTO test.aliens (id, name) VALUES (@0, @1)', N'@0 int, @1 nvarchar(255)', @0 = 420, @1 = N'Mork'" @identity_insert_sql_non_dbo_unordered_sp = "EXEC sp_executesql N'INSERT INTO [test].[aliens] ([name],[id]) VALUES (@0, @1)', N'@0 nvarchar(255), @1 int', @0 = N'Mork', @1 = 420" + @non_identity_insert_sql_cross_database = "INSERT INTO #{arunit2_database}.dbo.dogs SELECT * FROM #{arunit_database}.dbo.dogs" @identity_insert_sql_cross_database = "INSERT INTO #{arunit2_database}.dbo.dogs(id) SELECT id FROM #{arunit_database}.dbo.dogs" end @@ -223,7 +224,7 @@ class AdapterTestSQLServer < ActiveRecord::TestCase end it "return false to #query_requires_identity_insert? for normal SQL" do - [basic_insert_sql, basic_update_sql, basic_select_sql].each do |sql| + [basic_insert_sql, basic_update_sql, basic_select_sql, @non_identity_insert_sql_cross_database].each do |sql| assert !connection.send(:query_requires_identity_insert?, sql), "SQL was #{sql}" end end @@ -244,8 +245,6 @@ class AdapterTestSQLServer < ActiveRecord::TestCase assert_equal id_column.sql_type, arunit_connection.send(:identity_columns, OtherDog.table_name).first.sql_type end - - it "return an empty array when calling #identity_columns for a table_name with no identity" do _(connection.send(:identity_columns, Subscriber.table_name)).must_equal [] end From fe138c03f64bfd3a2836cbcbcba89a4ec2d5de07 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 14 Aug 2025 13:08:33 +0100 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44e244a01..299be0ad4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ - [#1341](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1341) Support more Azure services by changing language source. +#### Fixed + +- [#1357(https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1357) Support cross database inserts. + ## v7.2.6 #### Fixed