Intent execute

Dockerfile.ci

@@ -9,6 +9,6 @@ WORKDIR $WORKDIR
 
 COPY . $WORKDIR
 
-RUN RAILS_COMMIT=65dc2163e3 bundle install --jobs `expr $(cat /proc/cpuinfo | grep -c "cpu cores") - 1` --retry 3
+RUN RAILS_BRANCH=main bundle install --jobs `expr $(cat /proc/cpuinfo | grep -c "cpu cores") - 1` --retry 3
 
 CMD ["sh"]

compose.ci.yaml

@@ -4,7 +4,7 @@ services:
   ci:
     environment:
       - ACTIVERECORD_UNITTEST_HOST=sqlserver
-      - RAILS_COMMIT=65dc2163e3
+      - RAILS_BRANCH=main
     build:
       context: .
       dockerfile: Dockerfile.ci
@@ -13,7 +13,7 @@ services:
       - "sqlserver"
   standardrb:
     environment:
-      - RAILS_COMMIT=65dc2163e3
+      - RAILS_BRANCH=main
     build:
       context: .
       dockerfile: Dockerfile.ci

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -16,7 +16,7 @@ def write_query?(sql) # :nodoc:
         def perform_query(raw_connection, intent)
           sql = intent.processed_sql
 
-          unless intent.binds.nil? || intent.binds.empty?
+          if intent.has_binds?
             types, params = sp_executesql_types_and_parameters(intent.binds)
             sql = sp_executesql_sql(intent.processed_sql, types, params, intent.notification_payload[:name])
           end
@@ -73,15 +73,12 @@ def delete(arel, name = nil, binds = [])
 
           intent = QueryIntent.new(adapter: self, arel: arel, name: name, binds: binds)
 
-          # Compile Arel to get SQL
-          compile_arel_in_intent(intent)
-
           # Add `SELECT @@ROWCOUNT` to the end of the SQL to get the number of affected rows. This is needed because SQL Server does not return the number of affected rows in the same way as other databases.
           sql = intent.processed_sql.present? ? intent.processed_sql : intent.raw_sql
-          ensure_writes_are_allowed(sql) if write_query?(sql)
-          intent.processed_sql = "#{sql}; SELECT @@ROWCOUNT AS AffectedRows"
+          intent.instance_variable_set(:@processed_sql, "#{sql}; SELECT @@ROWCOUNT AS AffectedRows")
 
-          affected_rows(raw_execute(intent))
+          intent.execute!
+          intent.affected_rows
         end
 
         # Executes the update statement and returns the number of rows affected.
@@ -93,19 +90,16 @@ def update(arel, name = nil, binds = [])
 
           intent = QueryIntent.new(adapter: self, arel: arel, name: name, binds: binds)
 
-          # Compile Arel to get SQL
-          compile_arel_in_intent(intent)
-
           # Add `SELECT @@ROWCOUNT` to the end of the SQL to get the number of affected rows. This is needed because SQL Server does not return the number of affected rows in the same way as other databases.
           sql = intent.processed_sql.present? ? intent.processed_sql : intent.raw_sql
-          ensure_writes_are_allowed(sql) if write_query?(sql)
-          intent.processed_sql = "#{sql}; SELECT @@ROWCOUNT AS AffectedRows"
+          intent.instance_variable_set(:@processed_sql, "#{sql}; SELECT @@ROWCOUNT AS AffectedRows")
 
-          affected_rows(raw_execute(intent))
+          intent.execute!
+          intent.affected_rows
         end
 
         def begin_db_transaction
-          internal_execute("BEGIN TRANSACTION", "TRANSACTION", allow_retry: true, materialize_transactions: false)
+          query_command("BEGIN TRANSACTION", "TRANSACTION", allow_retry: true, materialize_transactions: false)
         end
 
         def transaction_isolation_levels
@@ -118,15 +112,15 @@ def begin_isolated_db_transaction(isolation)
         end
 
         def set_transaction_isolation_level(isolation_level)
-          internal_execute("SET TRANSACTION ISOLATION LEVEL #{isolation_level}", "TRANSACTION", allow_retry: true, materialize_transactions: false)
+          query_command("SET TRANSACTION ISOLATION LEVEL #{isolation_level}", "TRANSACTION", allow_retry: true, materialize_transactions: false)
         end
 
         def commit_db_transaction
