Merge pull request #767 from aidanharan/query-cache-types-reset

Rails 6: Allow primary key query in query cache test

Author: wpolicarpo

lib/arel/visitors/sqlserver.rb

@@ -153,7 +153,6 @@ def visit_Arel_Nodes_SelectStatement_SQLServer_Lock collector, options = {}
       def visit_Orders_And_Let_Fetch_Happen o, collector
         make_Fetch_Possible_And_Deterministic o
         unless o.orders.empty?
-          collector << " "
           collector << " ORDER BY "
           len = o.orders.length - 1
           o.orders.each_with_index { |x, i|

test/cases/coerced_tests.rb

@@ -756,6 +756,33 @@ def test_cache_does_not_wrap_results_in_arrays_coerced
       assert_kind_of Numeric, Task.connection.select_value("SELECT count(*) AS count_all FROM tasks")
     end
   end
+
+
+  # Same as original test except that we expect one query to be performed to retrieve the table's primary key.
+  # When we generate the SQL for the `find` it includes ordering on the primary key. If we reset the column
+  # information then the primary key needs to be retrieved from the database again to generate the SQL causing the
+  # original test's `assert_no_queries` assertion to fail. Assert that the query was to get the primary key.
+  coerce_tests! :test_query_cached_even_when_types_are_reset
+  def test_query_cached_even_when_types_are_reset_coerced
+    Task.cache do
+      # Warm the cache
+      Task.find(1)
+
+      # Preload the type cache again (so we don't have those queries issued during our assertions)
+      Task.connection.send(:reload_type_map)
+
+      # Clear places where type information is cached
+      Task.reset_column_information
+      Task.initialize_find_by_cache
+      Task.define_attribute_methods
+
+      assert_queries(1, ignore_none: true) do
+        Task.find(1)
+      end
+
+      assert_includes ActiveRecord::SQLCounter.log_all.first , "TC.CONSTRAINT_TYPE = N''PRIMARY KEY''"
+    end
+  end
 end