Fix insights not working on cached results (#12889)

* Fix insights not working on cached results

* Make test more robust

src/metabase/query_processor/middleware/cache.clj

@@ -126,7 +126,7 @@
          (rf))
 
         ([acc]
-         ;; if results are in the 'normal format' the use return the final metadata from the cache rather than
+         ;; if results are in the 'normal format' then use the final metadata from the cache rather than
          ;; whatever `acc` is right now since we don't run the entire post-processing pipeline for cached results
          (let [normal-format? (and (map? acc) (seq (get-in acc [:data :cols])))
                acc*           (-> (if normal-format?

src/metabase/sync/analyze/fingerprint/insights.clj

@@ -43,15 +43,15 @@
    https://en.wikipedia.org/wiki/Reservoir_sampling"
   [n]
   (fn
-    ([] [(transient []) 0])
+    ([] [[] 0])
     ([[reservoir c] x]
      (let [c   (inc c)
            idx (rand-int c)]
        (cond
-         (<= c n)  [(conj! reservoir x) c]
-         (< idx n) [(assoc! reservoir idx x) c]
+         (<= c n)  [(conj reservoir x) c]
+         (< idx n) [(assoc reservoir idx x) c]
          :else     [reservoir c])))
-    ([[reservoir _]] (persistent! reservoir))))
+    ([[reservoir _]] reservoir)))
 
 (defn mae
   "Given two functions: (fŷ input) and (fy input), returning the predicted and actual values of y

test/metabase/query_processor/middleware/cache_test.clj

@@ -19,7 +19,9 @@
             [metabase.query-processor.middleware.cache :as cache]
             [metabase.query-processor.middleware.cache-backend.interface :as i]
             [metabase.query-processor.middleware.cache.impl :as impl]
-            [metabase.test.fixtures :as fixtures]
+            [metabase.test
+             [fixtures :as fixtures]
+             [util :as tu]]
             [pretty.core :as pretty]))
 
 (use-fixtures :once (fixtures/initialize :db))
@@ -313,6 +315,30 @@
                            (m/dissoc-in [:data :results_metadata :checksum])))
                     "Cached result should be in the same format as the uncached result, except for added keys")))))))))
 
+(deftest insights-from-cache-test
+  (testing "Insights should work on cahced results (#12556)"
+    (with-mock-cache [save-chan]
+      (let [query (-> checkins
+                      (mt/mbql-query {:breakout    [[:datetime-field (mt/id :checkins :date) :month]]
+                                      :aggregation [[:count]]})
+                      (assoc :cache-ttl 100))]
+        (qp/process-query query)
+        ;; clear any existing values in the `save-chan`
+        (while (a/poll! save-chan))
+        (mt/wait-for-result save-chan)
+        (is (= {:previous-value 24
+                :unit           :month
+                :offset         -45.27
+                :last-change    -0.46
+                :last-value     13
+                :col            "count"}
+               (tu/round-all-decimals 2 (-> query
+                                            qp/process-query
+                                            :data
+                                            :insights
+                                            first
+                                            (dissoc :best-fit :slope)))))))))
+
 (deftest export-test
   (testing "Should be able to cache results streaming as an alternate download format, e.g. csv"
     (with-mock-cache [save-chan]