Merge pull request #23622 from kamipo/primary_key_should_be_not_null

Primary key should be `NOT NULL`

6 files changed, 28 insertions, 4 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 9e51fb0ca3..c177f1bc8c 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,20 @@
+*   SQLite: Force NOT NULL primary keys.
+
+    From SQLite docs: https://www.sqlite.org/lang_createtable.html
+        According to the SQL standard, PRIMARY KEY should always imply NOT
+        NULL. Unfortunately, due to a bug in some early versions, this is not
+        the case in SQLite. Unless the column is an INTEGER PRIMARY KEY or the
+        table is a WITHOUT ROWID table or the column is declared NOT NULL,
+        SQLite allows NULL values in a PRIMARY KEY column. SQLite could be
+        fixed to conform to the standard, but doing so might break legacy
+        applications. Hence, it has been decided to merely document the fact
+        that SQLite allowing NULLs in most PRIMARY KEY columns.
+
+    Now we override column options to explicitly set NOT NULL rather than rely
+    on implicit NOT NULL like MySQL and PostgreSQL adapters.
+
+    *Ryuta Kamizono*
+
 *   Added notice when a database is successfully created or dropped.
 
     Example:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
index 69d2780f7c..6b5fad00ab 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
@@ -16,7 +16,7 @@ module ActiveRecord
       def column_spec_for_primary_key(column)
         return {} if default_primary_key?(column)
         spec = { id: schema_type(column).inspect }
-        spec.merge!(prepare_column_options(column))
+        spec.merge!(prepare_column_options(column).except!(:null))
       end
 
       # This can be overridden on an Adapter level basis to support other
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb
index be40df4101..2ba9657f24 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb
@@ -7,7 +7,7 @@ module ActiveRecord
             spec = { id: :bigint.inspect }
             spec[:default] = schema_default(column) || 'nil' unless column.auto_increment?
           else
-            spec = super.except!(:null)
+            spec = super
           end
           spec[:unsigned] = 'true' if column.unsigned?
           spec
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb
index 1047ba8cac..a1e10fd364 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb
@@ -3,7 +3,7 @@ module ActiveRecord
     module PostgreSQL
       module ColumnDumper
         def column_spec_for_primary_key(column)
-          spec = super.except!(:null)
+          spec = super
           if schema_type(column) == :uuid
             spec[:default] ||= 'nil'
           end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb
index fe1dcbd710..70c0d28830 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb
@@ -3,6 +3,13 @@ module ActiveRecord
     module SQLite3
       class SchemaCreation < AbstractAdapter::SchemaCreation
         private
+
+          def column_options(o)
+            options = super
+            options[:null] = false if o.primary_key
+            options
+          end
+
           def add_column_options!(sql, options)
             if options[:collation]
               sql << " COLLATE \"#{options[:collation]}\""
diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb
index 32bccce2ed..52eac4a124 100644
--- a/activerecord/test/cases/primary_keys_test.rb
+++ b/activerecord/test/cases/primary_keys_test.rb
@@ -229,7 +229,7 @@ class PrimaryKeyAnyTypeTest < ActiveRecord::TestCase
     assert_equal "code", Barcode.primary_key
 
     column = Barcode.column_for_attribute(Barcode.primary_key)
-    assert_not column.null unless current_adapter?(:SQLite3Adapter)
+    assert_not column.null
     assert_equal :string, column.type
     assert_equal 42, column.limit
   end