Add implicit fields clause to "rows" aggregation queries

src/metabase/driver/query_processor.clj

@@ -8,6 +8,7 @@
 
 (declare add-implicit-breakout-order-by
          add-implicit-limit
+         add-implicit-fields
          get-special-column-info
          preprocess-cumulative-sum
          preprocess-structured
@@ -51,6 +52,7 @@
                                                            remove-empty-clauses
                                                            add-implicit-breakout-order-by
                                                            add-implicit-limit
+                                                           add-implicit-fields
                                                            preprocess-cumulative-sum))]
     (when-not *disable-qp-logging*
       (log/debug (colorize.core/cyan "\n******************** PREPROCESSED: ********************\n"
@@ -102,7 +104,7 @@
 ;;; ### ADD-IMPLICIT-LIMIT
 
 (defn add-implicit-limit
-  "Add an implicit limit clause to queries with `rows` aggregations."
+  "Add an implicit `limit` clause to queries with `rows` aggregations."
   [{:keys [limit aggregation] :as query}]
   (if (and (= aggregation ["rows"])
            (not limit))
@@ -110,6 +112,21 @@
     query))
 
 
+;;; ### ADD-IMPLICIT-FIELDS
+
+(defn add-implicit-fields
+  "Add an implicit `fields` clause to queries with `rows` aggregations."
+  [{:keys [fields aggregation source_table] :as query}]
+  (if (or (not= aggregation ["rows"])
+          fields)
+    ;; If we're not doing a "rows" aggregation or the fields clause was specified return the query as-is
+    query
+    ;; Otherwise we need to look up the Fields for the Table in question so we don't return ones that are supposed to be hidden
+    (let [fields (sel :many :id Field :table_id source_table, :active true, :preview_display true, :field_type [not= "sensitive"])]
+      (assoc query
+             :fields fields))))
+
+
 ;; ### PREPROCESS-CUMULATIVE-SUM
 
 (defn preprocess-cumulative-sum

test/metabase/driver/query_processor_test.clj

@@ -4,7 +4,8 @@
             [metabase.db :refer :all]
             [metabase.driver :as driver]
             [metabase.driver.query-processor :refer :all]
-            (metabase.models [table :refer [Table]])
+            (metabase.models [field :refer [Field]]
+                             [table :refer [Table]])
             [metabase.test.data.datasets :as datasets :refer [*dataset*]]))
 
 ;; ##  Dataset-Independent Data Fns
@@ -706,3 +707,24 @@
    :aggregation  ["stddev" (id :venues :category_id)]
    :breakout     [(id :venues :price)]
    :order_by     [[["aggregation" 0] "descending"]]})
+
+;;; ### make sure that rows where preview_display = false don't get displayed
+
+(datasets/expect-with-all-datasets
+ [(set (->columns "category_id" "name" "latitude" "id" "longitude" "price"))
+  (set (->columns "category_id" "name" "latitude" "id" "longitude"))
+  (set (->columns "category_id" "name" "latitude" "id" "longitude" "price"))]
+ (let [get-col-names (fn [] (-> (driver/process-query {:database (db-id)
+                                                       :type     "query"
+                                                       :query    {:aggregation  ["rows"]
+                                                                  :source_table (id :venues)
+                                                                  :order_by     [[(id :venues :id) "ascending"]]
+                                                                  :limit        1}})
+                                :data
+                                :columns
+                                set))]
+   [(get-col-names)
+    (do (upd Field (id :venues :price) :preview_display false)
+        (get-col-names))
+    (do (upd Field (id :venues :price) :preview_display true)
+        (get-col-names))]))