-          internal_execute("COMMIT TRANSACTION", "TRANSACTION", allow_retry: false, materialize_transactions: true)
+          query_command("COMMIT TRANSACTION", "TRANSACTION", allow_retry: false, materialize_transactions: true)
         end
 
         def exec_rollback_db_transaction
-          internal_execute("IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION", "TRANSACTION", allow_retry: false, materialize_transactions: true)
+          query_command("IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION", "TRANSACTION", allow_retry: false, materialize_transactions: true)
         end
 
         def case_sensitive_comparison(attribute, value)
@@ -370,7 +364,7 @@ def newsequentialid_function
         def sql_for_insert(sql, pk, binds, returning)
           if pk.nil?
             table_name = query_requires_identity_insert?(sql)
-            pk = primary_key(table_name)
+            pk = schema_cache.primary_keys(table_name)
           end
 
           sql = if pk && use_output_inserted? && !database_prefix_remote_server?

lib/active_record/connection_adapters/sqlserver/quoting.rb

@@ -79,6 +79,7 @@ def quote_string_single_national(s)
 
         def quote_default_expression(value, column)
           cast_type = lookup_cast_type(column.sql_type)
+
           if cast_type.type == :uuid && value.is_a?(String) && value.include?("()")
             value
           else
@@ -118,7 +119,7 @@ def quote(value)
             "0x#{value.hex}"
           when ActiveRecord::Type::SQLServer::Data
             value.quoted
-          when String, ActiveSupport::Multibyte::Chars
+          when String
             "N#{super}"
           else
             super
@@ -133,6 +134,10 @@ def type_cast(value)
             super
           end
         end
+
+        def lookup_cast_type(sql_type)
+          type_map.lookup(sql_type)
+        end
       end
     end
   end

lib/active_record/connection_adapters/sqlserver/savepoints.rb

@@ -9,11 +9,11 @@ def current_savepoint_name
         end
 
         def create_savepoint(name = current_savepoint_name)
-          internal_execute("SAVE TRANSACTION #{name}", "TRANSACTION")
+          query_command("SAVE TRANSACTION #{name}", "TRANSACTION")
         end
 
         def exec_rollback_to_savepoint(name = current_savepoint_name)
-          internal_execute("ROLLBACK TRANSACTION #{name}", "TRANSACTION")
+          query_command("ROLLBACK TRANSACTION #{name}", "TRANSACTION")
         end
 
         # SQL Server does require save-points to be explicitly released.

lib/active_record/connection_adapters/sqlserver/schema_statements.rb

@@ -10,31 +10,34 @@ def create_table(table_name, **options)
           res
         end
 
-        def drop_table(*table_names, **options)
-          table_names.each do |table_name|
-            # Mimic CASCADE option as best we can.
-            if options[:force] == :cascade
-              execute_procedure(:sp_fkeys, pktable_name: table_name).each do |fkdata|
-                fktable = fkdata["FKTABLE_NAME"]
-                fkcolmn = fkdata["FKCOLUMN_NAME"]
-                pktable = fkdata["PKTABLE_NAME"]
-                pkcolmn = fkdata["PKCOLUMN_NAME"]
-                remove_foreign_key fktable, name: fkdata["FK_NAME"]
-                execute "DELETE FROM #{quote_table_name(fktable)} WHERE #{quote_column_name(fkcolmn)} IN ( SELECT #{quote_column_name(pkcolmn)} FROM #{quote_table_name(pktable)} )"
-              end
-            end
-            if options[:if_exists] && version_year < 2016
-              execute "IF EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = #{quote(table_name)}) DROP TABLE #{quote_table_name(table_name)}", "SCHEMA"
-            else
-              super
+        def drop_table_sql(table_name, if_exists: nil, force: nil, **)
+          sqls = []
+
+          # Mimic CASCADE option as best we can.
+          if force == :cascade
+            execute_procedure(:sp_fkeys, pktable_name: table_name).each do |fkdata|
+              fktable = fkdata["FKTABLE_NAME"]
+              fkcolmn = fkdata["FKCOLUMN_NAME"]
+              pktable = fkdata["PKTABLE_NAME"]
+              pkcolmn = fkdata["PKCOLUMN_NAME"]
+              remove_foreign_key fktable, name: fkdata["FK_NAME"]
+
+              sqls << "DELETE FROM #{quote_table_name(fktable)} WHERE #{quote_column_name(fkcolmn)} IN ( SELECT #{quote_column_name(pkcolmn)} FROM #{quote_table_name(pktable)} )"
             end
           end
+
+          sqls << if if_exists && version_year < 2016
+            "IF EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = #{quote(table_name)}) DROP TABLE #{quote_table_name(table_name)}"
+          else
+            super
+          end
+
+          sqls.join("; ")
         end
 
         def indexes(table_name)
           data = begin
-            intent = QueryIntent.new(adapter: self, raw_sql: "EXEC sp_helpindex #{quote(table_name)}", name: "SCHEMA")
-            select(intent)
+            select_all("EXEC sp_helpindex #{quote(table_name)}", "SCHEMA")
           rescue
             []
           end
@@ -162,7 +165,7 @@ def primary_keys_select(table_name)
           binds << Relation::QueryAttribute.new("TABLE_NAME", identifier.object, nv128)
           binds << Relation::QueryAttribute.new("TABLE_SCHEMA", identifier.schema, nv128) unless identifier.schema.blank?
 
-          internal_exec_query(sql, "SCHEMA", binds).map { |row| row["name"] }
+          select_all(sql, "SCHEMA", binds).map { |row| row["name"] }
         end
 
         def rename_table(table_name, new_name, **options)
@@ -323,7 +326,7 @@ def check_constraints(table_name)
             st.name = '#{table_name}'
           SQL
 
-          chk_info = internal_exec_query(sql, "SCHEMA")
+          chk_info = select_all(sql, "SCHEMA")
 
           chk_info.map do |row|
             options = {
@@ -513,7 +516,7 @@ def column_definitions(table_name)
               FROM #{database}.INFORMATION_SCHEMA.COLUMNS c
               WHERE c.TABLE_NAME = #{quote(view_table_name(table_name))}
             SQL
-            results = internal_exec_query(sql, "SCHEMA")
+            results = select_all(sql, "SCHEMA")
             default_functions = results.each.with_object({}) { |row, out| out[row["name"]] = row["default"] }.compact
           end
 
@@ -524,7 +527,7 @@ def column_definitions(table_name)
           binds << Relation::QueryAttribute.new("TABLE_NAME", identifier.object, nv128)
           binds << Relation::QueryAttribute.new("TABLE_SCHEMA", identifier.schema, nv128) unless identifier.schema.blank?
 
-          results = internal_exec_query(sql, "SCHEMA", binds)
+          results = select_all(sql, "SCHEMA", binds)
           raise ActiveRecord::StatementInvalid, "Table '#{table_name}' doesn't exist" if results.empty?
 
           results.map do |ci|

lib/active_record/connection_adapters/sqlserver/showplan.rb

@@ -14,7 +14,7 @@ module Showplan
 
         def explain(arel, binds = [], options = [])
           sql = to_sql(arel)
-          result = with_showplan_on { internal_exec_query(sql, "EXPLAIN", binds) }
+          result = with_showplan_on { select_all(sql, "EXPLAIN", binds) }
           printer = showplan_printer.new(result)
 
           printer.pp
@@ -31,7 +31,8 @@ def with_showplan_on
 
         def set_showplan_option(enable = true)
           intent = QueryIntent.new(adapter: self, raw_sql: "SET #{showplan_option} #{enable ? "ON" : "OFF"}", name: "SCHEMA")
-          raw_execute(intent)
+          intent.execute!
+          intent.finish
         rescue
           raise ActiveRecordError, "#{showplan_option} could not be turned #{enable ? "ON" : "OFF"}, perhaps you do not have SHOWPLAN permissions?"
         end

test/cases/adapter_test_sqlserver.rb

@@ -615,7 +615,9 @@ def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
 
     it "records can be inserted using SQL" do
       assert_difference("Alien.count", 2) do
-        Alien.lease_connection.exec_insert("insert into [test].[aliens] (id, name) VALUES(1, 'Trisolarans'), (2, 'Xenomorph')")
+        assert_deprecated(ActiveRecord.deprecator) do
+          Alien.lease_connection.exec_insert("insert into [test].[aliens] (id, name) VALUES(1, 'Trisolarans'), (2, 'Xenomorph')")
+        end
       end
     end
   end
@@ -630,9 +632,14 @@ def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
 
   describe "exec_insert" do
     it "values clause should be case-insensitive" do
+      first_insert = nil
+      second_insert = nil
+
       assert_difference("Post.count", 4) do
-        first_insert = connection.exec_insert("INSERT INTO [posts] ([id],[title],[body]) VALUES(100, 'Title', 'Body'), (102, 'Title', 'Body')")
-        second_insert = connection.exec_insert("INSERT INTO [posts] ([id],[title],[body]) values(113, 'Body', 'Body'), (114, 'Body', 'Body')")
+        assert_deprecated(ActiveRecord.deprecator) do
+          first_insert = connection.exec_insert("INSERT INTO [posts] ([id],[title],[body]) VALUES(100, 'Title', 'Body'), (102, 'Title', 'Body')")
+          second_insert = connection.exec_insert("INSERT INTO [posts] ([id],[title],[body]) values(113, 'Body', 'Body'), (114, 'Body', 'Body')")
+        end
 
         assert_equal first_insert.rows.map(&:first), [100, 102]
         assert_equal second_insert.rows.map(&:first), [113, 114]

test/cases/coerced_tests.rb

@@ -523,8 +523,8 @@ def test_create_table_with_defaults_coerce
         five = columns.detect { |c| c.name == "five" }
 
         assert_equal "hello", one.default
-        assert_equal true, two.fetch_cast_type(connection).deserialize(two.default)
-        assert_equal false, three.fetch_cast_type(connection).deserialize(three.default)
+        assert_equal true, two.cast_type.deserialize(two.default)
+        assert_equal false, three.cast_type.deserialize(three.default)
         assert_equal 1, four.default
         assert_equal "hello", five.default
       end
@@ -1993,32 +1993,13 @@ class SchemaCacheTest < ActiveRecord::TestCase
       # Tests fail on Windows AppVeyor CI with 'Permission denied' error when renaming file during `File.atomic_write` call.
       coerce_tests! :test_yaml_dump_and_load, :test_yaml_dump_and_load_with_gzip if /mswin|mingw/.match?(RbConfig::CONFIG["host_os"])
 
-      # Cast type in SQL Server is :varchar rather than Unicode :string.
-      coerce_tests! :test_yaml_load_8_0_dump_without_cast_type_still_get_the_right_one
-      def test_yaml_load_8_0_dump_without_cast_type_still_get_the_right_one
-        cache = load_bound_reflection(schema_dump_8_0_path)
-
-        assert_no_queries do
-          columns = cache.columns_hash("courses")
-          assert_equal 3, columns.size
-          cast_type = columns["name"].fetch_cast_type(@connection)
-          assert_not_nil cast_type, "expected cast_type to be present"
-          assert_equal :varchar, cast_type.type
-        end
-      end
-
       private
 
       # We need to give the full paths for this to work.
       undef_method :schema_dump_5_1_path
       def schema_dump_5_1_path
         File.join(ARTest::SQLServer.root_activerecord, "test/assets/schema_dump_5_1.yml")
       end
-
-      undef_method :schema_dump_8_0_path
-      def schema_dump_8_0_path
-        File.join(ARTest::SQLServer.root_activerecord, "test/assets/schema_dump_8_0.yml")
-      end
     end
   end
 end
@@ -2843,3 +2824,33 @@ def test_in_batches_loaded_should_unscope_cursor_after_pluck_coerced
   end
 end
 
+class TransactionInstrumentationTest < ActiveRecord::TestCase
+  # SQL Server does not have query for release_savepoint.
+  coerce_tests! :test_sql_events_do_not_overlap_with_savepoints
+  def test_sql_events_do_not_overlap_with_savepoints_coerced
+    events = []
+    subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event|
+      events << event
+    end
+
+    Topic.transaction do
+      Topic.count
+      Topic.transaction(requires_new: true) { Topic.first }
+    end
+
+    assert_equal 5, events.size
+    begin_event, count_event, savepoint_event, select_event, commit_event = events
+
+    assert begin_event.payload[:sql].start_with?("BEGIN")
+    assert count_event.payload[:sql].start_with?("SELECT")
+    assert savepoint_event.payload[:sql].start_with?("SAVE TRANSACTION")
+    assert select_event.payload[:sql].start_with?("SELECT")
+    assert commit_event.payload[:sql].start_with?("COMMIT")
+
+    events.each_cons(2) do |a, b|
+      assert_operator a.end, :<=, b.time
+    end
+  ensure
+    ActiveSupport::Notifications.unsubscribe(subscriber)
+  end
+end