Add MBQL `:share` clause.

src/metabase/driver/sql/query_processor.clj

@@ -244,6 +244,14 @@
                                          (hsql/call := arg 0) nil
                                          :else                arg)))))
 
+(defmethod ->honeysql [:sql :share]
+  [driver [_ pred]]
+  (hsql/call :/
+             (hsql/call :sum (hsql/call :case
+                                        (->honeysql driver pred) 1.0
+                                        :else                    0.0))
+             :%count.*))
+
 (defmethod ->honeysql [:sql :named] [driver [_ ag ag-name]]
   (->honeysql driver ag))
 

src/metabase/mbql/normalize.clj

@@ -306,7 +306,7 @@
     [:rows & _]
     nil
 
-    ;; For named aggregations (`[:named <ag> <name>]`) we want to leave as-is an just canonicalize the ag it names
+    ;; For named aggregations (`[:named <ag> <name>]`) we want to leave as-is and just canonicalize the ag it names
     [:named ag ag-name]
     [:named (canonicalize-aggregation-subclause ag) ag-name]
 

src/metabase/mbql/schema.clj

@@ -250,92 +250,6 @@
     Field))
 
 
-;;; -------------------------------------------------- Aggregations --------------------------------------------------
-
-;; For all of the 'normal' Aggregations below (excluding Metrics) fields are implicit Field IDs
-
-;; cum-sum and cum-count are SUGAR because they're implemented in middleware. They clauses are swapped out with
-;; `count` and `sum` aggregations respectively and summation is done in Clojure-land
-(defclause ^{:requires-features #{:basic-aggregations}} ^:sugar count,     field (optional Field))
-(defclause ^{:requires-features #{:basic-aggregations}} ^:sugar cum-count, field (optional Field))
-
-;; technically aggregations besides count can also accept expressions as args, e.g.
-;;
-;;    [[:sum [:+ [:field-id 1] [:field-id 2]]]]
-;;
-;; Which is equivalent to SQL:
-;;
-;;    SUM(field_1 + field_2)
-
-(defclause ^{:requires-features #{:basic-aggregations}} avg,      field-or-expression FieldOrExpressionDef)
-(defclause ^{:requires-features #{:basic-aggregations}} cum-sum,  field-or-expression FieldOrExpressionDef)
-(defclause ^{:requires-features #{:basic-aggregations}} distinct, field-or-expression FieldOrExpressionDef)
-(defclause ^{:requires-features #{:basic-aggregations}} sum,      field-or-expression FieldOrExpressionDef)
-(defclause ^{:requires-features #{:basic-aggregations}} min,      field-or-expression FieldOrExpressionDef)
-(defclause ^{:requires-features #{:basic-aggregations}} max,      field-or-expression FieldOrExpressionDef)
-
-(defclause ^{:requires-features #{:standard-deviation-aggregations}} stddev, field-or-expression FieldOrExpressionDef)
-
-;; Metrics are just 'macros' (placeholders for other aggregations with optional filter and breakout clauses) that get
-;; expanded to other aggregations/etc. in the expand-macros middleware
-;;
-;; METRICS WITH STRING IDS, e.g. `[:metric "ga:sessions"]`, are Google Analytics metrics, not Metabase metrics! They
-;; pass straight thru to the GA query processor.
-(defclause ^:sugar metric, metric-id (s/cond-pre su/IntGreaterThanZero su/NonBlankString))
-
-;; the following are definitions for expression aggregations, e.g. [:+ [:sum [:field-id 10]] [:sum [:field-id 20]]]
-
-(declare Aggregation)
-
-(def ^:private ExpressionAggregationArg
-  (s/if number?
-    s/Num
-    (s/recursive #'Aggregation)))
-
-(defclause [^{:requires-features #{:expression-aggregations}} ag:+   +]
-  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
-
-(defclause [^{:requires-features #{:expression-aggregations}} ag:-   -]
-  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
-
-(defclause [^{:requires-features #{:expression-aggregations}} ag:*   *]
-  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
-
-(defclause [^{:requires-features #{:expression-aggregations}} ag:div /]
-  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
-;; ag:/ isn't a valid token
-
-(def ^:private UnnamedAggregation
-  (one-of count avg cum-count cum-sum distinct stddev sum min max ag:+ ag:- ag:* ag:div metric))
-
-;; any sort of aggregation can be wrapped in a `[:named <ag> <custom-name>]` clause, but you cannot wrap a `:named` in
-;; a `:named`
-
-(defclause named, aggregation UnnamedAggregation, aggregation-name su/NonBlankString)
-
-(def Aggregation
-  "Schema for anything that is a valid `:aggregation` clause."
-  (s/if (partial is-clause? :named)
-    named
-    UnnamedAggregation))
-
-
-;;; ---------------------------------------------------- Order-By ----------------------------------------------------
-
-;; order-by is just a series of `[<direction> <field>]` clauses like
-;;
-;;    {:order-by [[:asc [:field-id 1]], [:desc [:field-id 2]]]}
-;;
-;; Field ID is implicit in these clauses
-
-(defclause asc,  field FieldOrAggregationReference)
-(defclause desc, field FieldOrAggregationReference)
-
-(def OrderBy
-  "Schema for an `order-by` clause subclause."
-  (one-of asc desc))
-
-
 ;;; ----------------------------------------------------- Filter -----------------------------------------------------
 
 (declare Filter)
@@ -470,6 +384,93 @@
    does-not-contain inside is-null not-null time-interval segment))
 
 
+;;; -------------------------------------------------- Aggregations --------------------------------------------------
+
+;; For all of the 'normal' Aggregations below (excluding Metrics) fields are implicit Field IDs
+
+;; cum-sum and cum-count are SUGAR because they're implemented in middleware. They clauses are swapped out with
+;; `count` and `sum` aggregations respectively and summation is done in Clojure-land
+(defclause ^{:requires-features #{:basic-aggregations}} ^:sugar count,     field (optional Field))
+(defclause ^{:requires-features #{:basic-aggregations}} ^:sugar cum-count, field (optional Field))
+
+;; technically aggregations besides count can also accept expressions as args, e.g.
+;;
+;;    [[:sum [:+ [:field-id 1] [:field-id 2]]]]
+;;
+;; Which is equivalent to SQL:
+;;
+;;    SUM(field_1 + field_2)
+
+(defclause ^{:requires-features #{:basic-aggregations}} avg,      field-or-expression FieldOrExpressionDef)
+(defclause ^{:requires-features #{:basic-aggregations}} cum-sum,  field-or-expression FieldOrExpressionDef)
+(defclause ^{:requires-features #{:basic-aggregations}} distinct, field-or-expression FieldOrExpressionDef)
+(defclause ^{:requires-features #{:basic-aggregations}} sum,      field-or-expression FieldOrExpressionDef)
+(defclause ^{:requires-features #{:basic-aggregations}} min,      field-or-expression FieldOrExpressionDef)
+(defclause ^{:requires-features #{:basic-aggregations}} max,      field-or-expression FieldOrExpressionDef)
+(defclause ^{:requires-features #{:basic-aggregations}} share,    pred Filter)
+
+(defclause ^{:requires-features #{:standard-deviation-aggregations}} stddev, field-or-expression FieldOrExpressionDef)
+
+;; Metrics are just 'macros' (placeholders for other aggregations with optional filter and breakout clauses) that get
+;; expanded to other aggregations/etc. in the expand-macros middleware
+;;
+;; METRICS WITH STRING IDS, e.g. `[:metric "ga:sessions"]`, are Google Analytics metrics, not Metabase metrics! They
+;; pass straight thru to the GA query processor.
+(defclause ^:sugar metric, metric-id (s/cond-pre su/IntGreaterThanZero su/NonBlankString))
+
+;; the following are definitions for expression aggregations, e.g. [:+ [:sum [:field-id 10]] [:sum [:field-id 20]]]
+
+(declare Aggregation)
+
+(def ^:private ExpressionAggregationArg
+  (s/if number?
+    s/Num
+    (s/recursive #'Aggregation)))
+
+(defclause [^{:requires-features #{:expression-aggregations}} ag:+   +]
+  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
+
+(defclause [^{:requires-features #{:expression-aggregations}} ag:-   -]
+  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
+
+(defclause [^{:requires-features #{:expression-aggregations}} ag:*   *]
+  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
+
+(defclause [^{:requires-features #{:expression-aggregations}} ag:div /]
+  x ExpressionAggregationArg, y ExpressionAggregationArg, more (rest ExpressionAggregationArg))
+;; ag:/ isn't a valid token
+
+(def ^:private UnnamedAggregation
+  (one-of count avg cum-count cum-sum distinct stddev sum min max ag:+ ag:- ag:* ag:div metric share))
+
+;; any sort of aggregation can be wrapped in a `[:named <ag> <custom-name>]` clause, but you cannot wrap a `:named` in
+;; a `:named`
+
+(defclause named, aggregation UnnamedAggregation, aggregation-name su/NonBlankString)
+
+(def Aggregation
+  "Schema for anything that is a valid `:aggregation` clause."
+  (s/if (partial is-clause? :named)
+    named
+    UnnamedAggregation))
+
+
+;;; ---------------------------------------------------- Order-By ----------------------------------------------------
+
+;; order-by is just a series of `[<direction> <field>]` clauses like
+;;
+;;    {:order-by [[:asc [:field-id 1]], [:desc [:field-id 2]]]}
+;;
+;; Field ID is implicit in these clauses
+
+(defclause asc,  field FieldOrAggregationReference)
+(defclause desc, field FieldOrAggregationReference)
+
+(def OrderBy
+  "Schema for an `order-by` clause subclause."
+  (one-of asc desc))
+
+
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                                    Queries                                                     |
 ;;; +----------------------------------------------------------------------------------------------------------------+

test/metabase/driver/sql/query_processor_test.clj

 (ns metabase.driver.sql.query-processor-test
   (:require [expectations :refer [expect]]
-            [metabase.driver.sql.query-processor :as sql.qp]))
+            [metabase.driver.sql.query-processor :as sql.qp]
+            [metabase.query-processor :as qp]
+            [metabase.test.data :as data]))
 
 ;; make sure our logic for deciding which order to process keys in the query works as expected
 (expect
@@ -11,3 +13,24 @@
                                               :aggregation  3
                                               :fields       4
                                               :breakout     2}))
+
+(defn- test-query
+  [query]
+  (:data (qp/process-query {:database (data/id)
+                            :type :query
+                            :query query})))
+
+(expect
+  0.94
+  (-> (test-query {:source-table (data/id :venues)
+                   :aggregation [[:share [:< [:field-id (data/id :venues :price)] 4]]]})
+      :rows
+      ffirst
+      double))
+(expect
+  nil
+  (-> (test-query {:source-table (data/id :venues)
+                   :aggregation [[:share [:< [:field-id (data/id :venues :price)] 4]]]
+                   :filter [:> [:field-id (data/id :venues :price)] Long/MAX_VALUE]})
+      :rows
+      ffirst))