From 8327c589dfe2fa75da7825daf141c9fcd56c0ba3 Mon Sep 17 00:00:00 2001 From: B N Date: Fri, 20 Jun 2025 14:05:51 +0300 Subject: [PATCH 1/4] Fix change_column re-adding indexes without their options (#1344) --- .../connection_adapters/sqlserver/schema_statements.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index dbe4148a8..c1fd12c61 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -210,12 +210,14 @@ def change_column(table_name, column_name, type, options = {}) sql_commands << "ALTER TABLE #{quote_table_name(table_name)} ADD CONSTRAINT #{default_constraint_name(table_name, column_name)} DEFAULT #{default} FOR #{quote_column_name(column_name)}" end + sql_commands.each { |c| execute(c) } + # Add any removed indexes back indexes.each do |index| - sql_commands << "CREATE INDEX #{quote_table_name(index.name)} ON #{quote_table_name(table_name)} (#{index.columns.map { |c| quote_column_name(c) }.join(", ")})" + create_index_def = CreateIndexDefinition.new(index) + execute schema_creation.accept(create_index_def) end - sql_commands.each { |c| execute(c) } clear_cache! end From 708558f168d8ea20f2b15c48bf5fe6469f423ae9 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Mon, 23 Jun 2025 11:53:38 +0100 Subject: [PATCH 2/4] Added test case and updated changelog --- CHANGELOG.md | 1 + .../change_column_index_test_sqlserver.rb | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/cases/change_column_index_test_sqlserver.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b04046870..169c88648 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,5 +17,6 @@ - [#1320](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1320) Fix SQL statement to calculate `updated_at` when upserting. - [#1327](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1327) Fix multiple `nil` identity columns for merge insert. - [#1338](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1338) Fix `insert_all`/`upsert_all` for table names containing numbers. +- [#1345](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1345) Maintain index options during `change_column` operations. Please check [8-0-stable](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/blob/8-0-stable/CHANGELOG.md) for previous changes. diff --git a/test/cases/change_column_index_test_sqlserver.rb b/test/cases/change_column_index_test_sqlserver.rb new file mode 100644 index 000000000..dbe2aca6d --- /dev/null +++ b/test/cases/change_column_index_test_sqlserver.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "cases/helper_sqlserver" + +class ChangeColumnIndexTestSqlServer < ActiveRecord::TestCase + class CreateClientsWithUniqueIndex < ActiveRecord::Migration[8.0] + def up + create_table :clients do |t| + t.string :name, limit: 15 + end + add_index :clients, :name, unique: true + end + + def down + drop_table :clients + end + end + + class ChangeClientsNameLength < ActiveRecord::Migration[8.0] + def up + change_column :clients, :name, :string, limit: 30 + end + end + + before do + CreateClientsWithUniqueIndex.new.up + end + + after do + CreateClientsWithUniqueIndex.new.down + end + + def test_index_uniqueness_is_maintained_after_column_change + indexes = ActiveRecord::Base.connection.indexes("clients") + columns = ActiveRecord::Base.connection.columns("clients") + assert_equal columns.find { |column| column.name == "name" }.limit, 15 + assert_equal indexes.size, 1 + assert_equal indexes.first.name, "index_clients_on_name" + assert indexes.first.unique + + ChangeClientsNameLength.new.up + + indexes = ActiveRecord::Base.connection.indexes("clients") + columns = ActiveRecord::Base.connection.columns("clients") + assert_equal columns.find { |column| column.name == "name" }.limit, 30 + assert_equal indexes.size, 1 + assert_equal indexes.first.name, "index_clients_on_name" + assert indexes.first.unique + end +end From d1ed46dd19d7398d7094dfd04afbb6f577a83d5b Mon Sep 17 00:00:00 2001 From: B N Date: Mon, 23 Jun 2025 15:00:26 +0300 Subject: [PATCH 3/4] Added extra change_column with index test case --- .../change_column_index_test_sqlserver.rb | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/cases/change_column_index_test_sqlserver.rb b/test/cases/change_column_index_test_sqlserver.rb index dbe2aca6d..ed551009a 100644 --- a/test/cases/change_column_index_test_sqlserver.rb +++ b/test/cases/change_column_index_test_sqlserver.rb @@ -16,18 +16,46 @@ def down end end + class CreateBlogPostsWithMultipleIndexesOnTheSameColumn < ActiveRecord::Migration[8.0] + def up + create_table :blog_posts do |t| + t.string :title, limit: 15 + t.string :subtitle + end + add_index :blog_posts, :title, unique: true, where: "([blog_posts].[title] IS NOT NULL)", name: "custom_index_name" + add_index :blog_posts, [:title, :subtitle], unique: true + end + + def down + drop_table :blog_posts + end + end + class ChangeClientsNameLength < ActiveRecord::Migration[8.0] def up change_column :clients, :name, :string, limit: 30 end end + class ChangeBlogPostsTitleLength < ActiveRecord::Migration[8.0] + def up + change_column :blog_posts, :title, :string, limit: 30 + end + end + before do + @old_verbose = ActiveRecord::Migration.verbose + ActiveRecord::Migration.verbose = false + CreateClientsWithUniqueIndex.new.up + CreateBlogPostsWithMultipleIndexesOnTheSameColumn.new.up end after do CreateClientsWithUniqueIndex.new.down + CreateBlogPostsWithMultipleIndexesOnTheSameColumn.new.down + + ActiveRecord::Migration.verbose = @old_verbose end def test_index_uniqueness_is_maintained_after_column_change @@ -47,4 +75,36 @@ def test_index_uniqueness_is_maintained_after_column_change assert_equal indexes.first.name, "index_clients_on_name" assert indexes.first.unique end + + def test_multiple_index_options_are_maintained_after_column_change + indexes = ActiveRecord::Base.connection.indexes("blog_posts") + columns = ActiveRecord::Base.connection.columns("blog_posts") + assert_equal columns.find { |column| column.name == "title" }.limit, 15 + assert_equal indexes.size, 2 + + index_1 = indexes.find { |index| index.columns == ["title"] } + assert_equal index_1.name, "custom_index_name" + assert_equal index_1.where, "([blog_posts].[title] IS NOT NULL)" + assert index_1.unique + + index_2 = indexes.find { |index| index.columns == ["title", "subtitle"] } + assert index_2.unique + + + ChangeBlogPostsTitleLength.new.up + + + indexes = ActiveRecord::Base.connection.indexes("blog_posts") + columns = ActiveRecord::Base.connection.columns("blog_posts") + assert_equal columns.find { |column| column.name == "title" }.limit, 30 + assert_equal indexes.size, 2 + + index_1 = indexes.find { |index| index.columns == ["title"] } + assert_equal index_1.name, "custom_index_name" + assert_equal index_1.where, "([blog_posts].[title] IS NOT NULL)" + assert index_1.unique + + index_2 = indexes.find { |index| index.columns == ["title", "subtitle"] } + assert index_2.unique + end end From a7bd7bcf902a1069f191c35245099109f166dfa0 Mon Sep 17 00:00:00 2001 From: B N Date: Mon, 23 Jun 2025 15:42:25 +0300 Subject: [PATCH 4/4] Remove extra blank lines --- test/cases/change_column_index_test_sqlserver.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/cases/change_column_index_test_sqlserver.rb b/test/cases/change_column_index_test_sqlserver.rb index ed551009a..08206a961 100644 --- a/test/cases/change_column_index_test_sqlserver.rb +++ b/test/cases/change_column_index_test_sqlserver.rb @@ -90,10 +90,8 @@ def test_multiple_index_options_are_maintained_after_column_change index_2 = indexes.find { |index| index.columns == ["title", "subtitle"] } assert index_2.unique - ChangeBlogPostsTitleLength.new.up - indexes = ActiveRecord::Base.connection.indexes("blog_posts") columns = ActiveRecord::Base.connection.columns("blog_posts") assert_equal columns.find { |column| column.name == "title" }.limit, 30