Merge pull request #26584 from Shopify/cached-query-name

Preserve cached queries name in AS notifications

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

@@ -65,7 +65,7 @@ def select_all(arel, name = nil, binds = [], preparable: nil)
         if @query_cache_enabled && !locked?(arel)
           arel, binds = binds_from_relation arel, binds
           sql = to_sql(arel, binds)
-          cache_sql(sql, binds) { super(sql, name, binds, preparable: preparable) }
+          cache_sql(sql, name, binds) { super(sql, name, binds, preparable: preparable) }
         else
           super
         end
@@ -73,11 +73,17 @@ def select_all(arel, name = nil, binds = [], preparable: nil)
 
       private
 
-        def cache_sql(sql, binds)
+        def cache_sql(sql, name, binds)
           result =
             if @query_cache[sql].key?(binds)
-              ActiveSupport::Notifications.instrument("sql.active_record",
-                sql: sql, binds: binds, name: "CACHE", connection_id: object_id)
+              ActiveSupport::Notifications.instrument(
+                "sql.active_record",
+                sql: sql,
+                binds: binds,
+                name: name,
+                connection_id: object_id,
+                cached: true,
+              )
               @query_cache[sql][binds]
             else
               @query_cache[sql][binds] = yield

activerecord/lib/active_record/explain_subscriber.rb

@@ -18,10 +18,13 @@ def finish(name, id, payload)
     #
     # On the other hand, we want to monitor the performance of our real database
     # queries, not the performance of the access to the query cache.
-    IGNORED_PAYLOADS = %w(SCHEMA EXPLAIN CACHE)
+    IGNORED_PAYLOADS = %w(SCHEMA EXPLAIN)
     EXPLAINED_SQLS = /\A\s*(with|select|update|delete|insert)\b/i
     def ignore_payload?(payload)
-      payload[:exception] || IGNORED_PAYLOADS.include?(payload[:name]) || payload[:sql] !~ EXPLAINED_SQLS
+      payload[:exception] ||
+        payload[:cached] ||
+        IGNORED_PAYLOADS.include?(payload[:name]) ||
+        payload[:sql] !~ EXPLAINED_SQLS
     end
 
     ActiveSupport::Notifications.subscribe("sql.active_record", new)

activerecord/lib/active_record/log_subscriber.rb

@@ -35,6 +35,7 @@ def sql(event)
       return if IGNORE_PAYLOAD_NAMES.include?(payload[:name])
 
       name  = "#{payload[:name]} (#{event.duration.round(1)}ms)"
+      name  = "CACHE #{name}" if payload[:cached]
       sql   = payload[:sql]
       binds = nil
 

activerecord/test/cases/test_case.rb

@@ -125,12 +125,9 @@ def initialize(ignore = Regexp.union(self.class.ignored_sql))
     end
 
     def call(name, start, finish, message_id, values)
-      sql = values[:sql]
-
-      # FIXME: this seems bad. we should probably have a better way to indicate
-      # the query was cached
-      return if "CACHE" == values[:name]
+      return if values[:cached]
 
+      sql = values[:sql]
       self.class.log_all << sql
       self.class.log << sql unless ignore.match?(sql)
     end