Ensure correct order of COLLATE and NOT NULL in CREATE TABLE statemen…

…ts (#1145)
rails-sqlserver · Jan 8, 2024 · 7e07687 · 7e07687
1 parent c7d2eab
commit 7e07687
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## Unreleased
+
+#### Fixed
+
+- [#1145](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1145) Ensure correct order of COLLATE and NOT NULL in CREATE TABLE statements
+
 ## v7.1.0
 
 #### Added

diff --git a/lib/active_record/connection_adapters/sqlserver/schema_creation.rb b/lib/active_record/connection_adapters/sqlserver/schema_creation.rb
@@ -51,12 +51,12 @@ def visit_CreateIndexDefinition(o)
 
         def add_column_options!(sql, options)
           sql << " DEFAULT #{quote_default_expression(options[:default], options[:column])}" if options_include_default?(options)
-          if options[:null] == false
-            sql << " NOT NULL"
-          end
           if options[:collation].present?
             sql << " COLLATE #{options[:collation]}"
           end
+          if options[:null] == false
+            sql << " NOT NULL"
+          end
           if options[:is_identity] == true
             sql << " IDENTITY(1,1)"
           end

diff --git a/test/cases/active_schema_test_sqlserver.rb b/test/cases/active_schema_test_sqlserver.rb
@@ -3,43 +3,44 @@
 require "cases/helper_sqlserver"
 
 class ActiveSchemaTestSQLServer < ActiveRecord::TestCase
-  before do
-    connection.create_table :schema_test_table, force: true, id: false do |t|
-      t.column :foo, :string, limit: 100
-      t.column :state, :string
+  describe "indexes" do
+
+    before do
+      connection.create_table :schema_test_table, force: true, id: false do |t|
+        t.column :foo, :string, limit: 100
+        t.column :state, :string
+      end
     end
-  end
 
-  after do
-    connection.drop_table :schema_test_table rescue nil
-  end
+    after do
+      connection.drop_table :schema_test_table rescue nil
+    end
 
-  it 'default index' do
-    assert_sql('CREATE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo])') do
-      connection.add_index :schema_test_table, "foo"
+    it 'default index' do
+      assert_sql('CREATE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo])') do
+        connection.add_index :schema_test_table, "foo"
+      end
     end
-  end
 
-  it 'unique index' do
-    assert_sql('CREATE UNIQUE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo])') do
-      connection.add_index :schema_test_table, "foo", unique: true
+    it 'unique index' do
+      assert_sql('CREATE UNIQUE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo])') do
+        connection.add_index :schema_test_table, "foo", unique: true
+      end
     end
-  end
 
-  it 'where condition on index' do
-    assert_sql("CREATE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo]) WHERE state = 'active'") do
-      connection.add_index :schema_test_table, "foo", where: "state = 'active'"
+    it 'where condition on index' do
+      assert_sql("CREATE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo]) WHERE state = 'active'") do
+        connection.add_index :schema_test_table, "foo", where: "state = 'active'"
+      end
     end
-  end
 
-  it 'if index does not exist' do
-    assert_sql("IF NOT EXISTS (SELECT name FROM sysindexes WHERE name = 'index_schema_test_table_on_foo') " \
-               "CREATE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo])") do
-      connection.add_index :schema_test_table, "foo", if_not_exists: true
+    it 'if index does not exist' do
+      assert_sql("IF NOT EXISTS (SELECT name FROM sysindexes WHERE name = 'index_schema_test_table_on_foo') " \
+                 "CREATE INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo])") do
+        connection.add_index :schema_test_table, "foo", if_not_exists: true
+      end
     end
-  end
 
-  describe "index types" do
     it 'clustered index' do
       assert_sql('CREATE CLUSTERED INDEX [index_schema_test_table_on_foo] ON [schema_test_table] ([foo])') do
         connection.add_index :schema_test_table, "foo", type: :clustered
@@ -52,4 +53,14 @@ class ActiveSchemaTestSQLServer < ActiveRecord::TestCase
       end
     end
   end
+
+  it "create column with NOT NULL and COLLATE" do
+    assert_nothing_raised do
+      connection.create_table :not_null_with_collation_table, force: true, id: false do |t|
+        t.text :not_null_text_with_collation, null: false, collation: "Latin1_General_CS_AS"
+      end
+    end
+  ensure
+    connection.drop_table :not_null_with_collation_table rescue nil
+  end
 end