From 9ac54e4522de51614d7af2a2e4d2316d10a8c1fe Mon Sep 17 00:00:00 2001 From: Cam Saul <cam@geotip.com> Date: Wed, 3 Jun 2015 18:48:26 -0700 Subject: [PATCH] :cry: --- src/metabase/driver.clj | 3 ++- src/metabase/driver/query_processor.clj | 25 ++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj index 2f581cc51ea..149d859eaae 100644 --- a/src/metabase/driver.clj +++ b/src/metabase/driver.clj @@ -128,7 +128,8 @@ [query] {:pre [(map? query)]} (try - (binding [qp/*query* query] + (binding [qp/*query* query + qp/*internal-context* (atom {})] (let [driver (database-id->driver (:database query)) query (qp/preprocess query) results (binding [qp/*query* query] diff --git a/src/metabase/driver/query_processor.clj b/src/metabase/driver/query_processor.clj index 7116d61ee2c..1ea2b51e625 100644 --- a/src/metabase/driver/query_processor.clj +++ b/src/metabase/driver/query_processor.clj @@ -36,6 +36,10 @@ "Should we disable logging for the QP? (e.g., during sync we probably want to turn it off to keep logs less cluttered)." false) +(def ^:dynamic *internal-context* + "A neat place to store 'notes-to-self': things individual implementations don't need to know about, like if the `fields` clause was added implicitly." + (atom nil)) + ;; # PREPROCESSOR @@ -118,12 +122,15 @@ (defn add-implicit-fields "Add an implicit `fields` clause to queries with `rows` aggregations." [{:keys [fields aggregation breakout source_table] :as query}] - (cond-> query + (if-not (and (= aggregation ["rows"]) + (not breakout) + (not fields)) + query ;; If we're doing a "rows" aggregation with no breakout or fields clauses add one that will exclude Fields that are supposed to be hidden - (and (= aggregation ["rows"]) - (not breakout) - (not fields)) (assoc :fields (sel :many :id Field :table_id source_table, :active true, :preview_display true, - :field_type [not= "sensitive"], (order :position :asc), (order :id :desc))))) + (do (swap! *internal-context* assoc :fields-is-implicit true) + (println "---------------------------------------- INTERNAL CONTEXT >>>> " @*internal-context*) + (assoc query :fields (sel :many :id Field :table_id source_table, :active true, :preview_display true, + :field_type [not= "sensitive"], (order :position :asc), (order :id :desc)))))) ;; ### PREPROCESS-CUMULATIVE-SUM @@ -147,7 +154,7 @@ ;; to just fetch all the values of the field in question. cum-sum-with-same-breakout? (-> query (dissoc :breakout) - (assoc :cum_sum ag-field + (assoc :cum_sum ag-field ; TODO - move this to *internal-context* instead? :aggregation ["rows"] :fields [ag-field])) @@ -318,8 +325,7 @@ (:id %)) (contains? (set field-field-ids) (:id %))))) - (sort-by (fn [{:keys [position id]}] - [position (when id (- id))])))] + (sort-by :position))] (->> (concat breakout-fields field-fields other-fields) (map :castified) (filter identity)))) @@ -335,9 +341,10 @@ [{{source-table :source_table, breakout-field-ids :breakout, field-field-ids :fields} :query} castified-field-names] {:post [(every? keyword? %)]} (try + (println "FIELDS-IS-IMPLICIT? >>>>" @*internal-context*) (-order-columns (sel :many :fields [Field :id :name :position] :table_id source-table) breakout-field-ids - field-field-ids + (when-not (:fields-is-implicit @*internal-context*) field-field-ids) castified-field-names) (catch Exception e (.printStackTrace e) -- GitLab