Deprecate `{insert|update|delete}_sql` in `DatabaseStatements`

Originally, `{insert|update|delete}_sql` is protected methods.
We can use the `{insert|update|delete}` public methods instead.

5 files changed, 17 insertions, 60 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 7438ac8a9d..861f2c3c36 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Deprecate `{insert|update|delete}_sql` in `DatabaseStatements`.
+    Use the `{insert|update|delete}` public methods instead.
+
+    *Ryuta Kamizono*
+
 *   Added a configuration option to have active record raise an ArgumentError
     if the order or limit is ignored in a batch query, rather than logging a
     warning message.
@@ -81,7 +86,6 @@
 
     *Bogdan Gusiev*
 
-
 *   Allow `joins` to be unscoped.
 
     Fixes #13775.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
index aa5ae15285..824040775d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -125,18 +125,21 @@ module ActiveRecord
       end
       alias create insert
       alias insert_sql insert
+      deprecate insert_sql: :insert
 
       # Executes the update statement and returns the number of rows affected.
       def update(arel, name = nil, binds = [])
         exec_update(to_sql(arel, binds), name, binds)
       end
       alias update_sql update
+      deprecate update_sql: :update
 
       # Executes the delete statement and returns the number of rows affected.
       def delete(arel, name = nil, binds = [])
         exec_delete(to_sql(arel, binds), name, binds)
       end
       alias delete_sql delete
+      deprecate delete_sql: :delete
 
       # Returns +true+ when the connection adapter supports prepared statement
       # caching, otherwise returns +false+
diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
index fbdaab2fdd..8b08ebc3c4 100644
--- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
@@ -60,50 +60,6 @@ module ActiveRecord
         end
       end
 
-      def test_insert_sql_with_proprietary_returning_clause
-        with_example_table do
-          id = @connection.insert_sql("insert into ex (number) values(5150)", nil, "number")
-          assert_equal 5150, id
-        end
-      end
-
-      def test_insert_sql_with_quoted_schema_and_table_name
-        with_example_table do
-          id = @connection.insert_sql('insert into "public"."ex" (number) values(5150)')
-          expect = @connection.query('select max(id) from ex').first.first
-          assert_equal expect, id
-        end
-      end
-
-      def test_insert_sql_with_no_space_after_table_name
-        with_example_table do
-          id = @connection.insert_sql("insert into ex(number) values(5150)")
-          expect = @connection.query('select max(id) from ex').first.first
-          assert_equal expect, id
-        end
-      end
-
-      def test_multiline_insert_sql
-        with_example_table do
-          id = @connection.insert_sql(<<-SQL)
-          insert into ex(
-                         number)
-          values(
-                 5152
-                 )
-          SQL
-          expect = @connection.query('select max(id) from ex').first.first
-          assert_equal expect, id
-        end
-      end
-
-      def test_insert_sql_with_returning_disabled
-        connection = connection_without_insert_returning
-        id = connection.insert_sql("insert into postgresql_partitioned_table_parent (number) VALUES (1)")
-        expect = connection.query('select max(id) from postgresql_partitioned_table_parent').first.first
-        assert_equal expect.to_i, id
-      end
-
       def test_exec_insert_with_returning_disabled
         connection = connection_without_insert_returning
         result = connection.exec_insert("insert into postgresql_partitioned_table_parent (number) VALUES (1)", nil, [], 'id', 'postgresql_partitioned_table_parent_id_seq')
diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
index 02c3358ba6..bbc9f978bf 100644
--- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
+++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
@@ -213,24 +213,12 @@ module ActiveRecord
         assert_equal "''", @conn.quote_string("'")
       end
 
-      def test_insert_sql
-        with_example_table do
-          2.times do |i|
-            rv = @conn.insert_sql "INSERT INTO ex (number) VALUES (#{i})"
-            assert_equal(i + 1, rv)
-          end
-
-          records = @conn.execute "SELECT * FROM ex"
-          assert_equal 2, records.length
-        end
-      end
-
-      def test_insert_sql_logged
+      def test_insert_logged
         with_example_table do
           sql = "INSERT INTO ex (number) VALUES (10)"
           name = "foo"
           assert_logged [[sql, name, []]] do
-            @conn.insert_sql sql, name
+            @conn.insert(sql, name)
           end
         end
       end
@@ -239,7 +227,7 @@ module ActiveRecord
         with_example_table do
           sql = "INSERT INTO ex (number) VALUES (10)"
           idval = 'vuvuzela'
-          id = @conn.insert_sql sql, nil, nil, idval
+          id = @conn.insert(sql, nil, nil, idval)
           assert_equal idval, id
         end
       end
diff --git a/activerecord/test/cases/database_statements_test.rb b/activerecord/test/cases/database_statements_test.rb
index ba085991e0..3169408ac0 100644
--- a/activerecord/test/cases/database_statements_test.rb
+++ b/activerecord/test/cases/database_statements_test.rb
@@ -13,6 +13,12 @@ class DatabaseStatementsTest < ActiveRecord::TestCase
     assert_not_nil return_the_inserted_id(method: :create)
   end
 
+  def test_insert_update_delete_sql_is_deprecated
+    assert_deprecated { @connection.insert_sql("INSERT INTO accounts (firm_id,credit_limit) VALUES (42,5000)") }
+    assert_deprecated { @connection.update_sql("UPDATE accounts SET credit_limit = 6000 WHERE firm_id = 42") }
+    assert_deprecated { @connection.delete_sql("DELETE FROM accounts WHERE firm_id = 42") }
+  end
+
   private
 
   def return_the_inserted_id(method:)