From c4c99208e27e8463213ac73db69dc98cac292474 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Sat, 29 Aug 2026 11:01:34 +0100 Subject: [PATCH] Consolidate ALTER TABLE building into AlterTable Ref: https://github.com/rails/rails/commit/39e3525f64d5d00046026c3d0ec875c6087d57bf --- .../sqlserver/database_statements.rb | 46 +++++++++++-------- .../sqlserver/schema_creation.rb | 22 +++++++++ .../sqlserver/schema_statements.rb | 17 +++---- .../sqlserver/table_definition.rb | 9 ++++ 4 files changed, 64 insertions(+), 30 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 19742dae7..991f25508 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/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? diff --git a/lib/active_record/connection_adapters/sqlserver/schema_creation.rb b/lib/active_record/connection_adapters/sqlserver/schema_creation.rb index c7661a47d..3c116b075 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_creation.rb +++ b/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 diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index d908d3e34..d2dcdc673 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/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 diff --git a/lib/active_record/connection_adapters/sqlserver/table_definition.rb b/lib/active_record/connection_adapters/sqlserver/table_definition.rb index e68a17785..4ce243b0a 100644 --- a/lib/active_record/connection_adapters/sqlserver/table_definition.rb +++ b/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