Merge pull request rails#54962 from Shopify/insert-all-default-primary-key

Set default for primary keys in `insert_all`/`upsert_all`

Author: eileencodes

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Set default for primary keys in `insert_all`/`upsert_all`.
+
+    Previously in Postgres, updating and inserting new records in one upsert wasn't possible
+    due to null primary key values. `nil` primary key values passed into `insert_all`/`upsert_all`
+    are now implicitly set to the default insert value specified by adapter.
+
+    *Jenny Shen*
+
 *   Add a load hook `active_record_database_configurations` for `ActiveRecord::DatabaseConfigurations`
 
     *Mike Dalessio*

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

@@ -550,7 +550,14 @@ def internal_exec_query(...) # :nodoc:
         cast_result(internal_execute(...))
       end
 
+      def default_insert_value(column) # :nodoc:
+        DEFAULT_INSERT_VALUE
+      end
+
       private
+        DEFAULT_INSERT_VALUE = Arel.sql("DEFAULT").freeze
+        private_constant :DEFAULT_INSERT_VALUE
+
         # Lowest level way to execute a query. Doesn't check for illegal writes, doesn't annotate queries, yields a native result object.
         def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true, batch: false)
           type_casted_binds = type_casted_binds(binds)
@@ -607,13 +614,6 @@ def execute_batch(statements, name = nil, **kwargs)
           end
         end
 
-        DEFAULT_INSERT_VALUE = Arel.sql("DEFAULT").freeze
-        private_constant :DEFAULT_INSERT_VALUE
-
-        def default_insert_value(column)
-          DEFAULT_INSERT_VALUE
-        end
-
         def build_fixture_sql(fixtures, table_name)
           columns = schema_cache.columns_hash(table_name).reject { |_, column| supports_virtual_columns? && column.virtual? }
 

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

@@ -45,16 +45,16 @@ def build_explain_clause(options = [])
           end
         end
 
+        def default_insert_value(column) # :nodoc:
+          super unless column.auto_increment?
+        end
+
         private
           # https://mariadb.com/kb/en/analyze-statement/
           def analyze_without_explain?
             mariadb? && database_version >= "10.1.0"
           end
 
-          def default_insert_value(column)
-            super unless column.auto_increment?
-          end
-
           def returning_column_values(result)
             if supports_insert_returning?
               result.rows.first

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

@@ -61,6 +61,14 @@ def reset_isolation_level # :nodoc:
           @previous_read_uncommitted = nil
         end
 
+        def default_insert_value(column) # :nodoc:
+          if column.default_function
+            Arel.sql(column.default_function)
+          else
+            column.default
+          end
+        end
+
         private
           def internal_begin_transaction(mode, isolation)
             if isolation
@@ -136,14 +144,6 @@ def build_truncate_statement(table_name)
           def returning_column_values(result)
             result.rows.first
           end
-
-          def default_insert_value(column)
-            if column.default_function
-              Arel.sql(column.default_function)
-            else
-              column.default
-            end
-          end
       end
     end
   end

activerecord/lib/active_record/insert_all.rb

@@ -225,7 +225,7 @@ def timestamps_for_create
       class Builder # :nodoc:
         attr_reader :model
 
-        delegate :skip_duplicates?, :update_duplicates?, :keys, :keys_including_timestamps, :record_timestamps?, to: :insert_all
+        delegate :skip_duplicates?, :update_duplicates?, :keys, :keys_including_timestamps, :record_timestamps?, :primary_keys, to: :insert_all
 
         def initialize(insert_all)
           @insert_all, @model, @connection = insert_all, insert_all.model, insert_all.connection
@@ -239,8 +239,13 @@ def values_list
           types = extract_types_from_columns_on(model.table_name, keys: keys_including_timestamps)
 
           values_list = insert_all.map_key_with_value do |key, value|
-            next value if Arel::Nodes::SqlLiteral === value
-            ActiveModel::Type::SerializeCastValue.serialize(type = types[key], type.cast(value))
+            if Arel::Nodes::SqlLiteral === value
+              value
+            elsif primary_keys.include?(key) && value.nil?
+              connection.default_insert_value(column_from_key(key))
+            else
+              ActiveModel::Type::SerializeCastValue.serialize(type = types[key], type.cast(value))
+            end
           end
 
           connection.visitor.compile(Arel::Nodes::ValuesList.new(values_list))
@@ -323,6 +328,10 @@ def quote_columns(columns)
           def quote_column(column)
             connection.quote_column_name(column)
           end
+
+          def column_from_key(key)
+            model.schema_cache.columns_hash(model.table_name)[key]
+          end
       end
   end
 end

activerecord/test/cases/insert_all_test.rb

@@ -409,6 +409,24 @@ def test_upsert_all_updates_existing_record_by_primary_key
     assert_equal "New edition", Book.find(1).name
   end
 
+  def test_upsert_all_implicitly_sets_primary_keys_when_nil
+    assert_difference "Book.count", 2 do
+      Book.upsert_all [
+        { id: 1, name: "New edition" },
+        { id: nil, name: "New edition 2" },
+        { id: nil, name: "New edition 3" },
+      ]
+    end
+
+    assert_equal "New edition", Book.find(1).name
+  end
+
+  def test_insert_all_implicitly_sets_primary_keys_when_nil
+    assert_difference "Book.count", 1 do
+      Book.insert_all [ { id: nil, name: "New edition" } ]
+    end
+  end
+
   def test_upsert_all_only_applies_last_value_when_given_duplicate_identifiers
     skip unless supports_insert_on_duplicate_update? && !current_adapter?(:PostgreSQLAdapter)