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

fix QueryCache nil dup

5 files changed, 34 insertions, 5 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 4b74527144..eac54e7566 100644
--- a/activerecord/CHANGELOG.md
+++ b/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*
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 faccd1d641..947796eea0 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -10,9 +10,9 @@ module ActiveRecord
       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
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 237367c8b3..284529b46e 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -598,7 +598,12 @@ module ActiveRecord
 
         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)
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 8de69869a4..3fce0a1df1 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -285,13 +285,13 @@ module ActiveRecord
 
     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
 
diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb
index 90054ce83d..4a49bfe9b1 100644
--- a/activerecord/test/cases/query_cache_test.rb
+++ b/activerecord/test/cases/query_cache_test.rb
@@ -202,6 +202,20 @@ class QueryCacheTest < ActiveRecord::TestCase
     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); }