Merge pull request #27341 from richardmonette/fix-querycache-nil-dup

fix QueryCache nil dup

activerecord/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Notifications see frozen SQL string.
+
+    Fixes #23774
+
+    *Richard Monette*
+
+*   RuntimeErrors are no longer translated to ActiveRecord::StatementInvalid.
+
+    *Richard Monette*
+
 *   Change the schema cache format to use YAML instead of Marshal.
 
     *Kir Shatrov*

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

@@ -10,9 +10,9 @@ def initialize
       def to_sql(arel, binds = [])
         if arel.respond_to?(:ast)
           collected = visitor.accept(arel.ast, collector)
-          collected.compile(binds, self)
+          collected.compile(binds, self).freeze
         else
-          arel
+          arel.dup.freeze
         end
       end
 

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -598,7 +598,12 @@ def log(sql, name = "SQL", binds = [], type_casted_binds = [], statement_name =
 
         def translate_exception(exception, message)
           # override in derived class
-          ActiveRecord::StatementInvalid.new(message)
+          case exception
+          when RuntimeError
+            exception
+          else
+            ActiveRecord::StatementInvalid.new(message)
+          end
         end
 
         def without_prepared_statement?(binds)

activerecord/test/cases/adapter_test.rb

@@ -285,13 +285,13 @@ def test_select_methods_passing_a_relation
 
     unless current_adapter?(:PostgreSQLAdapter)
       def test_log_invalid_encoding
-        error = assert_raise ActiveRecord::StatementInvalid do
+        error = assert_raises RuntimeError do
           @connection.send :log, "SELECT 'ы' FROM DUAL" do
             raise "ы".force_encoding(Encoding::ASCII_8BIT)
           end
         end
 
-        assert_not_nil error.cause
+        assert_not_nil error.message
       end
     end
 

activerecord/test/cases/query_cache_test.rb

@@ -202,6 +202,20 @@ def test_cache_does_not_raise_exceptions
     ActiveSupport::Notifications.unsubscribe subscriber
   end
 
+  def test_query_cache_does_not_allow_sql_key_mutation
+    subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |_, _, _, _, payload|
+      payload[:sql].downcase!
+    end
+
+    assert_raises RuntimeError do
+      ActiveRecord::Base.cache do
+        assert_queries(1) { Task.find(1); Task.find(1) }
+      end
+    end
+  ensure
+    ActiveSupport::Notifications.unsubscribe subscriber
+  end
+
   def test_cache_is_flat
     Task.cache do
       assert_queries(1) { Topic.find(1); Topic.find(1); }