Skip to content
Snippets Groups Projects
Commit c0e9ea79 authored by Cam Saül's avatar Cam Saül
Browse files

Use schema for validation :yum:

parent 144ede39
Branches
Tags
No related merge requests found
Expand all Hide whitespace changes
Inline Side-by-side
project.clj
+ 1
− 0
View file @ c0e9ea79
......@@ -48,6 +48,7 @@
[org.yaml/snakeyaml "1.16"] ; YAML parser (required by liquibase)
[org.xerial/sqlite-jdbc "3.8.11.2"] ; SQLite driver
[postgresql "9.3-1102.jdbc41"] ; Postgres driver
[prismatic/schema "1.0.4"] ; Data schema declaration and validation library
[ring/ring-jetty-adapter "1.4.0"] ; Ring adapter using Jetty webserver (used to run a Ring server for unit tests)
[ring/ring-json "0.4.0"] ; Ring middleware for reading/writing JSON automatically
[stencil "0.5.0"] ; Mustache templates for Clojure
......
src/metabase/driver/query_processor/expand.clj
+ 199
− 90
View file @ c0e9ea79
This diff is collapsed.
src/metabase/driver/query_processor/interface.clj
+ 128
− 43
View file @ c0e9ea79
......@@ -2,6 +2,9 @@
"Definitions of `Field`, `Value`, and other record types present in an expanded query.
This namespace should just contain definitions of various protocols and record types; associated logic
should go in `metabase.driver.query-processor.expand`."
(:require [schema.core :as s]
[metabase.models.field :as field]
[metabase.util :as u])
(:import clojure.lang.Keyword
java.sql.Timestamp))
......@@ -18,13 +21,13 @@
;; These are just used by the QueryExpander to record information about how joins should occur.
(defrecord JoinTableField [^Integer field-id
^String field-name])
(s/defrecord JoinTableField [field-id :- s/Int
field-name :- s/Str])
(defrecord JoinTable [^JoinTableField source-field
^JoinTableField pk-field
^Integer table-id
^String table-name])
(s/defrecord JoinTable [source-field :- JoinTableField
pk-field :- JoinTableField
table-id :- s/Int
table-name :- s/Str])
;;; # ------------------------------------------------------------ PROTOCOLS ------------------------------------------------------------
......@@ -37,19 +40,19 @@
;;; # ------------------------------------------------------------ "RESOLVED" TYPES: FIELD + VALUE ------------------------------------------------------------
;; Field is the expansion of a Field ID in the standard QL
(defrecord Field [^Integer field-id
^String field-name
^String field-display-name
^Keyword base-type
^Keyword special-type
^Integer table-id
^String schema-name
^String table-name
^Integer position
^String description
^Integer parent-id
;; Field once its resolved; FieldPlaceholder before that
parent]
(s/defrecord Field [field-id :- s/Int
field-name :- s/Str
field-display-name :- s/Str
base-type :- (apply s/enum field/base-types)
special-type :- (s/maybe (apply s/enum field/special-types))
table-id :- s/Int
schema-name :- (s/maybe s/Str)
table-name :- s/Str
position :- (s/maybe s/Int)
description :- (s/maybe s/Str)
parent-id :- (s/maybe s/Int)
;; Field once its resolved; FieldPlaceholder before that
parent :- s/Any]
IField
(qualified-name-components [this]
(conj (if parent
......@@ -57,51 +60,133 @@
[table-name])
field-name)))
(def ^:const datetime-field-units
"Valid units for a `DateTimeField`."
#{:default :minute :minute-of-hour :hour :hour-of-day :day :day-of-week :day-of-month :day-of-year
:week :week-of-year :month :month-of-year :quarter :quarter-of-year :year})
(def ^:const relative-datetime-value-units
"Valid units for a `RelativeDateTimeValue`."
#{:minute :hour :day :week :month :quarter :year})
(def DatetimeFieldUnit (s/named (apply s/enum datetime-field-units) "Valid datetime unit for a field"))
(def DatetimeValueUnit (s/named (apply s/enum relative-datetime-value-units) "Valid datetime unit for a relative datetime"))
(defn datetime-field-unit? [unit]
(contains? datetime-field-units (keyword unit)))
(defn relative-datetime-value-unit? [unit]
(contains? relative-datetime-value-units (keyword unit)))
;; wrapper around Field
(defrecord DateTimeField [^Field field
^Keyword unit])
(s/defrecord DateTimeField [field :- Field
unit :- DatetimeFieldUnit])
;; Value is the expansion of a value within a QL clause
;; Information about the associated Field is included for convenience
(defrecord Value [^Object value
^Field field])
(s/defrecord Value [value :- (s/maybe (s/cond-pre s/Bool s/Num s/Str))
field :- Field])
;; e.g. an absolute point in time (literal)
(defrecord DateTimeValue [^Timestamp value
^DateTimeField field])
(s/defrecord DateTimeValue [value :- Timestamp
field :- DateTimeField])
(def ^:const relative-datetime-value-units
"Valid units for a `RelativeDateTimeValue`."
#{:minute :hour :day :week :month :quarter :year})
(defn relative-datetime-value-unit? [unit]
(contains? relative-datetime-value-units (keyword unit)))
(defrecord RelativeDateTimeValue [^Integer amount
^Keyword unit
^DateTimeField field])
(s/defrecord RelativeDateTimeValue [amount :- s/Int
unit :- DatetimeValueUnit
field :- DateTimeField])
;;; # ------------------------------------------------------------ PLACEHOLDER TYPES: FIELDPLACEHOLDER + VALUEPLACEHOLDER ------------------------------------------------------------
;; Replace Field IDs with these during first pass
(defrecord FieldPlaceholder [^Integer field-id
^Integer fk-field-id
^Keyword datetime-unit])
(s/defrecord FieldPlaceholder [field-id :- s/Int
fk-field-id :- (s/maybe s/Int)
datetime-unit :- (s/maybe (apply s/enum datetime-field-units))])
(s/defrecord OrderByAggregateField [index :- s/Int]) ; e.g. 0
(def FieldPlaceholderOrAgRef (s/named (s/cond-pre FieldPlaceholder OrderByAggregateField) "Valid field (field ID or aggregate field reference)"))
(s/defrecord RelativeDatetime [amount :- s/Int
unit :- (s/maybe DatetimeValueUnit)])
(def LiteralDatetimeString (s/constrained s/Str u/date-string? "ISO-8601 datetime string literal"))
(def LiteralDatetime (s/named (s/cond-pre java.sql.Date LiteralDatetimeString) "Datetime literal (ISO-8601 string or java.sql.Date)"))
(def Datetime (s/named (s/cond-pre RelativeDatetime LiteralDatetime) "Valid datetime (ISO-8601 string literal or relative-datetime form)"))
(def OrderableValue (s/named (s/cond-pre s/Num Datetime) "Orderable value (number or datetime)"))
(def AnyValue (s/named (s/maybe (s/cond-pre s/Bool s/Str OrderableValue)) "Valid value (nil, boolean, number, string, or relative-datetime form)"))
;; Replace values with these during first pass over Query.
;; Include associated Field ID so appropriate the info can be found during Field resolution
(defrecord ValuePlaceholder [^FieldPlaceholder field-placeholder
^Keyword relative-unit
value])
(s/defrecord ValuePlaceholder [field-placeholder :- FieldPlaceholder
value :- AnyValue])
(def OrderableValuePlaceholder (s/both ValuePlaceholder {:field-placeholder s/Any, :value OrderableValue}))
(def StringValuePlaceholder (s/both ValuePlaceholder {:field-placeholder s/Any, :value s/Str}))
;; (def FieldOrAnyValue (s/named (s/cond-pre FieldPlaceholder ValuePlaceholder) "Field or value"))
;; (def FieldOrOrderableValue (s/named (s/cond-pre FieldPlaceholder OrderableValuePlaceholder) "Field or orderable value (number or datetime)"))
;; (def FieldOrStringValue (s/named (s/cond-pre FieldPlaceholder StringValuePlaceholder) "Field or string literal"))
;;; # ------------------------------------------------------------ CLAUSE SCHEMAS ------------------------------------------------------------
(def CountAggregation {:aggregation-type (s/eq :count)
(s/optional-key :field) FieldPlaceholder})
(def OtherAggregation {:aggregation-type (s/named (s/enum :avg :cumulative-sum :distinct :stddev :sum) "Valid aggregation type")
:field FieldPlaceholder})
(def Aggregation (s/named (s/if #(= (get % :aggregation-type) :count)
CountAggregation
OtherAggregation)
"Valid aggregation clause"))
(s/defrecord EqualityFilter [filter-type :- (s/enum := :!=)
field :- FieldPlaceholder
value :- ValuePlaceholder])
(s/defrecord ComparisonFilter [filter-type :- (s/enum :< :<= :> :>=)
field :- FieldPlaceholder
value :- OrderableValuePlaceholder])
(s/defrecord BetweenFilter [filter-type :- (s/eq :between)
min-val :- OrderableValuePlaceholder
field :- FieldPlaceholder
max-val :- OrderableValuePlaceholder])
(s/defrecord StringFilter [filter-type :- (s/enum :starts-with :contains :ends-with)
field :- FieldPlaceholder
value :- StringValuePlaceholder])
(def SimpleFilter (s/cond-pre EqualityFilter ComparisonFilter BetweenFilter StringFilter))
(s/defrecord CompoundFilter [compound-type :- (s/enum :and :or)
subclauses :- [(s/named (s/cond-pre SimpleFilter CompoundFilter) "Valid filter subclause in compound (and/or) filter")]])
(def Filter (s/named (s/cond-pre SimpleFilter CompoundFilter) "Valid filter clause"))
(def OrderBy (s/named {:field FieldPlaceholderOrAgRef
:direction (s/named (s/enum :ascending :descending) "Valid order-by direction")}
"Valid order-by subclause"))
(def Page (s/named {:page s/Int
:items s/Int}
"Valid page clause"))
(defrecord OrderByAggregateField [^Keyword source ; Name used in original query. Always :aggregation for right now
^Integer index]) ; e.g. 0
(def Query
{(s/optional-key :aggregation) Aggregation
(s/optional-key :breakout) [FieldPlaceholder]
(s/optional-key :fields) [FieldPlaceholderOrAgRef]
(s/optional-key :filter) Filter
(s/optional-key :limit) s/Int
(s/optional-key :order-by) [OrderBy]
(s/optional-key :page) Page
:source-table s/Int})
src/metabase/driver/query_processor/parse.clj
+ 9
− 13
View file @ c0e9ea79
(ns metabase.driver.query-processor.parse
"Logic relating to parsing values associated with different Query Processor `Field` types."
(:require [clojure.core.match :refer [match]]
[metabase.driver.query-processor.interface :refer :all]
(:require [metabase.driver.query-processor.interface :refer :all]
[metabase.util :as u])
(:import (metabase.driver.query_processor.interface DateTimeField
Field)))
Field
RelativeDatetime)))
(defprotocol IParseValueForField
(parse-value [this value]
......@@ -17,16 +17,12 @@
DateTimeField
(parse-value [this value]
(match value
(_ :guard u/date-string?)
(cond
(u/date-string? value)
(map->DateTimeValue {:field this, :value (u/->Timestamp value)})
["relative_datetime" "current"]
(map->RelativeDateTimeValue {:amount 0, :field this})
(instance? RelativeDatetime value)
(map->RelativeDateTimeValue {:field this, :amount (:amount value), :unit (:unit value)})
["relative_datetime" (amount :guard integer?) (unit :guard relative-datetime-value-unit?)]
(map->RelativeDateTimeValue {:amount amount
:field this
:unit (keyword unit)})
_ (throw (Exception. (format "Invalid value '%s': expected a DateTime." value))))))
:else
(throw (Exception. (format "Invalid value '%s': expected a DateTime." value))))))
test/metabase/driver/query_processor_test.clj
+ 107
− 79
View file @ c0e9ea79
......@@ -7,6 +7,7 @@
[metabase.db :refer :all]
[metabase.driver :as driver]
[metabase.driver.query-processor :refer :all]
[metabase.driver.query-processor.expand :as ql]
(metabase.models [field :refer [Field]]
[table :refer [Table]])
(metabase.test.data [dataset-definitions :as defs]
......@@ -14,7 +15,8 @@
[interface :refer [create-database-definition], :as i])
[metabase.test.data :refer :all]
[metabase.test.util.q :refer [Q]]
[metabase.util :as u]))
[metabase.util :as u]
[metabase.util.quotation :as q]))
;; ## Dataset-Independent QP Tests
......@@ -247,6 +249,27 @@
(def ^:private formatted-venues-rows (partial format-rows-by [int str int (partial u/round-to-decimals 4) (partial u/round-to-decimals 4) int]))
(defn- rows [results]
(or (-> results :data :rows)
(throw (ex-info "Error!" results))))
(defn- $->id [body]
(clojure.walk/prewalk (fn [form]
(if (and (symbol? form)
(= (first (name form)) \$))
`(~'id ~(keyword (apply str (rest (name form)))))
form))
body))
(defmacro ^:private query
{:style/indent 1}
[table & forms]
`(let [table-id# (id ~(keyword table))
~'id (partial id ~(keyword table))]
(ql/run-query (ql/source-table table-id#)
~@(map $->id forms))))
;; # THE TESTS THEMSELVES (!)
;; structured-query?
......@@ -268,8 +291,9 @@
{:rows [[100]]
:columns ["count"]
:cols [(aggregate-col :count)]}
(Q aggregate count of venues
return (format-rows-by [int])))
(->> (query venues
(ql/aggregation :count))
(format-rows-by [int])))
;; ### "SUM" AGGREGATION
......@@ -277,8 +301,9 @@
{:rows [[203]]
:columns ["sum"]
:cols [(aggregate-col :sum (venues-col :price))]}
(Q aggregate sum price of venues
return (format-rows-by [int])))
(->> (query venues
(ql/aggregation (ql/sum $price)))
(format-rows-by [int])))
;; ## "AVG" AGGREGATION
......@@ -286,8 +311,9 @@
{:rows [[35.5059]]
:columns ["avg"]
:cols [(aggregate-col :avg (venues-col :latitude))]}
(Q aggregate avg latitude of venues
return (format-rows-by [(partial u/round-to-decimals 4)])))
(->> (query venues
(ql/aggregation (ql/avg $latitude)))
(format-rows-by [(partial u/round-to-decimals 4)])))
;; ### "DISTINCT COUNT" AGGREGATION
......@@ -337,15 +363,15 @@
;; ### PAGE - Get the second page
(datasets/expect-with-all-engines
[[ 6 "Bakery"]
[ 7 "Bar"]
[ 8 "Beer Garden"]
[ 9 "Breakfast / Brunch"]
[10 "Brewery"]]
(Q aggregate rows of categories
page 2 items 5
order id+
return rows (format-rows-by [int str])))
[[ 6 "Bakery"]
[ 7 "Bar"]
[ 8 "Beer Garden"]
[ 9 "Breakfast / Brunch"]
[10 "Brewery"]]
(->> (query categories
(ql/page {:page 2, :items 5})
(ql/order-by [$id :ascending]))
rows (format-rows-by [int str])))
;; ## "ORDER_BY" CLAUSE
......@@ -361,11 +387,13 @@
[2 9 833]
[2 8 380]
[2 5 719]]
(Q aggregate rows of checkins
fields venue_id user_id id
order venue_id+ user_id- id+
limit 10
return rows (format-rows-by [int int int])))
(->> (query checkins
(ql/fields $venue_id $user_id $id)
(ql/order-by [$venue_id :ascending]
[$user_id :descending]
[$id :ascending])
(ql/limit 10))
rows (format-rows-by [int int int])))
;; ## "FILTER" CLAUSE
......@@ -377,10 +405,11 @@
[77 "Sushi Nakazawa" 40 40.7318 -74.0045 4]
[79 "Sushi Yasuda" 40 40.7514 -73.9736 4]
[81 "Tanoshi Sushi & Sake Bar" 40 40.7677 -73.9533 4]]
(Q aggregate rows of venues
filter and > id 50, >= price 4
order id+
return rows formatted-venues-rows))
(-> (query venues
(ql/filter (ql/and (ql/> $id 50)
(ql/>= $price 4)))
(ql/order-by [$id :ascending]))
rows formatted-venues-rows))
(defmacro compaare [a b]
`(compare-expr ~a ~b '~a '~b))
......@@ -389,20 +418,23 @@
(datasets/expect-with-all-engines
[[21 "PizzaHacker" 58 37.7441 -122.421 2]
[23 "Taqueria Los Coyotes" 50 37.765 -122.42 2]]
(Q aggregate rows of venues
filter and < id 24, > id 20, != id 22
order id+
return rows formatted-venues-rows))
(-> (query venues
(ql/filter (ql/and (ql/< $id 24)
(ql/> $id 20)
(ql/!= $id 22)))
(ql/order-by [$id :ascending]))
rows formatted-venues-rows))
;; ### FILTER WITH A FALSE VALUE
;; Check that we're checking for non-nil values, not just logically true ones.
;; There's only one place (out of 3) that I don't like
(datasets/expect-with-all-engines
[[1]]
(Q dataset places-cam-likes
return rows (format-rows-by [int])
aggregate count of places
filter = liked false))
(->> (with-db (get-or-create-database! defs/places-cam-likes)
(query places
(ql/aggregation :count)
(ql/filter (ql/= $liked false))))
rows (format-rows-by [int])))
(defn- ->bool [x] ; SQLite returns 0/1 for false/true;
(condp = x ; Redshift returns nil/true.
......@@ -1094,10 +1126,10 @@
(datasets/expect-with-all-engines
[[41 "Cheese Steak Shop" 18 37.7855 -122.44 1]
[74 "Chez Jay" 2 34.0104 -118.493 2]]
(Q aggregate rows of venues
filter starts-with name "Che"
order id
return rows formatted-venues-rows))
(-> (query venues
(ql/filter (ql/starts-with $name "Che"))
(ql/order-by [$id :ascending]))
rows formatted-venues-rows))
;;; ## ENDS_WITH
......@@ -1137,15 +1169,17 @@
(datasets/expect-with-all-engines
[[81]]
(Q aggregate count of venues
filter = price 1 2
return rows (format-rows-by [int])))
(->> (query venues
(ql/aggregation :count)
(ql/filter (ql/= $price 1 2)))
rows (format-rows-by [int])))
(datasets/expect-with-all-engines
[[19]]
(Q aggregate count of venues
filter != price 1 2
return rows (format-rows-by [int])))
(->> (query venues
(ql/aggregation :count)
(ql/filter (ql/!= $price 1 2)))
rows (format-rows-by [int])))
;; +-------------------------------------------------------------------------------------------------------------+
......@@ -1156,12 +1190,13 @@
;; ## BUCKETING
(defn- sad-toucan-incidents-with-bucketing [unit]
(vec (Q dataset sad-toucan-incidents
aggregate count of incidents
breakout ["datetime_field" (id :incidents :timestamp) "as" unit]
limit 10
return rows (format-rows-by [(fn [x] (if (number? x) (int x) x))
int]))))
(->> (with-db (get-or-create-database! defs/sad-toucan-incidents)
(query incidents
(ql/aggregation :count)
(ql/breakout (ql/datetime-field $timestamp unit))
(ql/limit 10)))
rows (format-rows-by [(fn [x] (if (number? x) (int x) x))
int])))
(datasets/expect-with-all-engines
(cond
......@@ -1428,10 +1463,12 @@
(def ^:private checkins:1-per-day (partial database-def-with-timestamps (* 60 60 24)))
(defn- count-of-grouping [db field-grouping & relative-datetime-args]
(with-temp-db [_ db]
(Q aggregate count of checkins
filter = ["datetime_field" (id :checkins :timestamp) "as" (name field-grouping)] (apply vector "relative_datetime" relative-datetime-args)
return first-row first int)))
(-> (with-temp-db [_ db]
(query checkins
(ql/aggregation :count)
(ql/filter (ql/= (ql/datetime-field $timestamp field-grouping)
(apply ql/relative-datetime relative-datetime-args)))))
rows first first int))
(datasets/expect-with-all-engines 4 (count-of-grouping (checkins:4-per-minute) :minute "current"))
(datasets/expect-with-all-engines 4 (count-of-grouping (checkins:4-per-minute) :minute -1 "minute"))
......@@ -1450,41 +1487,32 @@
;; SYNTACTIC SUGAR
(datasets/expect-with-all-engines
1
(with-temp-db [_ (checkins:1-per-day)]
(-> (driver/process-query
{:database (id)
:type :query
:query {:source_table (id :checkins)
:aggregation ["count"]
:filter ["TIME_INTERVAL" (id :checkins :timestamp) "current" "day"]}})
:data :rows first first int)))
(-> (with-temp-db [_ (checkins:1-per-day)]
(query checkins
(ql/aggregation :count)
(ql/filter (ql/time-interval $timestamp :current :day))))
rows first first int))
(datasets/expect-with-all-engines
7
(with-temp-db [_ (checkins:1-per-day)]
(-> (driver/process-query
{:database (id)
:type :query
:query {:source_table (id :checkins)
:aggregation ["count"]
:filter ["TIME_INTERVAL" (id :checkins :timestamp) "last" "week"]}})
:data :rows first first int)))
(-> (with-temp-db [_ (checkins:1-per-day)]
(query checkins
(ql/aggregation :count)
(ql/filter (ql/time-interval $timestamp :last :week))))
rows first first int))
;; Make sure that when referencing the same field multiple times with different units we return the one
;; that actually reflects the units the results are in.
;; eg when we breakout by one unit and filter by another, make sure the results and the col info
;; use the unit used by breakout
(defn- date-bucketing-unit-when-you [& {:keys [breakout-by filter-by]}]
(with-temp-db [_ (checkins:1-per-day)]
(let [results (driver/process-query
{:database (id)
:type :query
:query {:source_table (id :checkins)
:aggregation ["count"]
:breakout [["datetime_field" (id :checkins :timestamp) "as" breakout-by]]
:filter ["TIME_INTERVAL" (id :checkins :timestamp) "current" filter-by]}})]
{:rows (-> results :row_count)
:unit (-> results :data :cols first :unit)})))
(let [results (with-temp-db [_ (checkins:1-per-day)]
(query checkins
(ql/aggregation :count)
(ql/breakout (ql/datetime-field $timestamp breakout-by))
(ql/filter (ql/time-interval $timestamp :current filter-by))))]
{:rows (-> results :row_count)
:unit (-> results :data :cols first :unit)}))
(datasets/expect-with-all-engines
{:rows 1, :unit :day}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment