Force Arel.sql for returning and on_duplicate

activerecord/CHANGELOG.md

@@ -1,15 +1,15 @@
-*   Add `update_sql` option to `#upsert_all` to make it possible to use raw SQL to update columns on conflict:
+*   Allow passing SQL as `on_duplicate` value to `#upsert_all` to make it possible to use raw SQL to update columns on conflict:
 
     ```ruby
     Book.upsert_all(
       [{ id: 1, status: 1 }, { id: 2, status: 1 }],
-      update_sql: "status = GREATEST(books.status, EXCLUDED.status)"
+      on_duplicate: Arel.sql("status = GREATEST(books.status, EXCLUDED.status)")
     )
     ```
 
     *Vladimir Dementyev*
 
-*   Allow passing raw SQL as `returning` statement to `#upsert_all`:
+*   Allow passing SQL as `returning` statement to `#upsert_all`:
 
     ```ruby
     Article.insert_all(
@@ -17,8 +17,7 @@
         { title: "Article 1", slug: "article-1", published: false },
         { title: "Article 2", slug: "article-2", published: false }
       ],
-      # Some PostgreSQL magic here to detect which rows have been actually inserted
-      returning: "id, (xmax = '0') as inserted, name as new_name"
+      returning: Arel.sql("id, (xmax = '0') as inserted, name as new_name")
     )
     ```
 

activerecord/lib/active_record/insert_all.rb

@@ -7,11 +7,19 @@ module ActiveRecord
     attr_reader :model, :connection, :inserts, :keys
     attr_reader :on_duplicate, :returning, :unique_by, :update_sql
 
-    def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil, update_sql: nil)
+    def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil)
       raise ArgumentError, "Empty list of attributes passed" if inserts.blank?
 
       @model, @connection, @inserts, @keys = model, model.connection, inserts, inserts.first.keys.map(&:to_s)
-      @on_duplicate, @returning, @unique_by, @update_sql = on_duplicate, returning, unique_by, update_sql
+      @on_duplicate, @returning, @unique_by = on_duplicate, returning, unique_by
+
+      disallow_raw_sql!(returning)
+      disallow_raw_sql!(on_duplicate)
+
+      if Arel.arel_node?(on_duplicate)
+        @update_sql = on_duplicate
+        @on_duplicate = :update
+      end
 
       if model.scope_attributes?
         @scope_attributes = model.scope_attributes
@@ -127,6 +135,15 @@ module ActiveRecord
         end
       end
 
+      def disallow_raw_sql!(value)
+        return if !value.is_a?(String) || Arel.arel_node?(value)
+
+        raise ArgumentError, "Dangerous query method (method whose arguments are used as raw " \
+                             "SQL) called: #{value}. " \
+                             "Known-safe values can be passed " \
+                             "by wrapping them in Arel.sql()."
+      end
+
       class Builder # :nodoc:
         attr_reader :model
 

activerecord/lib/active_record/persistence.rb

@@ -198,8 +198,8 @@ module ActiveRecord
       # go through Active Record's type casting and serialization.
       #
       # See <tt>ActiveRecord::Persistence#upsert_all</tt> for documentation.
-      def upsert(attributes, returning: nil, unique_by: nil, update_sql: nil)
-        upsert_all([ attributes ], returning: returning, unique_by: unique_by, update_sql: update_sql)
+      def upsert(attributes, on_duplicate: :update, returning: nil, unique_by: nil)
+        upsert_all([ attributes ], on_duplicate: on_duplicate, returning: returning, unique_by: unique_by)
       end
 
       # Updates or inserts (upserts) multiple records into the database in a
@@ -245,7 +245,7 @@ module ActiveRecord
       # <tt>:unique_by</tt> is recommended to be paired with
       # Active Record's schema_cache.
       #
-      # [:update_sql]
+      # [:on_duplicate]
       #   Specify a custom SQL for updating rows on conflict.
       #
       #   NOTE: in this case you must provide all the columns you want to update by yourself.
@@ -261,8 +261,8 @@ module ActiveRecord
       #   ], unique_by: :isbn)
       #
       #   Book.find_by(isbn: "1").title # => "Eloquent Ruby"
-      def upsert_all(attributes, returning: nil, unique_by: nil, update_sql: nil)
-        InsertAll.new(self, attributes, on_duplicate: :update, returning: returning, unique_by: unique_by, update_sql: update_sql).execute
+      def upsert_all(attributes, on_duplicate: :update, returning: nil, unique_by: nil, update_sql: nil)
+        InsertAll.new(self, attributes, on_duplicate: on_duplicate, returning: returning, unique_by: unique_by).execute
       end
 
       # Given an attributes hash, +instantiate+ returns a new instance of

activerecord/test/cases/insert_all_test.rb

@@ -112,7 +112,7 @@ class InsertAllTest < ActiveRecord::TestCase
   def test_insert_all_returns_requested_sql_fields
     skip unless supports_insert_returning?
 
-    result = Book.insert_all! [{ name: "Rework", author_id: 1 }], returning: "UPPER(name) as name"
+    result = Book.insert_all! [{ name: "Rework", author_id: 1 }], returning: Arel.sql("UPPER(name) as name")
     assert_equal %w[ REWORK ], result.pluck("name")
   end
 
@@ -480,7 +480,7 @@ class InsertAllTest < ActiveRecord::TestCase
 
     Book.upsert_all(
       [{ id: 1, status: 1 }, { id: 2, status: 1 }],
-      update_sql: "status = #{operator}(books.status, 1)"
+      on_duplicate: Arel.sql("status = #{operator}(books.status, 1)")
     )
     assert_equal "published", Book.find(1).status
     assert_equal "written", Book.find(2).status