Merge pull request #23622 from kamipo/primary_key_should_be_not_null

Primary key should be `NOT NULL`

activerecord/CHANGELOG.md

+*   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:

activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb

@@ -16,7 +16,7 @@ def column_spec(column)
       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

activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb

@@ -7,7 +7,7 @@ def column_spec_for_primary_key(column)
             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

activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb

@@ -3,7 +3,7 @@ module ConnectionAdapters
     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

activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb

@@ -3,6 +3,13 @@ module ConnectionAdapters
     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]}\""

activerecord/test/cases/primary_keys_test.rb

@@ -229,7 +229,7 @@ def test_any_type_primary_key
     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