diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index adc204a0be246495cfec6288fd311b5ed23e15c7..c2e2a1a2887dca4ef1bd1d596f9743bfd521f877 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -135,7 +135,8 @@
* `:standard-deviation-aggregations` - Does this driver support [standard deviation aggregations](https://github.com/metabase/metabase/wiki/Query-Language-'98#stddev-aggregation)?
* `:expressions` - Does this driver support [expressions](https://github.com/metabase/metabase/wiki/Query-Language-'98#expressions) (e.g. adding the values of 2 columns together)?
* `:dynamic-schema` - Does this Database have no fixed definitions of schemas? (e.g. Mongo)
- * `:native-parameters` - Does the driver support parameter substitution on native queries?")
+ * `:native-parameters` - Does the driver support parameter substitution on native queries?
+ * `:expression-aggregations` - Does the driver support using expressions inside aggregations? e.g. something like \"sum(x) + count(y)\" or \"avg(x + y)\"")
(field-values-lazy-seq ^clojure.lang.Sequential [this, ^FieldInstance field]
"Return a lazy sequence of all values of FIELD.
diff --git a/src/metabase/driver/druid.clj b/src/metabase/driver/druid.clj
index 5ad989640e911e78139aeec0f8470400988f8e37..6f652e66cf5b1a5e6bcb642b4b3c152275923c38 100644
--- a/src/metabase/driver/druid.clj
+++ b/src/metabase/driver/druid.clj
@@ -158,7 +158,7 @@
:type :integer
:default 8082}])
:execute-query (fn [_ query] (qp/execute-query do-query query))
- :features (constantly #{:basic-aggregations :set-timezone})
+ :features (constantly #{:basic-aggregations :set-timezone :expression-aggregations})
:field-values-lazy-seq (u/drop-first-arg field-values-lazy-seq)
:mbql->native (u/drop-first-arg qp/mbql->native)}))
diff --git a/src/metabase/driver/druid/js.clj b/src/metabase/driver/druid/js.clj
new file mode 100644
index 0000000000000000000000000000000000000000..5b55efc48b39e6125aaaa847fbb78d4ad4dffb7e
--- /dev/null
+++ b/src/metabase/driver/druid/js.clj
@@ -0,0 +1,68 @@
+(ns metabase.driver.druid.js
+ "Util fns for creating Javascript functions."
+ (:refer-clojure :exclude [+ - * / or])
+ (:require [clojure.string :as s]))
+
+(defn- ->js [x]
+ {:pre [(not (coll? x))]}
+ (if (keyword? x)
+ (name x)
+ x))
+
+(defn parens
+ "Wrap S (presumably a string) in parens."
+ ^String [s]
+ (str "(" s ")"))
+
+(defn argslist
+ "Generate a list of arguments, e.g. for a function declaration or function call.
+ ARGS are separated by commas and wrapped in parens.
+
+ (argslist [:x :y]) -> \"(x, y)\""
+ ^String [args]
+ (parens (s/join ", " (map ->js args))))
+
+(defn return
+ "Generate a javascript `return` statement. STATEMENT-PARTS are combined directly into a single string.
+
+ (return :x :+ :y) -> \"return x+y;\""
+ ^String [& statement-parts]
+ (str "return " (apply str (map ->js statement-parts)) ";"))
+
+(defn function
+ "Create a JavaScript function with ARGS and BODY."
+ {:style/indent 1}
+ ^String [args & body]
+ (str "function" (argslist args) " { "
+ (apply str body)
+ " }"))
+
+(defn- arithmetic-operator
+ "Interpose artihmetic OPERATOR between ARGS, and wrap the entire expression in parens."
+ ^String [operator & args]
+ (parens (s/join (str " " (name operator) " ")
+ (map ->js args))))
+
+(def ^{:arglists '([& args])} ^String + "Interpose `+` between ARGS, and wrap the entire expression in parens." (partial arithmetic-operator :+))
+(def ^{:arglists '([& args])} ^String - "Interpose `-` between ARGS, and wrap the entire expression in parens." (partial arithmetic-operator :-))
+(def ^{:arglists '([& args])} ^String * "Interpose `*` between ARGS, and wrap the entire expression in parens." (partial arithmetic-operator :*))
+(def ^{:arglists '([& args])} ^String / "Interpose `/` between ARGS, and wrap the entire expression in parens." (partial arithmetic-operator :/))
+
+(defn fn-call
+ "Generate a JavaScript function call.
+
+ (fn-call :parseFloat :x :y) -> \"parseFloat(x, y)\""
+ ^String [fn-name & args]
+ (str (name fn-name) (argslist args)))
+
+(def ^{:arglists '([x])} ^String parse-float
+ "Generate a call to the JavaScript `parseFloat` function."
+ (partial fn-call :parseFloat))
+
+
+(defn or
+ "Interpose the JavaScript or operator (`||`) betwen ARGS, and wrap the entire expression in parens.
+
+ (or :x :y) -> \"(x || y)\""
+ ^String [& args]
+ (parens (s/join " || " (map ->js args))))
diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj
index 2ad096d4aa2eeb5f423fd67d3f01da4eeda28678..abcbc0fbd723d2d0766f5677c01155fe542a3a0d 100644
--- a/src/metabase/driver/druid/query_processor.clj
+++ b/src/metabase/driver/druid/query_processor.clj
@@ -4,13 +4,16 @@
[clojure.string :as s]
[clojure.tools.logging :as log]
[cheshire.core :as json]
+ [metabase.driver.druid.js :as js]
[metabase.query-processor :as qp]
- [metabase.query-processor.interface :as i]
+ (metabase.query-processor [annotate :as annotate]
+ [interface :as i])
[metabase.util :as u])
(:import clojure.lang.Keyword
(metabase.query_processor.interface AgFieldRef
DateTimeField
DateTimeValue
+ Expression
Field
RelativeDateTimeValue
Value)))
@@ -100,42 +103,112 @@
(declare filter:not filter:nil?)
+(defn- field? [arg]
+ (or (instance? Field arg)
+ (instance? DateTimeField arg)))
+
+(defn- expression->field-names [{:keys [args]}]
+ {:post [(every? u/string-or-keyword? %)]}
+ (flatten (for [arg args
+ :when (or (field? arg)
+ (instance? Expression arg))]
+ (cond
+ (instance? Expression arg) (expression->field-names arg)
+ (field? arg) (->rvalue arg)))))
+
+(defn- expression-arg->js [arg default-value]
+ (if-not (field? arg)
+ arg
+ (js/or (js/parse-float (->rvalue arg))
+ default-value)))
+
+(defn- expression->js [{:keys [operator args]} default-value]
+ (apply (case operator
+ :+ js/+
+ :- js/-
+ :* js/*
+ :/ js//)
+ (for [arg args]
+ (expression-arg->js arg default-value))))
+
+(defn- ag:doubleSum:expression [{operator :operator, :as expression} output-name]
+ (let [field-names (expression->field-names expression)]
+ {:type :javascript
+ :name output-name
+ :fieldNames field-names
+ :fnReset (js/function []
+ (js/return 0))
+ :fnAggregate (js/function (cons :current field-names)
+ (js/return (js/+ :current (expression->js expression (if (= operator :/) 1 0)))))
+ :fnCombine (js/function [:x :y]
+ (js/return (js/+ :x :y)))}))
+
(defn- ag:doubleSum [field output-name]
- ;; metrics can use the built-in :doubleSum aggregator, but for dimensions we have to roll something that does the same thing in JS
- (case (dimension-or-metric? field)
- :metric {:type :doubleSum
- :name output-name
- :fieldName (->rvalue field)}
- :dimension {:type :javascript
- :name output-name
- :fieldNames [(->rvalue field)]
- :fnReset "function() { return 0 ; }"
- :fnAggregate "function(current, x) { return current + (parseFloat(x) || 0); }"
- :fnCombine "function(x, y) { return x + y; }"}))
-
-(defn- ag:doubleMin [field]
- (case (dimension-or-metric? field)
- :metric {:type :doubleMin
- :name :min
- :fieldName (->rvalue field)}
- :dimension {:type :javascript
- :name :min
- :fieldNames [(->rvalue field)]
- :fnReset "function() { return Number.MAX_VALUE ; }"
- :fnAggregate "function(current, x) { return Math.min(current, (parseFloat(x) || Number.MAX_VALUE)); }"
- :fnCombine "function(x, y) { return Math.min(x, y); }"}))
-
-(defn- ag:doubleMax [field]
- (case (dimension-or-metric? field)
- :metric {:type :doubleMax
- :name :max
- :fieldName (->rvalue field)}
- :dimension {:type :javascript
- :name :max
- :fieldNames [(->rvalue field)]
- :fnReset "function() { return Number.MIN_VALUE ; }"
- :fnAggregate "function(current, x) { return Math.max(current, (parseFloat(x) || Number.MIN_VALUE)); }"
- :fnCombine "function(x, y) { return Math.max(x, y); }"}))
+ (if (instance? Expression field)
+ (ag:doubleSum:expression field output-name)
+ ;; metrics can use the built-in :doubleSum aggregator, but for dimensions we have to roll something that does the same thing in JS
+ (case (dimension-or-metric? field)
+ :metric {:type :doubleSum
+ :name output-name
+ :fieldName (->rvalue field)}
+ :dimension {:type :javascript
+ :name output-name
+ :fieldNames [(->rvalue field)]
+ :fnReset "function() { return 0 ; }"
+ :fnAggregate "function(current, x) { return current + (parseFloat(x) || 0); }"
+ :fnCombine "function(x, y) { return x + y; }"})))
+
+(defn- ag:doubleMin:expression [expression output-name]
+ (let [field-names (expression->field-names expression)]
+ {:type :javascript
+ :name output-name
+ :fieldNames field-names
+ :fnReset (js/function []
+ (js/return "Number.MAX_VALUE"))
+ :fnAggregate (js/function (cons :current field-names)
+ (js/return (js/fn-call :Math.min :current (expression->js expression :Number.MAX_VALUE))))
+ :fnCombine (js/function [:x :y]
+ (js/return (js/fn-call :Math.min :x :y)))}))
+
+(defn- ag:doubleMin [field output-name]
+ (if (instance? Expression field)
+ (ag:doubleMin:expression field output-name)
+ (case (dimension-or-metric? field)
+ :metric {:type :doubleMin
+ :name output-name
+ :fieldName (->rvalue field)}
+ :dimension {:type :javascript
+ :name output-name
+ :fieldNames [(->rvalue field)]
+ :fnReset "function() { return Number.MAX_VALUE ; }"
+ :fnAggregate "function(current, x) { return Math.min(current, (parseFloat(x) || Number.MAX_VALUE)); }"
+ :fnCombine "function(x, y) { return Math.min(x, y); }"})))
+
+(defn- ag:doubleMax:expression [expression output-name]
+ (let [field-names (expression->field-names expression)]
+ {:type :javascript
+ :name output-name
+ :fieldNames field-names
+ :fnReset (js/function []
+ (js/return "Number.MIN_VALUE"))
+ :fnAggregate (js/function (cons :current field-names)
+ (js/return (js/fn-call :Math.max :current (expression->js expression :Number.MIN_VALUE))))
+ :fnCombine (js/function [:x :y]
+ (js/return (js/fn-call :Math.max :x :y)))}))
+
+(defn- ag:doubleMax [field output-name]
+ (if (instance? Expression field)
+ (ag:doubleMax:expression field output-name)
+ (case (dimension-or-metric? field)
+ :metric {:type :doubleMax
+ :name output-name
+ :fieldName (->rvalue field)}
+ :dimension {:type :javascript
+ :name output-name
+ :fieldNames [(->rvalue field)]
+ :fnReset "function() { return Number.MIN_VALUE ; }"
+ :fnAggregate "function(current, x) { return Math.max(current, (parseFloat(x) || Number.MIN_VALUE)); }"
+ :fnCombine "function(x, y) { return Math.max(x, y); }"})))
(defn- ag:filtered [filtr aggregator] {:type :filtered, :filter filtr, :aggregator aggregator})
@@ -144,40 +217,82 @@
([field output-name] (ag:filtered (filter:not (filter:nil? field))
(ag:count output-name))))
-(defn- handle-aggregation [query-type {ag-type :aggregation-type, ag-field :field} druid-query]
+
+(defn- handle-aggregation [query-type {ag-type :aggregation-type, ag-field :field, output-name :output-name, :as ag} druid-query]
(when (isa? query-type ::ag-query)
(merge-with concat
druid-query
(let [ag-type (when-not (= ag-type :rows) ag-type)]
(match [ag-type ag-field]
;; 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 :___count)]}
-
- [:count nil] {:aggregations [(ag:count :count)]}
+ [nil nil] {:aggregations [(ag:count (or output-name :___count))]}
- [:count _] {:aggregations [(ag:count ag-field :count)]}
+ [:count nil] {:aggregations [(ag:count (or output-name :count))]}
- [:avg _] {:aggregations [(ag:count ag-field :___count)
- (ag:doubleSum ag-field :___sum)]
- :postAggregations [{:type :arithmetic
- :name :avg
- :fn :/
- :fields [{:type :fieldAccess, :fieldName :___sum}
- {:type :fieldAccess, :fieldName :___count}]}]}
+ [:count _] {:aggregations [(ag:count ag-field (or output-name :count))]}
+ [:avg _] (let [count-name (name (gensym "___count_"))
+ sum-name (name (gensym "___sum_"))]
+ {:aggregations [(ag:count ag-field count-name)
+ (ag:doubleSum ag-field sum-name)]
+ :postAggregations [{:type :arithmetic
+ :name (or output-name :avg)
+ :fn :/
+ :fields [{:type :fieldAccess, :fieldName sum-name}
+ {:type :fieldAccess, :fieldName count-name}]}]})
[:distinct _] {:aggregations [{:type :cardinality
- :name :distinct___count
+ :name (or output-name :distinct___count)
:fieldNames [(->rvalue ag-field)]}]}
- [:sum _] {:aggregations [(ag:doubleSum ag-field :sum)]}
- [:min _] {:aggregations [(ag:doubleMin ag-field)]}
- [:max _] {:aggregations [(ag:doubleMax ag-field)]})))))
+ [:sum _] {:aggregations [(ag:doubleSum ag-field (or output-name :sum))]}
+ [: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]
+ (for [arg args]
+ (cond
+ (number? arg) arg
+ (:aggregation-type arg) (assoc arg :output-name (or (:output-name arg)
+ (name (gensym (str "___" (name (:aggregation-type arg)) "_")))))
+ (instance? Expression arg) (update arg :args add-expression-aggregation-output-names))))
+
+(defn- expression-post-aggregation [{:keys [operator args], :as expression}]
+ {:type :arithmetic
+ :name (annotate/expression-aggregation-name expression)
+ :fn operator
+ :fields (for [arg args]
+ (cond
+ (number? arg) {:type :constant, :name (str arg), :value arg}
+ (:output-name arg) {:type :fieldAccess, :fieldName (:output-name arg)}
+ (instance? Expression arg) (expression-post-aggregation arg)))})
+
+(declare handle-aggregations)
+
+(defn- expression->actual-ags
+ "Return a flattened list of actual aggregations that are needed for EXPRESSION."
+ [expression]
+ (apply concat (for [arg (:args expression)
+ :when (not (number? arg))]
+ (if (instance? Expression arg)
+ (expression->actual-ags arg)
+ [arg]))))
+
+(defn- handle-expression-aggregation [query-type {:keys [operator args], :as expression} druid-query]
+ ;; filter out constants from the args list
+ (let [expression (update expression :args add-expression-aggregation-output-names)
+ ags (expression->actual-ags expression)
+ druid-query (handle-aggregations query-type {:aggregation ags} druid-query)]
+ (merge-with concat
+ druid-query
+ {:postAggregations [(expression-post-aggregation expression)]})))
(defn- handle-aggregations [query-type {aggregations :aggregation} druid-query]
(loop [[ag & more] aggregations, query druid-query]
- (let [query (handle-aggregation query-type ag query)]
- (if-not (seq more)
- query
- (recur more query)))))
+ (if (instance? Expression ag)
+ (handle-expression-aggregation query-type ag druid-query)
+ (let [query (handle-aggregation query-type ag query)]
+ (if-not (seq more)
+ query
+ (recur more query))))))
;;; ### handle-breakout
@@ -296,6 +411,10 @@
;;; ### handle-filter
+(defn- filter:and [filters]
+ {:type :and
+ :fields filters})
+
(defn- filter:not [filtr]
{:pre [filtr]}
(if (= (:type filtr) :not) ; it looks like "two nots don't make an identity" with druid
@@ -308,9 +427,13 @@
:value value})
(defn- filter:nil? [field]
- (filter:= field (case (dimension-or-metric? field)
- :dimension nil
- :metric 0)))
+ (if (instance? Expression field)
+ (filter:and (for [arg (:args field)
+ :when (field? arg)]
+ (filter:nil? arg)))
+ (filter:= field (case (dimension-or-metric? field)
+ :dimension nil
+ :metric 0))))
(defn- filter:js [field fn-format-str & args]
{:pre [field (string? fn-format-str)]}
diff --git a/src/metabase/query_processor/annotate.clj b/src/metabase/query_processor/annotate.clj
index 07aeae483e65ba6913c819a0733d13dcf4b68f36..22408841e21e95691c6176832c348527e94d1cb0 100644
--- a/src/metabase/query_processor/annotate.clj
+++ b/src/metabase/query_processor/annotate.clj
@@ -7,7 +7,8 @@
[metabase.models.field :refer [Field]]
[metabase.query-processor.interface :as i]
[metabase.util :as u])
- (:import metabase.query_processor.interface.ExpressionRef))
+ (:import (metabase.query_processor.interface Expression
+ ExpressionRef)))
;; Fields should be returned in the following order:
;; 1. Breakout Fields
@@ -93,29 +94,57 @@
:count
ag-type))
+(defn expression-aggregation-name
+ "Return an appropriate name for an expression aggregation, e.g. `sum + count`."
+ ^String [ag]
+ (cond
+ ;;
+ (instance? Expression ag) (let [{:keys [operator args]} ag]
+ (str/join (str " " (name operator) " ")
+ (for [arg args]
+ (if (instance? Expression arg)
+ (str "(" (expression-aggregation-name arg) ")")
+ (expression-aggregation-name arg)))))
+ (:aggregation-type ag) (name (:aggregation-type ag))
+ ;; a constant like
+ :else ag))
+
+(defn- expression-aggregate-field-info [expression]
+ (let [ag-name (expression-aggregation-name expression)]
+ {:source :aggregation
+ :field-name ag-name
+ :field-display-name ag-name
+ :base-type :type/Number
+ :special-type :type/Number}))
+
+(defn- aggregate-field-info [{ag-type :aggregation-type, ag-field :field}]
+ (merge (let [field-name (ag-type->field-name ag-type)]
+ {:source :aggregation
+ :field-name field-name
+ :field-display-name field-name
+ :base-type (:base-type ag-field)
+ :special-type (:special-type ag-field)})
+ ;; Always treat count or distinct count as an integer even if the DB in question returns it as something wacky like a BigDecimal or Float
+ (when (contains? #{:count :distinct} ag-type)
+ {:base-type :type/Integer
+ :special-type :type/Number})
+ ;; For the time being every Expression is an arithmetic operator and returns a floating-point number, so hardcoding these types is fine;
+ ;; In the future when we extend Expressions to handle more functionality we'll want to introduce logic that associates a return type with a given expression.
+ ;; But this will work for the purposes of a patch release.
+ (when (instance? ExpressionRef ag-field)
+ {:base-type :type/Float
+ :special-type :type/Number})))
+
(defn- add-aggregate-field-if-needed
"Add a Field containing information about an aggregate column such as `:count` or `:distinct` if needed."
[{aggregations :aggregation} fields]
(if (or (empty? aggregations)
(= (:aggregation-type (first aggregations)) :rows))
fields
- (concat fields (for [{ag-type :aggregation-type, ag-field :field} aggregations]
- (merge (let [field-name (ag-type->field-name ag-type)]
- {:source :aggregation
- :field-name field-name
- :field-display-name field-name
- :base-type (:base-type ag-field)
- :special-type (:special-type ag-field)})
- ;; Always treat count or distinct count as an integer even if the DB in question returns it as something wacky like a BigDecimal or Float
- (when (contains? #{:count :distinct} ag-type)
- {:base-type :type/Integer
- :special-type :type/Number})
- ;; For the time being every Expression is an arithmetic operator and returns a floating-point number, so hardcoding these types is fine;
- ;; In the future when we extend Expressions to handle more functionality we'll want to introduce logic that associates a return type with a given expression.
- ;; But this will work for the purposes of a patch release.
- (when (instance? ExpressionRef ag-field)
- {:base-type :type/Float
- :special-type :type/Number}))))))
+ (concat fields (for [ag aggregations]
+ (if (instance? Expression ag)
+ (expression-aggregate-field-info ag)
+ (aggregate-field-info ag))))))
(defn- add-unknown-fields-if-needed
"When create info maps for any fields we didn't expect to come back from the query.
diff --git a/src/metabase/query_processor/expand.clj b/src/metabase/query_processor/expand.clj
index 49920681f7cf90e319a45bedfd7037dbcf13861d..2ad8ad88d633ad97846c05f86e9e34454432da32 100644
--- a/src/metabase/query_processor/expand.clj
+++ b/src/metabase/query_processor/expand.clj
@@ -16,9 +16,9 @@
ComparisonFilter
CompoundFilter
EqualityFilter
+ Expression
ExpressionRef
FieldPlaceholder
- Expression
NotFilter
RelativeDatetime
StringFilter
@@ -114,8 +114,13 @@
;;; ## aggregation
+(defn- field-or-expression [f]
+ (if (instance? Expression f)
+ (update f :args (partial map field-or-expression)) ; recursively call field-or-expression on all the args of the expression
+ (field f)))
+
(s/defn ^:private ^:always-validate ag-with-field :- i/Aggregation [ag-type f]
- (i/strict-map->AggregationWithField {:aggregation-type ag-type, :field (field f)}))
+ (i/strict-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])} distinct "Aggregation clause. Return the number of distinct values of F." (partial ag-with-field :distinct))
@@ -162,9 +167,11 @@
(map? ag-or-ags) (recur query [ag-or-ags])
(empty? ag-or-ags) query
:else (assoc query :aggregation (vec (for [ag ag-or-ags]
- (u/prog1 ((if (:field ag)
- i/map->AggregationWithField
- i/map->AggregationWithoutField) (update ag :aggregation-type normalize-token))
+ ;; make sure the ag map is still typed correctly
+ (u/prog1 (cond
+ (:operator ag) (i/map->Expression ag)
+ (:field ag) (i/map->AggregationWithField (update ag :aggregation-type normalize-token))
+ :else (i/map->AggregationWithoutField (update ag :aggregation-type normalize-token)))
(s/validate i/Aggregation <>)))))))
;; also handle varargs for convenience
diff --git a/src/metabase/query_processor/interface.clj b/src/metabase/query_processor/interface.clj
index b157b6773b5674ae4679911883dae681d6a1b3be..9691104f6d1c75b46a5991f39a39e8f735e31b5d 100644
--- a/src/metabase/query_processor/interface.clj
+++ b/src/metabase/query_processor/interface.clj
@@ -177,12 +177,13 @@
unit :- DatetimeValueUnit])
-(declare RValue)
+(declare RValue Aggregation)
(def ^:private ExpressionOperator (s/named (s/enum :+ :- :* :/) "Valid expression operator"))
(s/defrecord Expression [operator :- ExpressionOperator
- args :- [(s/recursive #'RValue)]])
+ args :- [(s/cond-pre (s/recursive #'RValue)
+ (s/recursive #'Aggregation))]])
(def AnyField
"Schema for a `FieldPlaceholder`, `AgRef`, or `Expression`."
@@ -244,7 +245,8 @@
(s/defrecord AggregationWithField [aggregation-type :- (s/named (s/enum :avg :count :cumulative-sum :distinct :max :min :stddev :sum)
"Valid aggregation type")
- field :- FieldPlaceholderOrExpressionRef])
+ field :- (s/cond-pre FieldPlaceholderOrExpressionRef
+ Expression)])
(defn- valid-aggregation-for-driver? [{:keys [aggregation-type]}]
(when (= aggregation-type :stddev)
@@ -254,7 +256,7 @@
(def Aggregation
"Schema for an `aggregation` subclause in an MBQL query."
(s/constrained
- (s/cond-pre AggregationWithField AggregationWithoutField)
+ (s/cond-pre AggregationWithField AggregationWithoutField Expression)
valid-aggregation-for-driver?
"standard-deviation-aggregations is not supported by this driver."))
diff --git a/src/metabase/query_processor/resolve.clj b/src/metabase/query_processor/resolve.clj
index 903ab942b584676ea08bdbeaec27189e6f021ba6..a27df5592f9e7590cfc259d7b28007dc4acc46dc 100644
--- a/src/metabase/query_processor/resolve.clj
+++ b/src/metabase/query_processor/resolve.clj
@@ -39,12 +39,15 @@
(defprotocol ^:private IResolve
(^:private unresolved-field-id ^Integer [this]
"Return the unresolved Field ID associated with this object, if any.")
+
(^:private fk-field-id ^Integer [this]
"Return a the FK Field ID (for joining) associated with this object, if any.")
+
(^:private resolve-field [this, ^clojure.lang.IPersistentMap field-id->field]
"This method is called when walking the Query after fetching `Fields`.
Placeholder objects should lookup the relevant Field in FIELD-ID->FIELDS and
- return their expanded form. Other objects should just return themselves.a")
+ return their expanded form. Other objects should just return themselves.")
+
(resolve-table [this, ^clojure.lang.IPersistentMap fk-id+table-id->tables]
"Called when walking the Query after `Fields` have been resolved and `Tables` have been fetched.
Objects like `Fields` can add relevant information like the name of their `Table`."))
@@ -194,7 +197,7 @@
expanded-query-dict
;; Otherwise fetch + resolve the Fields in question
(let [fields (->> (u/key-by :id (db/select [field/Field :name :display_name :base_type :special_type :visibility_type :table_id :parent_id :description :id]
- :visibility_type [:not-in ["sensitive"]]
+ :visibility_type [:not= "sensitive"]
:id [:in field-ids]))
(m/map-vals rename-mb-field-keys)
(m/map-vals #(assoc % :parent (when-let [parent-id (:parent-id %)]
@@ -204,7 +207,7 @@
;; Those will be used for Table resolution in the next step.
(update expanded-query-dict :table-ids set/union (set (map :table-id (vals fields))))
;; Walk the query and resolve all fields
- (walk/postwalk #(resolve-field % fields))
+ (walk/postwalk (u/rpartial resolve-field fields))
;; Recurse in case any new (nested) unresolved fields were found.
(recur (dec max-iterations))))))))
@@ -218,10 +221,8 @@
[:target-table.name :target-table-name]
[:target-table.schema :target-table-schema]]
:from [[field/Field :source-fk]]
- :left-join [[field/Field :target-pk]
- [:= :source-fk.fk_target_field_id :target-pk.id]
- [Table :target-table]
- [:= :target-pk.table_id :target-table.id]]
+ :left-join [[field/Field :target-pk] [:= :source-fk.fk_target_field_id :target-pk.id]
+ [Table :target-table] [:= :target-pk.table_id :target-table.id]]
:where [:and [:in :source-fk.id (set fk-field-ids)]
[:= :source-fk.table_id source-table-id]
(db/isa :source-fk.special_type :type/FK)]})))
diff --git a/test/metabase/driver/druid_test.clj b/test/metabase/driver/druid_test.clj
index 752e544439e6a0ae3c4be7a38c269a316d1cf4e5..9f1f897c3a339de5ebff830c9163991f868ad914 100644
--- a/test/metabase/driver/druid_test.clj
+++ b/test/metabase/driver/druid_test.clj
@@ -2,9 +2,12 @@
(:require [cheshire.core :as json]
[expectations :refer :all]
[metabase.query-processor :as qp]
+ [metabase.query-processor.expand :as ql]
+ [metabase.query-processor-test :refer [rows]]
[metabase.test.data :as data]
[metabase.test.data.datasets :as datasets, :refer [expect-with-engine]]
- [metabase.timeseries-query-processor-test :as timeseries-qp-test]))
+ [metabase.timeseries-query-processor-test :as timeseries-qp-test]
+ [metabase.query :as q]))
(def ^:const ^:private ^String native-query-1
(json/generate-string
@@ -59,3 +62,123 @@
(expect-with-engine :druid
:completed
(:status (process-native-query native-query-2)))
+
+
+;;; +------------------------------------------------------------------------------------------------------------------------+
+;;; | MATH AGGREGATIONS |
+;;; +------------------------------------------------------------------------------------------------------------------------+
+
+(defmacro ^:private druid-query-returning-rows {:style/indent 0} [& body]
+ `(rows (timeseries-qp-test/with-flattened-dbdef
+ (qp/process-query {:database (data/id)
+ :type :query
+ :query (data/query ~'checkins
+ ~@body)}))))
+
+;; sum, *
+(expect-with-engine :druid
+ [["1" 110688.0]
+ ["2" 616708.0]
+ ["3" 179661.0]
+ ["4" 86284.0]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/sum (ql/* $id $venue_price)))
+ (ql/breakout $venue_price)))
+
+;; min, +
+(expect-with-engine :druid
+ [["1" 4.0]
+ ["2" 3.0]
+ ["3" 8.0]
+ ["4" 12.0]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/min (ql/+ $id $venue_price)))
+ (ql/breakout $venue_price)))
+
+;; max, /
+(expect-with-engine :druid
+ [["1" 1000.0]
+ ["2" 499.5]
+ ["3" 332.0]
+ ["4" 248.25]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/max (ql// $id $venue_price)))
+ (ql/breakout $venue_price)))
+
+;; avg, -
+(expect-with-engine :druid
+ [["1" 500.85067873303166]
+ ["2" 1002.7772357723577]
+ ["3" 1562.2695652173913]
+ ["4" 1760.8979591836735]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/avg (ql/* $id $venue_price)))
+ (ql/breakout $venue_price)))
+
+;; post-aggregation math w/ 2 args: count + sum
+(expect-with-engine :druid
+ [["1" 442.0]
+ ["2" 1845.0]
+ ["3" 460.0]
+ ["4" 245.0]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/+ (ql/count $id)
+ (ql/sum $venue_price)))
+ (ql/breakout $venue_price)))
+
+;; post-aggregation math w/ 3 args: count + sum + count
+(expect-with-engine :druid
+ [["1" 663.0]
+ ["2" 2460.0]
+ ["3" 575.0]
+ ["4" 294.0]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/+ (ql/count $id)
+ (ql/sum $venue_price)
+ (ql/count $venue_price)))
+ (ql/breakout $venue_price)))
+
+;; post-aggregation math w/ a constant: count * 10
+(expect-with-engine :druid
+ [["1" 2210.0]
+ ["2" 6150.0]
+ ["3" 1150.0]
+ ["4" 490.0]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/* (ql/count $id)
+ 10))
+ (ql/breakout $venue_price)))
+
+;; nested post-aggregation math: count + (count * sum)
+(expect-with-engine :druid
+ [["1" 49062.0]
+ ["2" 757065.0]
+ ["3" 39790.0]
+ ["4" 9653.0]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/+ (ql/count $id)
+ (ql/* (ql/count $id)
+ (ql/sum $venue_price))))
+ (ql/breakout $venue_price)))
+
+;; post-aggregation math w/ avg: count + avg
+(expect-with-engine :druid
+ [["1" 721.8506787330316]
+ ["2" 1116.388617886179]
+ ["3" 635.7565217391304]
+ ["4" 489.2244897959184]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/+ (ql/count $id)
+ (ql/avg $id)))
+ (ql/breakout $venue_price)))
+
+;; post aggregation math + math inside aggregations: max(venue_price) + min(venue_price - id)
+(expect-with-engine :druid
+ [["1" -998.0]
+ ["2" -995.0]
+ ["3" -990.0]
+ ["4" -985.0]]
+ (druid-query-returning-rows
+ (ql/aggregation (ql/+ (ql/max $venue_price)
+ (ql/min (ql/- $venue_price $id))))
+ (ql/breakout $venue_price)))