From a596a6ee825815e6e6ed6cefccf94bd5d594c63f Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Tue, 3 Oct 2023 13:42:20 +0100 Subject: [PATCH] Fix/coerce compatibility tests (#1104) --- .../sqlserver/schema_statements.rb | 9 ++- test/cases/coerced_tests.rb | 69 ++++++++++++++++++- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index 4c255f0cf..2ab28ef19 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -310,15 +310,18 @@ def update_table_definition(table_name, base) SQLServer::Table.new(table_name, base) end - def change_column_null(table_name, column_name, allow_null, default = nil) + def change_column_null(table_name, column_name, null, default = nil) + validate_change_column_null_argument!(null) + table_id = SQLServer::Utils.extract_identifiers(table_name) column_id = SQLServer::Utils.extract_identifiers(column_name) column = column_for(table_name, column_name) - if !allow_null.nil? && allow_null == false && !default.nil? + if !null.nil? && null == false && !default.nil? execute("UPDATE #{table_id} SET #{column_id}=#{quote(default)} WHERE #{column_id} IS NULL") end sql = "ALTER TABLE #{table_id} ALTER COLUMN #{column_id} #{type_to_sql column.type, limit: column.limit, precision: column.precision, scale: column.scale}" - sql += " NOT NULL" if !allow_null.nil? && allow_null == false + sql += " NOT NULL" if !null.nil? && null == false + execute sql end diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index 542014a35..22318d234 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -680,10 +680,77 @@ def migrate(x) error = assert_raises(StandardError) do ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate end - assert_match(/The identifier that starts with '#{long_table_name[0..-2]}' is too long/i, error.message) + assert_match(/The identifier that starts with '#{long_table_name[0...-1]}' is too long/i, error.message) ensure connection.drop_table(long_table_name) rescue nil end + + # Error message depends on the database adapter. + coerce_tests! :test_rename_table_on_7_0 + def test_rename_table_on_7_0_coerced + long_table_name = "a" * (connection.table_name_length + 1) + connection.create_table(:more_testings) + + migration = Class.new(ActiveRecord::Migration[7.0]) { + @@long_table_name = long_table_name + def version; 100 end + def migrate(x) + rename_table :more_testings, @@long_table_name + end + }.new + + error = assert_raises(StandardError) do + ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate + end + assert_match(/The new name '#{long_table_name[0...-1]}' is already in use as a object name and would cause a duplicate that is not permitted/i, error.message) + ensure + connection.drop_table(:more_testings) rescue nil + connection.drop_table(long_table_name) rescue nil + end + + # SQL Server has a different maximum index name length. + coerce_tests! :test_add_index_errors_on_too_long_name_7_0 + def test_add_index_errors_on_too_long_name_7_0_coerced + long_index_name = 'a' * (connection.index_name_length + 1) + + migration = Class.new(ActiveRecord::Migration[7.0]) { + @@long_index_name = long_index_name + def migrate(x) + add_column :testings, :very_long_column_name_to_test_with, :string + add_index :testings, [:foo, :bar, :very_long_column_name_to_test_with], name: @@long_index_name + end + }.new + + error = assert_raises(StandardError) do + ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate + end + assert_match(/Index name \'#{long_index_name}\' on table \'testings\' is too long/i, error.message) + end + + # SQL Server has a different maximum index name length. + coerce_tests! :test_create_table_add_index_errors_on_too_long_name_7_0 + def test_create_table_add_index_errors_on_too_long_name_7_0_coerced + long_index_name = 'a' * (connection.index_name_length + 1) + + migration = Class.new(ActiveRecord::Migration[7.0]) { + @@long_index_name = long_index_name + def migrate(x) + create_table :more_testings do |t| + t.integer :foo + t.integer :bar + t.integer :very_long_column_name_to_test_with + t.index [:foo, :bar, :very_long_column_name_to_test_with], name: @@long_index_name + end + end + }.new + + error = assert_raises(StandardError) do + ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate + end + assert_match(/Index name \'#{long_index_name}\' on table \'more_testings\' is too long/i, error.message) + ensure + connection.drop_table :more_testings rescue nil + end end end end