Skip to content
Snippets Groups Projects
Unverified Commit f8ca9b98 authored by Tom Robinson's avatar Tom Robinson
Browse files

Merge branch 'master' of github.com:metabase/metabase into math-aggregations-frontend

parents 9ff09f00 47c5f088
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 303 additions and 151 deletions
docs/api-documentation.md
+ 32
− 4
View file @ f8ca9b98
# API Documentation for Metabase v0.21.0-snapshot # API Documentation for Metabase v0.22.0-snapshot
## `GET /api/activity/` ## `GET /api/activity/`
...@@ -104,13 +104,24 @@ Run the query associated with a Card. ...@@ -104,13 +104,24 @@ Run the query associated with a Card.
## `POST /api/card/:card-id/query/csv` ## `POST /api/card/:card-id/query/csv`
Run the query associated with a Card, and return its results as CSV. Run the query associated with a Card, and return its results as CSV. Note that this expects the parameters as serialized JSON in the 'parameters' parameter
##### PARAMS: ##### PARAMS:
* **`card-id`** * **`card-id`**
* **`parameters`** * **`parameters`** value may be nil, or if non-nil, value must be a valid JSON string.
## `POST /api/card/:card-id/query/json`
Run the query associated with a Card, and return its results as JSON. Note that this expects the parameters as serialized JSON in the 'parameters' parameter
##### PARAMS:
* **`card-id`**
* **`parameters`** value may be nil, or if non-nil, value must be a valid JSON string.
## `PUT /api/card/:id` ## `PUT /api/card/:id`
...@@ -425,6 +436,15 @@ Get historical query execution duration. ...@@ -425,6 +436,15 @@ Get historical query execution duration.
* **`query`** * **`query`**
## `POST /api/dataset/json`
Execute a query and download the result data as a JSON file.
##### PARAMS:
* **`query`** value must be a valid JSON string.
## `POST /api/email/test` ## `POST /api/email/test`
Send a test email. You must be a superuser to do this. Send a test email. You must be a superuser to do this.
...@@ -718,7 +738,7 @@ You must be a superuser to do this. ...@@ -718,7 +738,7 @@ You must be a superuser to do this.
## `GET /api/permissions/group` ## `GET /api/permissions/group`
Fetch all `PermissionsGroups`. Fetch all `PermissionsGroups`, including a count of the number of `:members` in that group.
You must be a superuser to do this. You must be a superuser to do this.
...@@ -1381,6 +1401,14 @@ Logs. ...@@ -1381,6 +1401,14 @@ Logs.
You must be a superuser to do this. You must be a superuser to do this.
## `GET /api/util/stats`
Anonymous usage stats. Endpoint for testing, and eventually exposing this to instance admins to let them see
what is being phoned home.
You must be a superuser to do this.
## `POST /api/util/password_check` ## `POST /api/util/password_check`
Endpoint that checks if the supplied password meets the currently configured password complexity rules. Endpoint that checks if the supplied password meets the currently configured password complexity rules.
......
src/metabase/api/dashboard.clj
+ 4
− 3
View file @ f8ca9b98
...@@ -17,9 +17,10 @@ ...@@ -17,9 +17,10 @@
(defn- dashboards-list [filter-option] (defn- dashboards-list [filter-option]
(filter models/can-read? (-> (db/select Dashboard {:where (case (or (keyword filter-option) :all) (filter models/can-read? (-> (db/select Dashboard {:where (case (or (keyword filter-option) :all)
:all true :all true
:mine [:= :creator_id *current-user-id*])}) :mine [:= :creator_id *current-user-id*])
:order-by [:%lower.name]})
(hydrate :creator)))) (hydrate :creator))))
(defendpoint GET "/" (defendpoint GET "/"
......
src/metabase/driver/druid/query_processor.clj
+ 29
− 28
View file @ f8ca9b98
...@@ -218,34 +218,35 @@ ...@@ -218,34 +218,35 @@
(ag:count output-name)))) (ag:count output-name))))
(defn- handle-aggregation [query-type {ag-type :aggregation-type, ag-field :field, output-name :output-name, :as ag} druid-query] (defn- handle-aggregation [query-type {ag-type :aggregation-type, ag-field :field, output-name :output-name, custom-name :custom-name, :as ag} druid-query]
(when (isa? query-type ::ag-query) (let [output-name (or custom-name output-name)]
(merge-with concat (when (isa? query-type ::ag-query)
druid-query (merge-with concat
(let [ag-type (when-not (= ag-type :rows) ag-type)] druid-query
(match [ag-type ag-field] (let [ag-type (when-not (= ag-type :rows) ag-type)]
;; For 'distinct values' queries (queries with a breakout by no aggregation) just aggregate by count, but name it :___count so it gets discarded automatically (match [ag-type ag-field]
[nil nil] {:aggregations [(ag:count (or output-name :___count))]} ;; For 'distinct values' queries (queries with a breakout by no aggregation) just aggregate by count, but name it :___count so it gets discarded automatically
[nil nil] {:aggregations [(ag:count (or output-name :___count))]}
[:count nil] {:aggregations [(ag:count (or output-name :count))]}
[:count nil] {:aggregations [(ag:count (or output-name :count))]}
[:count _] {:aggregations [(ag:count ag-field (or output-name :count))]}
[:count _] {:aggregations [(ag:count ag-field (or output-name :count))]}
[:avg _] (let [count-name (name (gensym "___count_"))
sum-name (name (gensym "___sum_"))] [:avg _] (let [count-name (name (gensym "___count_"))
{:aggregations [(ag:count ag-field count-name) sum-name (name (gensym "___sum_"))]
(ag:doubleSum ag-field sum-name)] {:aggregations [(ag:count ag-field count-name)
:postAggregations [{:type :arithmetic (ag:doubleSum ag-field sum-name)]
:name (or output-name :avg) :postAggregations [{:type :arithmetic
:fn :/ :name (or output-name :avg)
:fields [{:type :fieldAccess, :fieldName sum-name} :fn :/
{:type :fieldAccess, :fieldName count-name}]}]}) :fields [{:type :fieldAccess, :fieldName sum-name}
[:distinct _] {:aggregations [{:type :cardinality {:type :fieldAccess, :fieldName count-name}]}]})
:name (or output-name :distinct___count) [:distinct _] {:aggregations [{:type :cardinality
:fieldNames [(->rvalue ag-field)]}]} :name (or output-name :distinct___count)
[:sum _] {:aggregations [(ag:doubleSum ag-field (or output-name :sum))]} :fieldNames [(->rvalue ag-field)]}]}
[:min _] {:aggregations [(ag:doubleMin ag-field (or output-name :min))]} [:sum _] {:aggregations [(ag:doubleSum ag-field (or output-name :sum))]}
[:max _] {:aggregations [(ag:doubleMax ag-field (or output-name :max))]}))))) [:min _] {:aggregations [(ag:doubleMin ag-field (or output-name :min))]}
[:max _] {:aggregations [(ag:doubleMax ag-field (or output-name :max))]}))))))
(defn- add-expression-aggregation-output-names [args] (defn- add-expression-aggregation-output-names [args]
(for [arg args] (for [arg args]
......
src/metabase/driver/generic_sql/query_processor.clj
+ 13
− 16
View file @ f8ca9b98
...@@ -153,24 +153,21 @@ ...@@ -153,24 +153,21 @@
(h/merge-select honeysql-form [(expression-aggregation->honeysql driver expression) (h/merge-select honeysql-form [(expression-aggregation->honeysql driver expression)
(hx/escape-dots (annotate/expression-aggregation-name expression))])) (hx/escape-dots (annotate/expression-aggregation-name expression))]))
(defn- apply-single-aggregation [driver honeysql-form {:keys [aggregation-type field], :as aggregation}]
(h/merge-select honeysql-form [(aggregation->honeysql driver aggregation-type field)
(hx/escape-dots (annotate/expression-aggregation-name aggregation))]))
(defn apply-aggregation (defn apply-aggregation
"Apply a `aggregation` clauses to HONEYSQL-FORM. Default implementation of `apply-aggregation` for SQL drivers." "Apply a `aggregation` clauses to HONEYSQL-FORM. Default implementation of `apply-aggregation` for SQL drivers."
([driver honeysql-form {aggregations :aggregation}] [driver honeysql-form {aggregations :aggregation}]
(loop [form honeysql-form, [ag & more] aggregations] (loop [form honeysql-form, [ag & more] aggregations]
(let [form (if (instance? Expression ag) (let [form (if (instance? Expression ag)
(apply-expression-aggregation driver form ag) (apply-expression-aggregation driver form ag)
(let [{:keys [aggregation-type field]} ag] (apply-single-aggregation driver form ag))]
(apply-aggregation driver form aggregation-type field)))] (if-not (seq more)
(if-not (seq more) form
form (recur form more)))))
(recur form more)))))
([driver honeysql-form aggregation-type field]
(h/merge-select honeysql-form [(aggregation->honeysql driver aggregation-type field)
;; the column alias is always the same as the ag type except for `:distinct` with is called `:count` (WHY?)
(if (= aggregation-type :distinct)
:count
aggregation-type)])))
(defn apply-breakout (defn apply-breakout
"Apply a `breakout` clause to HONEYSQL-FORM. Default implementation of `apply-breakout` for SQL drivers." "Apply a `breakout` clause to HONEYSQL-FORM. Default implementation of `apply-breakout` for SQL drivers."
......
src/metabase/query_processor/annotate.clj
+ 10
− 4
View file @ f8ca9b98
...@@ -94,18 +94,24 @@ ...@@ -94,18 +94,24 @@
:count :count
ag-type)) ag-type))
;; TODO - rename to something like `aggregation-name` or `aggregation-subclause-name` now that this handles any sort of aggregation
(defn expression-aggregation-name (defn expression-aggregation-name
"Return an appropriate name for an expression aggregation, e.g. `sum + count`." "Return an appropriate name for an `:aggregation` subclause (an aggregation or expression)."
^String [ag] ^String [{custom-name :custom-name, aggregation-type :aggregation-type, :as ag}]
(cond (cond
;; if a custom name was provided use it
custom-name custom-name
;; for unnamed expressions, just compute a name like "sum + count"
(instance? Expression ag) (let [{:keys [operator args]} ag] (instance? Expression ag) (let [{:keys [operator args]} ag]
(str/join (str " " (name operator) " ") (str/join (str " " (name operator) " ")
(for [arg args] (for [arg args]
(if (instance? Expression arg) (if (instance? Expression arg)
(str "(" (expression-aggregation-name arg) ")") (str "(" (expression-aggregation-name arg) ")")
(expression-aggregation-name arg))))) (expression-aggregation-name arg)))))
(:aggregation-type ag) (name (:aggregation-type ag)) ;; for unnamed normal aggregations, the column alias is always the same as the ag type except for `:distinct` with is called `:count` (WHY?)
:else ag)) aggregation-type (if (= (keyword aggregation-type) :distinct)
"count"
(name aggregation-type))))
(defn- expression-aggregate-field-info [expression] (defn- expression-aggregate-field-info [expression]
(let [ag-name (expression-aggregation-name expression)] (let [ag-name (expression-aggregation-name expression)]
......
src/metabase/query_processor/expand.clj
+ 15
− 8
View file @ f8ca9b98
...@@ -59,6 +59,13 @@ ...@@ -59,6 +59,13 @@
(field-id f)) (field-id f))
f)) f))
(s/defn ^:ql ^:always-validate named :- i/Aggregation
"Specify a CUSTOM-NAME to use for a top-level AGGREGATION-OR-EXPRESSION in the results.
(This will probably be extended to support Fields in the future, but for now, only the `:aggregation` clause is supported.)"
{:added "0.22.0"}
[aggregation-or-expression :- i/Aggregation, custom-name :- su/NonBlankString]
(assoc aggregation-or-expression :custom-name custom-name))
(s/defn ^:ql ^:always-validate datetime-field :- FieldPlaceholder (s/defn ^:ql ^:always-validate datetime-field :- FieldPlaceholder
"Reference to a `DateTimeField`. This is just a `Field` reference with an associated datetime UNIT." "Reference to a `DateTimeField`. This is just a `Field` reference with an associated datetime UNIT."
([f _ unit] (log/warn (u/format-color 'yellow (str "The syntax for datetime-field has changed in MBQL '98. [:datetime-field <field> :as <unit>] is deprecated. " ([f _ unit] (log/warn (u/format-color 'yellow (str "The syntax for datetime-field has changed in MBQL '98. [:datetime-field <field> :as <unit>] is deprecated. "
...@@ -122,11 +129,11 @@ ...@@ -122,11 +129,11 @@
(if (number? arg) (if (number? arg)
arg arg
(field-or-expression arg)))) (field-or-expression arg))))
;; otherwise if it's not an Expression it's a a ;; otherwise if it's not an Expression it's a Field
(field f))) (field f)))
(s/defn ^:private ^:always-validate ag-with-field :- i/Aggregation [ag-type f] (s/defn ^:private ^:always-validate ag-with-field :- i/Aggregation [ag-type f]
(i/strict-map->AggregationWithField {:aggregation-type ag-type, :field (field-or-expression f)})) (i/map->AggregationWithField {:aggregation-type ag-type, :field (field-or-expression f)}))
(def ^:ql ^{:arglists '([f])} avg "Aggregation clause. Return the average value of F." (partial ag-with-field :avg)) (def ^:ql ^{:arglists '([f])} avg "Aggregation clause. Return the average value of F." (partial ag-with-field :avg))
(def ^:ql ^{:arglists '([f])} distinct "Aggregation clause. Return the number of distinct values of F." (partial ag-with-field :distinct)) (def ^:ql ^{:arglists '([f])} distinct "Aggregation clause. Return the number of distinct values of F." (partial ag-with-field :distinct))
...@@ -144,13 +151,13 @@ ...@@ -144,13 +151,13 @@
(s/defn ^:ql ^:always-validate count :- i/Aggregation (s/defn ^:ql ^:always-validate count :- i/Aggregation
"Aggregation clause. Return total row count (e.g., `COUNT(*)`). If F is specified, only count rows where F is non-null (e.g. `COUNT(f)`)." "Aggregation clause. Return total row count (e.g., `COUNT(*)`). If F is specified, only count rows where F is non-null (e.g. `COUNT(f)`)."
([] (i/strict-map->AggregationWithoutField {:aggregation-type :count})) ([] (i/map->AggregationWithoutField {:aggregation-type :count}))
([f] (ag-with-field :count f))) ([f] (ag-with-field :count f)))
(s/defn ^:ql ^:always-validate cum-count :- i/Aggregation (s/defn ^:ql ^:always-validate cum-count :- i/Aggregation
"Aggregation clause. Return the cumulative row count (presumably broken out in some way)." "Aggregation clause. Return the cumulative row count (presumably broken out in some way)."
[] []
(i/strict-map->AggregationWithoutField {:aggregation-type :cumulative-count})) (i/map->AggregationWithoutField {:aggregation-type :cumulative-count}))
(defn ^:ql ^:deprecated rows (defn ^:ql ^:deprecated rows
"Bare rows aggregation. This is the default behavior, so specifying it is deprecated." "Bare rows aggregation. This is the default behavior, so specifying it is deprecated."
...@@ -405,10 +412,10 @@ ...@@ -405,10 +412,10 @@
(s/defn ^:private ^:always-validate expression-fn :- Expression (s/defn ^:private ^:always-validate expression-fn :- Expression
[k :- s/Keyword, & args] [k :- s/Keyword, & args]
(i/strict-map->Expression {:operator k, :args (vec (for [arg args] (i/map->Expression {:operator k, :args (vec (for [arg args]
(if (number? arg) (if (number? arg)
(float arg) ; convert args to floats so things like 5 / 10 -> 0.5 instead of 0 (float arg) ; convert args to floats so things like 5 / 10 -> 0.5 instead of 0
arg)))})) arg)))}))
(def ^:ql ^{:arglists '([rvalue1 rvalue2 & more]), :added "0.17.0"} + "Arithmetic addition function." (partial expression-fn :+)) (def ^:ql ^{:arglists '([rvalue1 rvalue2 & more]), :added "0.17.0"} + "Arithmetic addition function." (partial expression-fn :+))
(def ^:ql ^{:arglists '([rvalue1 rvalue2 & more]), :added "0.17.0"} - "Arithmetic subtraction function." (partial expression-fn :-)) (def ^:ql ^{:arglists '([rvalue1 rvalue2 & more]), :added "0.17.0"} - "Arithmetic subtraction function." (partial expression-fn :-))
......
src/metabase/query_processor/interface.clj
+ 8
− 5
View file @ f8ca9b98
...@@ -181,9 +181,10 @@ ...@@ -181,9 +181,10 @@
(def ^:private ExpressionOperator (s/named (s/enum :+ :- :* :/) "Valid expression operator")) (def ^:private ExpressionOperator (s/named (s/enum :+ :- :* :/) "Valid expression operator"))
(s/defrecord Expression [operator :- ExpressionOperator (s/defrecord Expression [operator :- ExpressionOperator
args :- [(s/cond-pre (s/recursive #'RValue) args :- [(s/cond-pre (s/recursive #'RValue)
(s/recursive #'Aggregation))]]) (s/recursive #'Aggregation))]
custom-name :- (s/maybe su/NonBlankString)])
(def AnyFieldOrExpression (def AnyFieldOrExpression
"Schema for a `FieldPlaceholder`, `AgRef`, or `Expression`." "Schema for a `FieldPlaceholder`, `AgRef`, or `Expression`."
...@@ -241,12 +242,14 @@ ...@@ -241,12 +242,14 @@
;;; # ------------------------------------------------------------ CLAUSE SCHEMAS ------------------------------------------------------------ ;;; # ------------------------------------------------------------ CLAUSE SCHEMAS ------------------------------------------------------------
(s/defrecord AggregationWithoutField [aggregation-type :- (s/named (s/enum :count :cumulative-count) (s/defrecord AggregationWithoutField [aggregation-type :- (s/named (s/enum :count :cumulative-count)
"Valid aggregation type")]) "Valid aggregation type")
custom-name :- (s/maybe su/NonBlankString)])
(s/defrecord AggregationWithField [aggregation-type :- (s/named (s/enum :avg :count :cumulative-sum :distinct :max :min :stddev :sum) (s/defrecord AggregationWithField [aggregation-type :- (s/named (s/enum :avg :count :cumulative-sum :distinct :max :min :stddev :sum)
"Valid aggregation type") "Valid aggregation type")
field :- (s/cond-pre FieldPlaceholderOrExpressionRef field :- (s/cond-pre FieldPlaceholderOrExpressionRef
Expression)]) Expression)
custom-name :- (s/maybe su/NonBlankString)])
(defn- valid-aggregation-for-driver? [{:keys [aggregation-type]}] (defn- valid-aggregation-for-driver? [{:keys [aggregation-type]}]
(when (= aggregation-type :stddev) (when (= aggregation-type :stddev)
......
src/metabase/query_processor/macros.clj
+ 44
− 38
View file @ f8ca9b98
(ns metabase.query-processor.macros (ns metabase.query-processor.macros
"TODO - this namespace is ancient and written with MBQL '95 in mind, e.g. it is case-sensitive.
At some point this ought to be reworked to be case-insensitive and cleaned up."
(:require [clojure.core.match :refer [match]] (:require [clojure.core.match :refer [match]]
[clojure.walk :as walk]
[metabase.db :as db] [metabase.db :as db]
[metabase.util :as u])) [metabase.util :as u]))
...@@ -18,6 +21,9 @@ ...@@ -18,6 +21,9 @@
~@match-forms ~@match-forms
form# (throw (Exception. (format ~(format "%s failed: invalid clause: %%s" fn-name) form#))))))) form# (throw (Exception. (format ~(format "%s failed: invalid clause: %%s" fn-name) form#)))))))
;;; ------------------------------------------------------------ Segments ------------------------------------------------------------
(defparser segment-parse-filter-subclause (defparser segment-parse-filter-subclause
["SEGMENT" (segment-id :guard integer?)] (:filter (db/select-one-field :definition 'Segment, :id segment-id)) ["SEGMENT" (segment-id :guard integer?)] (:filter (db/select-one-field :definition 'Segment, :id segment-id))
subclause subclause) subclause subclause)
...@@ -40,50 +46,50 @@ ...@@ -40,50 +46,50 @@
(seq addtl) addtl (seq addtl) addtl
:else [])) :else []))
(defn- merge-aggregation [aggregations new-ag]
(if (map? aggregations)
(recur [aggregations] new-ag)
(conj aggregations new-ag)))
(defn- merge-aggregations {:style/indent 0} [query-dict [aggregation & more]] ;;; ------------------------------------------------------------ Metrics ------------------------------------------------------------
(if-not aggregation
;; no more aggregations? we're done (defn- metric? [aggregation]
query-dict (match aggregation
;; otherwise determine if this aggregation is a METRIC and recur ["METRIC" (_ :guard integer?)] true
(let [metric-def (match aggregation _ false))
["METRIC" (metric-id :guard integer?)] (db/select-one-field :definition 'Metric, :id metric-id)
_ nil)] (defn- metric-id [metric]
(recur (if-not metric-def (when (metric? metric)
;; not a metric, move to next aggregation (second metric)))
query-dict
;; it *is* a metric, insert it into the query appropriately (defn- expand-metric [metric-clause filter-clauses-atom]
(-> query-dict (let [{filter-clause :filter, ag-clause :aggregation} (db/select-one-field :definition 'Metric, :id (metric-id metric-clause))]
(update-in [:query :aggregation] merge-aggregation (:aggregation metric-def)) (when filter-clause
(update-in [:query :filter] merge-filter-clauses (:filter metric-def)))) (swap! filter-clauses-atom conj filter-clause))
more)))) ag-clause))
(defn- remove-metrics [aggregations] (defn- expand-metrics-in-ag-clause [query-dict filter-clauses-atom]
(if-not (and (sequential? aggregations) (walk/postwalk (fn [form]
(every? coll? aggregations)) (if-not (metric? form)
(recur [aggregations]) form
(vec (for [ag aggregations (expand-metric form filter-clauses-atom)))
:when (match ag query-dict))
["METRIC" (_ :guard integer?)] false
_ true)] (defn- add-metrics-filter-clauses [query-dict filter-clauses]
ag)))) (update-in query-dict [:query :filter] merge-filter-clauses (if (> (count filter-clauses) 1)
(cons "AND" filter-clauses)
(first filter-clauses))))
(defn- expand-metrics [query-dict]
(let [filter-clauses-atom (atom [])
query-dict (expand-metrics-in-ag-clause query-dict filter-clauses-atom)]
(add-metrics-filter-clauses query-dict @filter-clauses-atom)))
(defn- macroexpand-metric [{{aggregations :aggregation} :query, :as query-dict}] (defn- macroexpand-metric [{{aggregations :aggregation} :query, :as query-dict}]
(if-not (seq aggregations) (if-not (seq aggregations)
;; :aggregation is empty, so no METRIC to expand ;; :aggregation is empty, so no METRIC to expand
query-dict query-dict
;; we have an aggregation clause, so lets see if we are using a METRIC ;; otherwise walk the query dict and expand METRIC clauses
;; (since `:aggregation` can be either single or multiple, wrap single ones so `merge-aggregations` can always assume input is multiple) (expand-metrics query-dict)))
(merge-aggregations
(update-in query-dict [:query :aggregation] remove-metrics)
(if (and (sequential? aggregations) ;;; ------------------------------------------------------------ Middleware ------------------------------------------------------------
(every? coll? aggregations))
aggregations
[aggregations]))))
(defn expand-macros "Expand the macros (SEGMENT, METRIC) in a QUERY-DICT." (defn expand-macros "Expand the macros (SEGMENT, METRIC) in a QUERY-DICT."
[query-dict] [query-dict]
......
test/metabase/api/dataset_test.clj
+ 2
− 2
View file @ f8ca9b98
...@@ -64,7 +64,7 @@ ...@@ -64,7 +64,7 @@
(query checkins (query checkins
(ql/aggregation (ql/count)))) (ql/aggregation (ql/count))))
(assoc :type "query") (assoc :type "query")
(assoc-in [:query :aggregation] [{:aggregation-type "count"}]) (assoc-in [:query :aggregation] [{:aggregation-type "count", :custom-name nil}])
(assoc :constraints query-constraints)) (assoc :constraints query-constraints))
:started_at true :started_at true
:finished_at true :finished_at true
...@@ -80,7 +80,7 @@ ...@@ -80,7 +80,7 @@
(query checkins (query checkins
(ql/aggregation (ql/count)))) (ql/aggregation (ql/count))))
(assoc :type "query") (assoc :type "query")
(assoc-in [:query :aggregation] [{:aggregation-type "count"}]) (assoc-in [:query :aggregation] [{:aggregation-type "count", :custom-name nil}])
(assoc :constraints query-constraints)) (assoc :constraints query-constraints))
:started_at true :started_at true
:finished_at true :finished_at true
......
test/metabase/driver/druid_test.clj
+ 52
− 7
View file @ f8ca9b98
(ns metabase.driver.druid-test (ns metabase.driver.druid-test
(:require [cheshire.core :as json] (:require [cheshire.core :as json]
[expectations :refer :all] [expectations :refer :all]
[metabase.models.metric :refer [Metric]]
[metabase.query-processor :as qp] [metabase.query-processor :as qp]
[metabase.query-processor.expand :as ql] [metabase.query-processor.expand :as ql]
[metabase.query-processor-test :refer [rows]] [metabase.query-processor-test :refer [rows rows+column-names]]
[metabase.test.data :as data] [metabase.test.data :as data]
[metabase.test.data.datasets :as datasets, :refer [expect-with-engine]] [metabase.test.data.datasets :as datasets, :refer [expect-with-engine]]
[metabase.test.util :as tu]
[metabase.timeseries-query-processor-test :as timeseries-qp-test] [metabase.timeseries-query-processor-test :as timeseries-qp-test]
[metabase.query :as q])) [metabase.util :as u]))
(def ^:const ^:private ^String native-query-1 (def ^:const ^:private ^String native-query-1
(json/generate-string (json/generate-string
...@@ -68,12 +70,15 @@ ...@@ -68,12 +70,15 @@
;;; | EXPRESSION AGGREGATIONS | ;;; | EXPRESSION AGGREGATIONS |
;;; +------------------------------------------------------------------------------------------------------------------------+ ;;; +------------------------------------------------------------------------------------------------------------------------+
(defmacro ^:private druid-query {:style/indent 0} [& body]
`(timeseries-qp-test/with-flattened-dbdef
(qp/process-query {:database (data/id)
:type :query
:query (data/query ~'checkins
~@body)})))
(defmacro ^:private druid-query-returning-rows {:style/indent 0} [& body] (defmacro ^:private druid-query-returning-rows {:style/indent 0} [& body]
`(rows (timeseries-qp-test/with-flattened-dbdef `(rows (druid-query ~@body)))
(qp/process-query {:database (data/id)
:type :query
:query (data/query ~'checkins
~@body)}))))
;; sum, * ;; sum, *
(expect-with-engine :druid (expect-with-engine :druid
...@@ -202,3 +207,43 @@ ...@@ -202,3 +207,43 @@
(druid-query-returning-rows (druid-query-returning-rows
(ql/aggregation (ql/sum (ql/+ $venue_price 1))) (ql/aggregation (ql/sum (ql/+ $venue_price 1)))
(ql/breakout $venue_price))) (ql/breakout $venue_price)))
;; check that we can name an expression aggregation w/ aggregation at top-level
(expect-with-engine :druid
{:rows [["1" 442.0]
["2" 1845.0]
["3" 460.0]
["4" 245.0]]
:columns ["venue_price"
"New Price"]}
(rows+column-names
(druid-query
(ql/aggregation (ql/named (ql/sum (ql/+ $venue_price 1)) "New Price"))
(ql/breakout $venue_price))))
;; check that we can name an expression aggregation w/ expression at top-level
(expect-with-engine :druid
{:rows [["1" 180.0]
["2" 1189.0]
["3" 304.0]
["4" 155.0]]
:columns ["venue_price" "Sum-41"]}
(rows+column-names
(druid-query
(ql/aggregation (ql/named (ql/- (ql/sum $venue_price) 41) "Sum-41"))
(ql/breakout $venue_price))))
;; check that we can handle METRICS (ick) inside expression aggregation clauses
(expect-with-engine :druid
[["2" 1231.0]
["3" 346.0]
["4" 197.0]]
(timeseries-qp-test/with-flattened-dbdef
(tu/with-temp Metric [metric {:definition {:aggregation [:sum [:field-id (data/id :checkins :venue_price)]]
:filter [:> [:field-id (data/id :checkins :venue_price)] 1]}}]
(rows (qp/process-query
{:database (data/id)
:type :query
:query {:source-table (data/id :checkins)
:aggregation [:+ ["METRIC" (u/get-id metric)] 1]
:breakout [(ql/breakout (ql/field-id (data/id :checkins :venue_price)))]}})))))
test/metabase/events/activity_feed_test.clj
+ 3
− 3
View file @ f8ca9b98
...@@ -271,7 +271,7 @@ ...@@ -271,7 +271,7 @@
:model "segment" :model "segment"
:model_id (:id segment) :model_id (:id segment)
:database_id (id) :database_id (id)
:table_id (id :venues) :table_id (id :checkins)
:details {:name (:name segment) :details {:name (:name segment)
:description (:description segment)}} :description (:description segment)}}
(with-temp-activities (with-temp-activities
...@@ -288,7 +288,7 @@ ...@@ -288,7 +288,7 @@
:model "segment" :model "segment"
:model_id (:id segment) :model_id (:id segment)
:database_id (id) :database_id (id)
:table_id (id :venues) :table_id (id :checkins)
:details {:name (:name segment) :details {:name (:name segment)
:description (:description segment) :description (:description segment)
:revision_message "update this mofo"}} :revision_message "update this mofo"}}
...@@ -310,7 +310,7 @@ ...@@ -310,7 +310,7 @@
:model "segment" :model "segment"
:model_id (:id segment) :model_id (:id segment)
:database_id (id) :database_id (id)
:table_id (id :venues) :table_id (id :checkins)
:details {:name (:name segment) :details {:name (:name segment)
:description (:description segment) :description (:description segment)
:revision_message "deleted"}} :revision_message "deleted"}}
......
test/metabase/query_processor/expand_resolve_test.clj
+ 21
− 19
View file @ f8ca9b98
...@@ -222,9 +222,10 @@ ...@@ -222,9 +222,10 @@
:type :query :type :query
:query {:source-table (id :checkins) :query {:source-table (id :checkins)
:aggregation [{:aggregation-type :sum :aggregation [{:aggregation-type :sum
:field {:field-id (id :venues :price) :custom-name nil
:fk-field-id (id :checkins :venue_id) :field {:field-id (id :venues :price)
:datetime-unit nil}}] :fk-field-id (id :checkins :venue_id)
:datetime-unit nil}}]
:breakout [{:field-id (id :checkins :date) :breakout [{:field-id (id :checkins :date)
:fk-field-id nil :fk-field-id nil
:datetime-unit :day-of-week}]}} :datetime-unit :day-of-week}]}}
...@@ -235,20 +236,21 @@ ...@@ -235,20 +236,21 @@
:name "CHECKINS" :name "CHECKINS"
:id (id :checkins)} :id (id :checkins)}
:aggregation [{:aggregation-type :sum :aggregation [{:aggregation-type :sum
:field {:description nil :custom-name nil
:base-type :type/Integer :field {:description nil
:parent nil :base-type :type/Integer
:table-id (id :venues) :parent nil
:special-type :type/Category :table-id (id :venues)
:field-name "PRICE" :special-type :type/Category
:field-display-name "Price" :field-name "PRICE"
:parent-id nil :field-display-name "Price"
:visibility-type :normal :parent-id nil
:position nil :visibility-type :normal
:field-id (id :venues :price) :position nil
:fk-field-id (id :checkins :venue_id) :field-id (id :venues :price)
:table-name "VENUES__via__VENUE_ID" :fk-field-id (id :checkins :venue_id)
:schema-name nil}}] :table-name "VENUES__via__VENUE_ID"
:schema-name nil}}]
:breakout [{:field {:description nil :breakout [{:field {:description nil
:base-type :type/Date :base-type :type/Date
:parent nil :parent nil
...@@ -275,7 +277,7 @@ ...@@ -275,7 +277,7 @@
:fk-field-ids #{(id :checkins :venue_id)} :fk-field-ids #{(id :checkins :venue_id)}
:table-ids #{(id :venues) (id :checkins)}}] :table-ids #{(id :venues) (id :checkins)}}]
(let [expanded-form (ql/expand (wrap-inner-query (query checkins (let [expanded-form (ql/expand (wrap-inner-query (query checkins
(ql/aggregation (ql/sum $venue_id->venues.price)) (ql/aggregation (ql/sum $venue_id->venues.price))
(ql/breakout (ql/datetime-field $checkins.date :day-of-week)))))] (ql/breakout (ql/datetime-field $checkins.date :day-of-week)))))]
(mapv obj->map [expanded-form (mapv obj->map [expanded-form
(resolve/resolve expanded-form)]))) (resolve/resolve expanded-form)])))
test/metabase/query_processor/macros_test.clj
+ 11
− 8
View file @ f8ca9b98
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
(expect (expect
{:database 1 {:database 1
:type :query :type :query
:query {:aggregation [["rows"]] :query {:aggregation ["rows"]
:filter ["AND" [">" 4 1]] :filter ["AND" [">" 4 1]]
:breakout [17]}} :breakout [17]}}
(expand-macros {:database 1 (expand-macros {:database 1
...@@ -27,8 +27,10 @@ ...@@ -27,8 +27,10 @@
(expect (expect
{:database 1 {:database 1
:type :query :type :query
:query {:aggregation [["rows"]] :query {:aggregation ["rows"]
:filter ["AND" ["AND" ["=" 5 "abc"]] ["OR" ["AND" ["IS_NULL" 7]] [">" 4 1]]] :filter ["AND" ["AND" ["=" 5 "abc"]]
["OR" ["AND" ["IS_NULL" 7]]
[">" 4 1]]]
:breakout [17]}} :breakout [17]}}
(tu/with-temp* [Database [{database-id :id}] (tu/with-temp* [Database [{database-id :id}]
Table [{table-id :id} {:db_id database-id}] Table [{table-id :id} {:db_id database-id}]
...@@ -46,8 +48,9 @@ ...@@ -46,8 +48,9 @@
(expect (expect
{:database 1 {:database 1
:type :query :type :query
:query {:aggregation [["count"]] :query {:aggregation ["count"]
:filter ["AND" ["AND" [">" 4 1]] ["AND" ["=" 5 "abc"]]] :filter ["AND" ["AND" [">" 4 1]]
["AND" ["=" 5 "abc"]]]
:breakout [17] :breakout [17]
:order_by [[1 "ASC"]]}} :order_by [[1 "ASC"]]}}
(tu/with-temp* [Database [{database-id :id}] (tu/with-temp* [Database [{database-id :id}]
...@@ -66,7 +69,7 @@ ...@@ -66,7 +69,7 @@
(expect (expect
{:database 1 {:database 1
:type :query :type :query
:query {:aggregation [["count"]] :query {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]] :filter ["AND" ["=" 5 "abc"]]
:breakout [17] :breakout [17]
:order_by [[1 "ASC"]]}} :order_by [[1 "ASC"]]}}
...@@ -86,7 +89,7 @@ ...@@ -86,7 +89,7 @@
(expect (expect
{:database 1 {:database 1
:type :query :type :query
:query {:aggregation [["count"]] :query {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]] :filter ["AND" ["=" 5 "abc"]]
:breakout [17] :breakout [17]
:order_by [[1 "ASC"]]}} :order_by [[1 "ASC"]]}}
...@@ -105,7 +108,7 @@ ...@@ -105,7 +108,7 @@
(expect (expect
{:database 1 {:database 1
:type :query :type :query
:query {:aggregation [["sum" 18]] :query {:aggregation ["sum" 18]
:filter ["AND" ["AND" [">" 4 1] ["AND" ["IS_NULL" 7]]] ["AND" ["=" 5 "abc"] ["AND" ["BETWEEN" 9 0 25]]]] :filter ["AND" ["AND" [">" 4 1] ["AND" ["IS_NULL" 7]]] ["AND" ["=" 5 "abc"] ["AND" ["BETWEEN" 9 0 25]]]]
:breakout [17] :breakout [17]
:order_by [[1 "ASC"]]}} :order_by [[1 "ASC"]]}}
......
test/metabase/query_processor_test.clj
+ 10
− 3
View file @ f8ca9b98
...@@ -288,15 +288,22 @@ ...@@ -288,15 +288,22 @@
(defn rows (defn rows
"Return the result rows from query results, or throw an Exception if they're missing." "Return the result rows from query RESULTS, or throw an Exception if they're missing."
{:style/indent 0} {:style/indent 0}
[results] [results]
(vec (or (-> results :data :rows) (vec (or (get-in results [:data :rows])
(println (u/pprint-to-str 'red results)) (println (u/pprint-to-str 'red results))
(throw (Exception. "Error!"))))) (throw (Exception. "Error!")))))
(defn rows+column-names
"Return the result rows and column names from query RESULTS, or throw an Exception if they're missing."
{:style/indent 0}
[results]
{:rows (rows results)
:columns (get-in results [:data :columns])})
(defn first-row (defn first-row
"Return the first row in the results of a query, or throw an Exception if they're missing." "Return the first row in the RESULTS of a query, or throw an Exception if they're missing."
{:style/indent 0} {:style/indent 0}
[results] [results]
(first (rows results))) (first (rows results)))
test/metabase/query_processor_test/expression_aggregations_test.clj
+ 45
− 0
View file @ f8ca9b98
(ns metabase.query-processor-test.expression-aggregations-test (ns metabase.query-processor-test.expression-aggregations-test
"Tests for expression aggregations." "Tests for expression aggregations."
(:require [expectations :refer :all] (:require [expectations :refer :all]
[metabase.models.metric :refer [Metric]]
[metabase.query-processor :as qp]
[metabase.query-processor.expand :as ql] [metabase.query-processor.expand :as ql]
[metabase.query-processor-test :refer :all] [metabase.query-processor-test :refer :all]
[metabase.test.data :as data] [metabase.test.data :as data]
[metabase.test.data.datasets :as datasets, :refer [*engine*]] [metabase.test.data.datasets :as datasets, :refer [*engine*]]
[metabase.test.util :as tu]
[metabase.util :as u])) [metabase.util :as u]))
;; sum, * ;; sum, *
...@@ -156,3 +159,45 @@ ...@@ -156,3 +159,45 @@
(rows (data/run-query venues (rows (data/run-query venues
(ql/aggregation (ql/sum (ql/+ $price 1))) (ql/aggregation (ql/sum (ql/+ $price 1)))
(ql/breakout $price))))) (ql/breakout $price)))))
;; check that we can name an expression aggregation w/ aggregation at top-level
(datasets/expect-with-engines (engines-that-support :expression-aggregations)
{:rows [[1 44]
[2 177]
[3 52]
[4 30]]
:columns [(data/format-name "price")
(if (= *engine* :redshift) "new price" "New Price")]} ; Redshift annoyingly always lowercases column aliases
(format-rows-by [int int]
(rows+column-names (data/run-query venues
(ql/aggregation (ql/named (ql/sum (ql/+ $price 1)) "New Price"))
(ql/breakout $price)))))
;; check that we can name an expression aggregation w/ expression at top-level
(datasets/expect-with-engines (engines-that-support :expression-aggregations)
{:rows [[1 -19]
[2 77]
[3 -2]
[4 -17]]
:columns [(data/format-name "price")
(if (= *engine* :redshift) "sum-41" "Sum-41")]}
(format-rows-by [int int]
(rows+column-names (data/run-query venues
(ql/aggregation (ql/named (ql/- (ql/sum $price) 41) "Sum-41"))
(ql/breakout $price)))))
;; check that we can handle METRICS (ick) inside expression aggregation clauses
(datasets/expect-with-engines (engines-that-support :expression-aggregations)
[[2 119]
[3 40]
[4 25]]
(tu/with-temp Metric [metric {:table_id (data/id :venues)
:definition {:aggregation [:sum [:field-id (data/id :venues :price)]]
:filter [:> [:field-id (data/id :venues :price)] 1]}}]
(format-rows-by [int int]
(rows (qp/process-query
{:database (data/id)
:type :query
:query {:source-table (data/id :venues)
:aggregation [:+ ["METRIC" (u/get-id metric)] 1]
:breakout [(ql/breakout (ql/field-id (data/id :venues :price)))]}})))))
test/metabase/test/data.clj
+ 1
− 0
View file @ f8ca9b98
...@@ -105,6 +105,7 @@ ...@@ -105,6 +105,7 @@
"Call `driver/process-query` on expanded inner QUERY, looking up the `Database` ID for the `source-table.` "Call `driver/process-query` on expanded inner QUERY, looking up the `Database` ID for the `source-table.`
(run-query* (query (source-table 5) ...))" (run-query* (query (source-table 5) ...))"
{:style/indent 0}
[query :- qi/Query] [query :- qi/Query]
(qp/process-query (wrap-inner-query query))) (qp/process-query (wrap-inner-query query)))
......
test/metabase/test/util.clj
+ 3
− 3
View file @ f8ca9b98
...@@ -124,7 +124,7 @@ ...@@ -124,7 +124,7 @@
{:with-temp-defaults (fn [_] {:base_type :type/Text {:with-temp-defaults (fn [_] {:base_type :type/Text
:name (random-name) :name (random-name)
:position 1 :position 1
:table_id (data/id :venues)})}) :table_id (data/id :checkins)})})
(u/strict-extend (class Metric) (u/strict-extend (class Metric)
WithTempDefaults WithTempDefaults
...@@ -132,7 +132,7 @@ ...@@ -132,7 +132,7 @@
:definition {} :definition {}
:description "Lookin' for a blueberry" :description "Lookin' for a blueberry"
:name "Toucans in the rainforest" :name "Toucans in the rainforest"
:table_id (data/id :venues)})}) :table_id (data/id :checkins)})})
(u/strict-extend (class PermissionsGroup) (u/strict-extend (class PermissionsGroup)
WithTempDefaults WithTempDefaults
...@@ -172,7 +172,7 @@ ...@@ -172,7 +172,7 @@
:definition {} :definition {}
:description "Lookin' for a blueberry" :description "Lookin' for a blueberry"
:name "Toucans in the rainforest" :name "Toucans in the rainforest"
:table_id (data/id :venues)})}) :table_id (data/id :checkins)})})
;; TODO - `with-temp` doesn't return `Sessions`, probably because their ID is a string? ;; TODO - `with-temp` doesn't return `Sessions`, probably because their ID is a string?
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment