Skip to content
Snippets Groups Projects
Unverified Commit b1388dff authored by Cam Saul's avatar Cam Saul
Browse files

Make sure QP 'macro' expansion code is case-insensitive

parent 6a1e5b66
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
project.clj
+ 1
− 1
View file @ b1388dff
......@@ -5,7 +5,7 @@
:description "Metabase Community Edition"
:url "http://metabase.com/"
:min-lein-version "2.5.0"
:aliases {"bikeshed" ["bikeshed" "--max-line-length" "220"]
:aliases {"bikeshed" ["bikeshed" "--max-line-length" "205"]
"check-reflection-warnings" ["with-profile" "+reflection-warnings" "check"]
"test" ["with-profile" "+expectations" "expectations"]
"generate-sample-dataset" ["with-profile" "+generate-sample-dataset" "run"]
......
src/metabase/models/permissions.clj
+ 41
− 31
View file @ b1388dff
......@@ -18,19 +18,20 @@
[db :as db]
[models :as models]]))
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; | UTIL FNS |
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | UTIL FNS |
;;; +----------------------------------------------------------------------------------------------------------------+
;;; ---------------------------------------- Dynamic Vars ----------------------------------------
(def ^:dynamic ^Boolean *allow-root-entries*
"Show we allow permissions entries like `/`? By default, this is disallowed, but you can temporarily disable it here when creating the default entry for `Admin`."
"Show we allow permissions entries like `/`? By default, this is disallowed, but you can temporarily disable it here
when creating the default entry for `Admin`."
false)
(def ^:dynamic ^Boolean *allow-admin-permissions-changes*
"Show we allow changes to be made to permissions belonging to the Admin group? By default this is disabled to prevent accidental tragedy, but you can enable it here
when creating the default entry for `Admin`."
"Show we allow changes to be made to permissions belonging to the Admin group? By default this is disabled to
prevent accidental tragedy, but you can enable it here when creating the default entry for `Admin`."
false)
......@@ -75,7 +76,8 @@
{:status-code 400}))))
(defn- assert-valid
"Check to make sure this PERMISSIONS entry is something that's allowed to be saved (i.e. it has a valid `:object` path and it's not for the admin group)."
"Check to make sure this PERMISSIONS entry is something that's allowed to be saved (i.e. it has a valid `:object`
path and it's not for the admin group)."
[permissions]
(assert-not-admin-group permissions)
(assert-valid-object permissions))
......@@ -96,8 +98,8 @@
(defn ^:deprecated native-read-path
"Return the native query *read* permissions path for a database.
This grants you permissions to view the results of an *existing* native query, i.e. view native Cards created by others.
(Deprecated because native read permissions are being phased out in favor of Collections.)"
This grants you permissions to view the results of an *existing* native query, i.e. view native Cards created by
others. (Deprecated because native read permissions are being phased out in favor of Collections.)"
^String [database-id]
(str (object-path database-id) "native/read/"))
......@@ -168,9 +170,9 @@
object-paths-set))
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; | ENTITY + LIFECYCLE |
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | ENTITY + LIFECYCLE |
;;; +----------------------------------------------------------------------------------------------------------------+
(models/defmodel Permissions :permissions)
......@@ -194,9 +196,9 @@
:pre-delete pre-delete}))
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; | GRAPH SCHEMA |
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GRAPH SCHEMA |
;;; +----------------------------------------------------------------------------------------------------------------+
(def ^:private TablePermissionsGraph
(s/enum :none :all))
......@@ -248,12 +250,13 @@
:groups {su/IntGreaterThanZero StrictGroupPermissionsGraph}})
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; | GRAPH FETCH |
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GRAPH FETCH |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- permissions-for-path
"Given a PERMISSIONS-SET of all allowed permissions paths for a Group, return the corresponding permissions status for an object with PATH."
"Given a PERMISSIONS-SET of all allowed permissions paths for a Group, return the corresponding permissions status
for an object with PATH."
[permissions-set path]
(u/prog1 (cond
(set-has-full-permissions? permissions-set path) :all
......@@ -304,16 +307,17 @@
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; | GRAPH UPDATE |
;;; +------------------------------------------------------------------------------------------------------------------------------------------------------+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GRAPH UPDATE |
;;; +----------------------------------------------------------------------------------------------------------------+
;;; ---------------------------------------- Helper Fns ----------------------------------------
;; TODO - why does this take a PATH when everything else takes PATH-COMPONENTS or IDs?
(defn delete-related-permissions!
"Delete all permissions for GROUP-OR-ID for ancestors or descendant objects of object with PATH.
You can optionally include OTHER-CONDITIONS, which are anded into the filter clause, to further restrict what is deleted."
You can optionally include OTHER-CONDITIONS, which are anded into the filter clause, to further restrict what is
deleted."
{:style/indent 2}
[group-or-id path & other-conditions]
{:pre [(integer? (u/get-id group-or-id)) (valid-object-path? path)]}
......@@ -402,12 +406,14 @@
;;; ---------------------------------------- Graph Updating Fns ----------------------------------------
(s/defn ^:private ^:always-validate update-table-perms! [group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, schema :- s/Str, table-id :- su/IntGreaterThanZero, new-table-perms :- SchemaPermissionsGraph]
(s/defn ^:private ^:always-validate update-table-perms!
[group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, schema :- s/Str, table-id :- su/IntGreaterThanZero, new-table-perms :- SchemaPermissionsGraph]
(case new-table-perms
:all (grant-permissions! group-id db-id schema table-id)
:none (revoke-permissions! group-id db-id schema table-id)))
(s/defn ^:private ^:always-validate update-schema-perms! [group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, schema :- s/Str, new-schema-perms :- SchemaPermissionsGraph]
(s/defn ^:private ^:always-validate update-schema-perms!
[group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, schema :- s/Str, new-schema-perms :- SchemaPermissionsGraph]
(cond
(= new-schema-perms :all) (do (revoke-permissions! group-id db-id schema) ; clear out any existing related permissions
(grant-permissions! group-id db-id schema)) ; then grant full perms for the schema
......@@ -415,7 +421,8 @@
(map? new-schema-perms) (doseq [[table-id table-perms] new-schema-perms]
(update-table-perms! group-id db-id schema table-id table-perms))))
(s/defn ^:private ^:always-validate update-native-permissions! [group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, new-native-perms :- NativePermissionsGraph]
(s/defn ^:private ^:always-validate update-native-permissions!
[group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, new-native-perms :- NativePermissionsGraph]
;; revoke-native-permissions! will delete all entires that would give permissions for native access.
;; Thus if you had a root DB entry like `/db/11/` this will delete that too.
;; In that case we want to create a new full schemas entry so you don't lose access to all schemas when we modify native access.
......@@ -429,7 +436,8 @@
:none nil))
(s/defn ^:private ^:always-validate update-db-permissions! [group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, new-db-perms :- StrictDBPermissionsGraph]
(s/defn ^:private ^:always-validate update-db-permissions!
[group-id :- su/IntGreaterThanZero, db-id :- su/IntGreaterThanZero, new-db-perms :- StrictDBPermissionsGraph]
(when-let [new-native-perms (:native new-db-perms)]
(update-native-permissions! group-id db-id new-native-perms))
(when-let [schemas (:schemas new-db-perms)]
......@@ -440,7 +448,8 @@
(map? schemas) (doseq [schema (keys schemas)]
(update-schema-perms! group-id db-id schema (get-in new-db-perms [:schemas schema]))))))
(s/defn ^:private ^:always-validate update-group-permissions! [group-id :- su/IntGreaterThanZero, new-group-perms :- StrictGroupPermissionsGraph]
(s/defn ^:private ^:always-validate update-group-permissions!
[group-id :- su/IntGreaterThanZero, new-group-perms :- StrictGroupPermissionsGraph]
(doseq [[db-id new-db-perms] new-group-perms]
(update-db-permissions! group-id db-id new-db-perms)))
......@@ -475,9 +484,10 @@
(s/defn ^:always-validate update-graph!
"Update the permissions graph, making any changes neccesary to make it match NEW-GRAPH.
This should take in a graph that is exactly the same as the one obtained by `graph` with any changes made as needed. The graph is revisioned,
so if it has been updated by a third party since you fetched it this function will fail and return a 409 (Conflict) exception.
If nothing needs to be done, this function returns `nil`; otherwise it returns the newly created `PermissionsRevision` entry."
This should take in a graph that is exactly the same as the one obtained by `graph` with any changes made as
needed. The graph is revisioned, so if it has been updated by a third party since you fetched it this function will
fail and return a 409 (Conflict) exception. If nothing needs to be done, this function returns `nil`; otherwise it
returns the newly created `PermissionsRevision` entry."
([new-graph :- StrictPermissionsGraph]
(let [old-graph (graph)
[old new] (data/diff (:groups old-graph) (:groups new-graph))]
......
src/metabase/query_processor/middleware/expand.clj
+ 22
− 13
View file @ b1388dff
......@@ -10,10 +10,13 @@
[metabase.util :as u]
[metabase.util.schema :as su]
[schema.core :as s])
(:import [metabase.query_processor.interface AgFieldRef BetweenFilter ComparisonFilter CompoundFilter DateTimeValue DateTimeField Expression
ExpressionRef FieldLiteral FieldPlaceholder RelativeDatetime RelativeDateTimeValue StringFilter Value ValuePlaceholder]))
(:import [metabase.query_processor.interface AgFieldRef BetweenFilter ComparisonFilter CompoundFilter DateTimeValue
DateTimeField Expression ExpressionRef FieldLiteral FieldPlaceholder RelativeDatetime
RelativeDateTimeValue StringFilter Value ValuePlaceholder]))
;;; # ------------------------------------------------------------ Clause Handlers ------------------------------------------------------------
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | CLAUSE HANDLERS |
;;; +----------------------------------------------------------------------------------------------------------------+
;; TODO - check that there's a matching :aggregation clause in the query ?
(s/defn ^:ql ^:always-validate aggregate-field :- AgFieldRef
......@@ -139,7 +142,8 @@
(defn- field-or-expression [f]
(if (instance? Expression f)
;; recursively call field-or-expression on all the args inside the expression unless they're numbers
;; plain numbers are always assumed to be numeric literals here; you must use MBQL '98 `:field-id` syntax to refer to Fields inside an expression <3
;; plain numbers are always assumed to be numeric literals here; you must use MBQL '98 `:field-id` syntax to refer
;; to Fields inside an expression <3
(update f :args #(for [arg %]
(if (number? arg)
arg
......@@ -467,7 +471,9 @@
(defn ^:ql segment "Placeholder expansion function for GA segment clauses. (This does not expand normal Segment macros; that is done in `metabase.query-processor.macros`.)" [& _])
;;; # ------------------------------------------------------------ Expansion ------------------------------------------------------------
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | EXPANSION |
;;; +----------------------------------------------------------------------------------------------------------------+
;; QL functions are any public function in this namespace marked with `^:ql`.
(def ^:private token->ql-fn
......@@ -499,10 +505,10 @@
(defn- walk-expand-ql-sexprs
"Walk QUERY depth-first and expand QL bracketed S-expressions."
[x]
(cond (map? x) (into x (for [[k v] x] ; do `into x` instead of `into {}` so we can keep the original class,
[k (walk-expand-ql-sexprs v)])) ; e.g. FieldPlaceholder
(vector? x) (expand-ql-sexpr (mapv walk-expand-ql-sexprs x))
:else x))
(cond (map? x) (into x (for [[k v] x] ; do `into x` instead of `into {}` so we can keep the original class,
[k (walk-expand-ql-sexprs v)])) ; e.g. FieldPlaceholder
(sequential? x) (expand-ql-sexpr (mapv walk-expand-ql-sexprs x))
:else x))
(s/defn ^:always-validate expand-inner :- i/Query
......@@ -521,8 +527,8 @@
query))))
(defn expand
"Expand a query dictionary as it comes in from the API and return an \"expanded\" form, (almost) ready for use by the Query Processor.
This includes steps like token normalization and function dispatch.
"Expand a query dictionary as it comes in from the API and return an \"expanded\" form, (almost) ready for use by
the Query Processor. This includes steps like token normalization and function dispatch.
(expand {:query {\"SOURCE_TABLE\" 10, \"FILTER\" [\"=\" 100 200]}})
......@@ -532,7 +538,8 @@
:value {:field-placeholder {:field-id 100}
:value 200}}}}
The \"placeholder\" objects above are fetched from the DB and replaced in the next QP step, in `metabase.query-processor.middleware.resolve`."
The \"placeholder\" objects above are fetched from the DB and replaced in the next QP step, in
`metabase.query-processor.middleware.resolve`."
[outer-query]
(update outer-query :query expand-inner))
......@@ -554,7 +561,9 @@
expand-inner))
;;; ------------------------------------------------------------ Other Helper Fns ------------------------------------------------------------
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | OTHER HELPER FNS |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn is-clause?
"Check to see whether CLAUSE is an instance of the clause named by normalized CLAUSE-KEYWORD.
......
src/metabase/query_processor/middleware/expand_macros.clj
+ 58
− 42
View file @ b1388dff
......@@ -8,15 +8,24 @@
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.tools.logging :as log]
(metabase.query-processor [interface :as i]
[util :as qputil])
[clojure.core.match :refer [match]]
[clojure.walk :as walk]
[toucan.db :as db]
[metabase.util :as u]))
;;; ------------------------------------------------------------ Utils ------------------------------------------------------------
[metabase.models
[metric :refer [Metric]]
[segment :refer [Segment]]]
[metabase.query-processor
[interface :as i]
[util :as qputil]]
[metabase.util :as u]
[toucan.db :as db]))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | UTIL FNS |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- is-clause? [clause-names object]
(and (sequential? object)
((some-fn string? keyword?) (first object))
(contains? clause-names (qputil/normalize-token (first object)))))
(defn- non-empty-clause? [clause]
(and clause
......@@ -24,53 +33,48 @@
(and (seq clause)
(not (every? nil? clause))))))
;;; ------------------------------------------------------------ Segments ------------------------------------------------------------
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | SEGMENTS |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- segment-parse-filter-subclause [form]
(when (non-empty-clause? form)
(match form
["SEGMENT" (segment-id :guard integer?)] (:filter (db/select-one-field :definition 'Segment :id segment-id))
subclause subclause
form (throw (java.lang.Exception. (format "segment-parse-filter-subclause failed: invalid clause: %s" form))))))
(if-not (is-clause? #{:segment} form)
form
(:filter (db/select-one-field :definition Segment :id (u/get-id (second form)))))))
(defn- segment-parse-filter [form]
(when (non-empty-clause? form)
(match form
["AND" & subclauses] (into ["AND"] (mapv segment-parse-filter subclauses))
["OR" & subclauses] (into ["OR"] (mapv segment-parse-filter subclauses))
subclause (segment-parse-filter-subclause subclause)
form (throw (java.lang.Exception. (format "segment-parse-filter failed: invalid clause: %s" form))))))
(if (is-clause? #{:and :or :not} form)
;; for forms that start with AND/OR/NOT recursively parse the subclauses and put them nicely back into their
;; compound form
(cons (first form) (mapv segment-parse-filter (rest form)))
;; otherwise we should have a filter subclause so parse it as such
(segment-parse-filter-subclause form))))
(defn- expand-segments [query-dict]
(if (non-empty-clause? (get-in query-dict [:query :filter]))
(update-in query-dict [:query :filter] segment-parse-filter)
query-dict))
(defn- merge-filter-clauses [base addtl]
(cond
(and (seq base)
(seq addtl)) ["AND" base addtl]
(seq base) base
(seq addtl) addtl
:else []))
;;; ------------------------------------------------------------ Metrics ------------------------------------------------------------
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | METRICS |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- metric? [aggregation]
(match aggregation
["METRIC" (_ :guard integer?)] true
_ false))
(is-clause? #{:metric} aggregation))
(defn- metric-id [metric]
(when (metric? metric)
(second metric)))
(u/get-id (second metric))))
(defn- maybe-unnest-ag-clause
"Unnest AG-CLAUSE if it's wrapped in a vector (i.e. if it is using the \"multiple-aggregation\" syntax).
(This is provided merely as a convenience to facilitate implementation of the Query Builder, so it can use the same UI for
normal aggregations and Metric creation. *METRICS DO NOT SUPPORT MULTIPLE AGGREGATIONS,* so if nested syntax is used, any
aggregation after the first will be ignored.)"
(This is provided merely as a convenience to facilitate implementation of the Query Builder, so it can use the same
UI for normal aggregations and Metric creation. *METRICS DO NOT SUPPORT MULTIPLE AGGREGATIONS,* so if nested syntax
is used, any aggregation after the first will be ignored.)"
[ag-clause]
(if (and (coll? ag-clause)
(every? coll? ag-clause))
......@@ -78,17 +82,27 @@
ag-clause))
(defn- expand-metric [metric-clause filter-clauses-atom]
(let [{filter-clause :filter, ag-clause :aggregation} (db/select-one-field :definition 'Metric, :id (metric-id metric-clause))]
(let [{filter-clause :filter, ag-clause :aggregation} (db/select-one-field :definition Metric
:id (metric-id metric-clause))]
(when filter-clause
(swap! filter-clauses-atom conj filter-clause))
(maybe-unnest-ag-clause ag-clause)))
(defn- expand-metrics-in-ag-clause [query-dict filter-clauses-atom]
(walk/postwalk (fn [form]
(if-not (metric? form)
form
(expand-metric form filter-clauses-atom)))
query-dict))
(walk/postwalk
(fn [form]
(if-not (metric? form)
form
(expand-metric form filter-clauses-atom)))
query-dict))
(defn- merge-filter-clauses [base-clause additional-clauses]
(cond
(and (seq base-clause)
(seq additional-clauses)) [:and base-clause additional-clauses]
(seq base-clause) base-clause
(seq additional-clauses) additional-clauses
:else []))
(defn- add-metrics-filter-clauses
"Add any FILTER-CLAUSES to the QUERY-DICT. If query has existing filter clauses, the new ones are
......@@ -97,7 +111,7 @@
(if-not (seq filter-clauses)
query-dict
(update-in query-dict [:query :filter] merge-filter-clauses (if (> (count filter-clauses) 1)
(cons "AND" filter-clauses)
(cons :and filter-clauses)
(first filter-clauses)))))
(defn- expand-metrics* [query-dict]
......@@ -113,7 +127,9 @@
(expand-metrics* query-dict)))
;;; ------------------------------------------------------------ Middleware ------------------------------------------------------------
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | MIDDLEWARE |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- expand-metrics-and-segments "Expand the macros (SEGMENT, METRIC) in a QUERY."
[query]
......
src/metabase/util/infer_spaces.clj
+ 3
− 1
View file @ b1388dff
(ns metabase.util.infer-spaces
"Logic for automatically inferring where spaces should go in table names. Ported from ported from https://stackoverflow.com/questions/8870261/how-to-split-text-without-spaces-into-list-of-words/11642687#11642687."
"Logic for automatically inferring where spaces should go in table names. Ported from
https://stackoverflow.com/questions/8870261/how-to-split-text-without-spaces-into-list-of-words/11642687#11642687."
;; TODO - The code in this namespace is very hard to understand. We should clean it up and make it readable.
(:require [clojure.java.io :as io]
[clojure.string :as s])
(:import java.lang.Math))
......
test/metabase/query_processor/middleware/expand_macros_test.clj
+ 70
− 44
View file @ b1388dff
......@@ -9,17 +9,13 @@
[metric :refer [Metric]]
[segment :refer [Segment]]
[table :refer [Table]]]
[metabase.query-processor.middleware.expand :as ql]
[metabase.query-processor.middleware.expand-macros :refer :all]
[metabase.test
[data :as data]
[util :as tu]]
[metabase.query-processor.middleware
[expand :as ql]
[expand-macros :as expand-macros :refer :all]]
[metabase.test.data :as data]
[metabase.test.data.datasets :as datasets]
[toucan.util.test :as tt]))
;; expand-metrics-and-segments
(tu/resolve-private-vars metabase.query-processor.middleware.expand-macros expand-metrics-and-segments)
;; no Segment or Metric should yield exact same query
(expect
{:database 1
......@@ -27,11 +23,11 @@
:query {:aggregation ["rows"]
:filter ["AND" [">" 4 1]]
:breakout [17]}}
(expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["rows"]
:filter ["AND" [">" 4 1]]
:breakout [17]}}))
(#'expand-macros/expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["rows"]
:filter ["AND" [">" 4 1]]
:breakout [17]}}))
;; just segments
(expect
......@@ -48,18 +44,42 @@
:definition {:filter ["AND" ["=" 5 "abc"]]}}]
Segment [{segment-2-id :id} {:table_id table-id
:definition {:filter ["AND" ["IS_NULL" 7]]}}]]
(expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["rows"]
:filter ["AND" ["SEGMENT" segment-1-id] ["OR" ["SEGMENT" segment-2-id] [">" 4 1]]]
:breakout [17]}})))
(#'expand-macros/expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["rows"]
:filter ["AND" ["SEGMENT" segment-1-id] ["OR" ["SEGMENT" segment-2-id] [">" 4 1]]]
:breakout [17]}})))
;; Does expansion work if "AND" isn't capitalized? (MBQL is case-insensitive!) (#5706, #5530)
(expect
{:database 1
:type :query
:query {:aggregation ["rows"]
:filter ["and"
["=" 5 "abc"]
["IS_NULL" 7]]
:breakout [17]}}
(tt/with-temp* [Database [{database-id :id}]
Table [{table-id :id} {:db_id database-id}]
Segment [{segment-1-id :id} {:table_id table-id
:definition {:filter ["=" 5 "abc"]}}]
Segment [{segment-2-id :id} {:table_id table-id
:definition {:filter ["IS_NULL" 7]}}]]
(#'expand-macros/expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["rows"]
:filter ["and"
["SEGMENT" segment-1-id]
["SEGMENT" segment-2-id]]
:breakout [17]}})))
;; just a metric (w/out nested segments)
(expect
{:database 1
:type :query
:query {:aggregation ["count"]
:filter ["AND" ["AND" [">" 4 1]]
:filter [:and
["AND" [">" 4 1]]
["AND" ["=" 5 "abc"]]]
:breakout [17]
:order_by [[1 "ASC"]]}}
......@@ -68,12 +88,12 @@
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]]}}]]
(expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter ["AND" [">" 4 1]]
:breakout [17]
:order_by [[1 "ASC"]]}})))
(#'expand-macros/expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter ["AND" [">" 4 1]]
:breakout [17]
:order_by [[1 "ASC"]]}})))
;; check that when the original filter is empty we simply use our metric filter definition instead
(expect
......@@ -88,12 +108,12 @@
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]]}}]]
(expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter []
:breakout [17]
:order_by [[1 "ASC"]]}})))
(#'expand-macros/expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter []
:breakout [17]
:order_by [[1 "ASC"]]}})))
;; metric w/ no filter definition
(expect
......@@ -107,19 +127,25 @@
Table [{table-id :id} {:db_id database-id}]
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["count"]}}]]
(expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter ["AND" ["=" 5 "abc"]]
:breakout [17]
:order_by [[1 "ASC"]]}})))
(#'expand-macros/expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter ["AND" ["=" 5 "abc"]]
:breakout [17]
:order_by [[1 "ASC"]]}})))
;; a metric w/ nested segments
(expect
{:database 1
:type :query
: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]
:order_by [[1 "ASC"]]}}
(tt/with-temp* [Database [{database-id :id}]
......@@ -131,12 +157,12 @@
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["sum" 18]
:filter ["AND" ["=" 5 "abc"] ["SEGMENT" segment-1-id]]}}]]
(expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter ["AND" [">" 4 1] ["SEGMENT" segment-2-id]]
:breakout [17]
:order_by [[1 "ASC"]]}})))
(#'expand-macros/expand-metrics-and-segments {:database 1
:type :query
:query {:aggregation ["METRIC" metric-1-id]
:filter ["AND" [">" 4 1] ["SEGMENT" segment-2-id]]
:breakout [17]
:order_by [[1 "ASC"]]}})))
;; Check that a metric w/ multiple aggregation syntax (nested vector) still works correctly
(datasets/expect-with-engines (engines-that-support :expression-aggregations)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment