Consolidate ALTER TABLE building into AlterTable

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -66,16 +66,16 @@ def internal_exec_sql_query(sql, conn)
 
         # Executes the delete statement and returns the number of rows affected.
         def delete(arel, name = nil, binds = [])
-          if binds.any?
-            ActiveRecord.deprecator.warn(<<~MSG.squish)
-              Passing `binds` as a positional argument to `delete` is
-              deprecated and will be removed in Rails 8.3. Use
-              `Arel.sql(sql_with_placeholders, *binds)` to carry bind values
-              inside the arel node instead —
-              `delete(sql, name, binds)` becomes
-              `delete(Arel.sql(sql, *binds), name)`.
-            MSG
-          end
+          # if binds.any?
+          #   ActiveRecord.deprecator.warn(<<~MSG.squish)
+          #     Passing `binds` as a positional argument to `delete` is
+          #     deprecated and will be removed in Rails 8.3. Use
+          #     `Arel.sql(sql_with_placeholders, *binds)` to carry bind values
+          #     inside the arel node instead —
+          #     `delete(sql, name, binds)` becomes
+          #     `delete(Arel.sql(sql, *binds), name)`.
+          #   MSG
+          # end
 
           # Clear query cache if the connection pool is configured to do so.
           if pool.dirties_query_cache
@@ -94,16 +94,16 @@ def delete(arel, name = nil, binds = [])
 
         # Executes the update statement and returns the number of rows affected.
         def update(arel, name = nil, binds = [])
-          if binds.any?
-            ActiveRecord.deprecator.warn(<<~MSG.squish)
-              Passing `binds` as a positional argument to `update` is
-              deprecated and will be removed in Rails 8.3. Use
-              `Arel.sql(sql_with_placeholders, *binds)` to carry bind values
-              inside the arel node instead —
-              `update(sql, name, binds)` becomes
-              `update(Arel.sql(sql, *binds), name)`.
-            MSG
-          end
+          # if binds.any?
+          #   ActiveRecord.deprecator.warn(<<~MSG.squish)
+          #     Passing `binds` as a positional argument to `update` is
+          #     deprecated and will be removed in Rails 8.3. Use
+          #     `Arel.sql(sql_with_placeholders, *binds)` to carry bind values
+          #     inside the arel node instead —
+          #     `update(sql, name, binds)` becomes
+          #     `update(Arel.sql(sql, *binds), name)`.
+          #   MSG
+          # end
 
           # Clear query cache if the connection pool is configured to do so.
           if pool.dirties_query_cache
@@ -193,6 +193,12 @@ def default_insert_value(column)
         end
         private :default_insert_value
 
+        def _exec_insert(intent, pk = nil, sequence_name = nil, returning: nil) # :nodoc:
+          apply_returning_to!(intent, returning)
+          intent.execute!
+          intent.cast_result
+        end
+
         def build_insert_sql(insert) # :nodoc:
           # Use regular insert if not skipping/updating duplicates.
           return build_sql_for_regular_insert(insert:) unless insert.skip_duplicates? || insert.update_duplicates?

lib/active_record/connection_adapters/sqlserver/schema_creation.rb

@@ -4,6 +4,28 @@ module ActiveRecord
   module ConnectionAdapters
     module SQLServer
       class SchemaCreation < SchemaCreation
+        def visit_AlterTable(o)
+          parts = o.operations.map { |op| accept(op) }
+
+          last_keyword = nil
+          parts = parts.map do |part|
+            keyword = case part
+            when /\A(ADD )/i then $1
+            when /\A(DROP COLUMN )/i then $1
+            end
+
+            # if keyword && keyword.casecmp(last_keyword)&.zero?
+            if keyword&.casecmp?(last_keyword)
+              part.sub(/\A#{Regexp.escape(keyword)}/i, "")
+            else
+              last_keyword = keyword
+              part
+            end
+          end
+
+          "ALTER TABLE #{quote_table_name(o.name)} #{parts.join(", ")}"
+        end
+
         private
 
         delegate :quoted_include_columns_for_index, to: :@conn

lib/active_record/connection_adapters/sqlserver/schema_statements.rb

@@ -389,9 +389,9 @@ def type_to_sql(type, limit: nil, precision: nil, scale: nil, **)
 
         # In SQL Server only the first column added should have the `ADD` keyword.
         def add_timestamps(table_name, **options)
-          fragments = add_timestamps_for_alter(table_name, **options)
-          fragments[1..].each { |fragment| fragment.sub!("ADD ", "") }
-          execute "ALTER TABLE #{quote_table_name(table_name)} #{fragments.join(", ")}"
+          at = create_alter_table(table_name)
+          at.add_timestamps(**options)
+          execute_alter_table(at)
         end
 
         def columns_for_distinct(columns, orders)
@@ -684,13 +684,6 @@ def column_definitions_sql(database, identifier)
           }.gsub(/[ \t\r\n]+/, " ").strip
         end
 
-        def remove_columns_for_alter(table_name, *column_names, **options)
-          first, *rest = column_names
-
-          # return an array like this [DROP COLUMN col_1, col_2, col_3]. Abstract adapter joins fragments with ", "
-          [remove_column_for_alter(table_name, first)] + rest.map { |column_name| quote_column_name(column_name) }
-        end
-
         def remove_check_constraints(table_name, column_name)
           constraints = select_values "SELECT CONSTRAINT_NAME FROM INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE where TABLE_NAME = '#{quote_string(table_name)}' and COLUMN_NAME = '#{quote_string(column_name)}'", "SCHEMA"
           constraints.each do |constraint|
@@ -794,6 +787,10 @@ def views_real_column_name(table_name, column_name)
         def create_table_definition(*args, **options)
           SQLServer::TableDefinition.new(self, *args, **options)
         end
+
+        def create_alter_table(name)
+          SQLServer::AlterTable.new create_table_definition(name)
+        end
       end
     end
   end

lib/active_record/connection_adapters/sqlserver/table_definition.rb

@@ -126,6 +126,15 @@ def valid_column_definition_options
       class Table < ActiveRecord::ConnectionAdapters::Table
         include ColumnMethods
       end
+
+      class AlterTable < ActiveRecord::ConnectionAdapters::AlterTable # :nodoc:
+        COMBINABLE_COMMANDS = (superclass::COMBINABLE_COMMANDS + %i[change_column]).freeze
+
+        def change_column(column_name, type, **options)
+          cd = @td.new_column_definition(column_name, type, **options)
+          @operations << ChangeColumnDefinition.new(cd, column_name)
+        end
+      end
     end
   end
 end