diff --git a/dev/src/dev.clj b/dev/src/dev.clj index ce5b6e8de1694f8969d9d7c4e26ee029dc38d1ee..fe7b2c8e185661d87fb0f6c95ac96a221bed7236 100644 --- a/dev/src/dev.clj +++ b/dev/src/dev.clj @@ -33,15 +33,35 @@ (stop!) (start!)) -(defn reload-run-tests - [& ns-names] - (doseq [ns-name ns-names] - (require ns-name :reload)) - (expectations/run-tests ns-names)) - -(defn run-tests - [& ns-names] - (expectations/run-tests ns-names)) +(defn ns-unmap-all + "Unmap all interned vars in a namespace. Reset the namespace to a blank slate! Perfect for when you rename everything + and want to make sure you didn't miss a reference or when you redefine a multimethod. + + (ns-unmap-all *ns*)" + [a-namespace] + (doseq [[symb] (ns-interns a-namespace)] + (ns-unmap a-namespace symb))) + +(defn run-tests* + [& namespaces] + (let [namespaces (map the-ns namespaces)] + (doseq [a-namespace namespaces] + (ns-unmap-all a-namespace) + (require (ns-name a-namespace) :reload)) + (expectations/run-tests namespaces))) + +(defmacro run-tests + "Run expectations test in `namespaces`. With no args, runs tests in the current namespace. Clears all interned vars in + the namespace, reloads it, and runs tests. `namespaces` may be either actual namespace objects or their symbol + names. + + (run-tests) ; current-namespace + (run-tests *ns*) ; current-namespace + (run-tests 'my-ns) ; run tests in my-ns" + ([] + `(run-tests* '~(ns-name *ns*))) + ([& namespaces] + `(run-tests* ~@(map #(list 'quote (ns-name (the-ns (eval %)))) namespaces)))) (defmacro require-model "Rather than requiring all models inn the ns declaration, make it easy to require the ones you need for your current session" diff --git a/src/metabase/query_processor/middleware/parameters.clj b/src/metabase/query_processor/middleware/parameters.clj index 9b0ad10643c7bb9ac4aaa82c51c8b63a9eb66e0c..ed2d48bca10db02b07f906c431d160135c4ac47d 100644 --- a/src/metabase/query_processor/middleware/parameters.clj +++ b/src/metabase/query_processor/middleware/parameters.clj @@ -1,8 +1,6 @@ (ns metabase.query-processor.middleware.parameters "Middleware for substituting parameters in queries." - (:require [clojure - [data :as data] - [set :as set]] + (:require [clojure.data :as data] [clojure.tools.logging :as log] [metabase.mbql [normalize :as normalize] @@ -11,7 +9,7 @@ [metabase.query-processor.interface :as i] [metabase.query-processor.middleware.parameters [mbql :as params.mbql] - [sql :as params.sql]] + [native :as params.native]] [metabase.util :as u] [schema.core :as s])) @@ -40,21 +38,11 @@ (cond-> expanded (join? m) move-join-condition-to-source-query))) -(defn- expand-native-params [outer-query {:keys [parameters], is-source-query? :native, :as m}] - ;; HACK totally rediculous, but top-level native queries use the key `:query` for SQL or equivalent, while native - ;; source queries use `:native`; rename `:native` to `:query` so the `params.sql/` code, which thinks it's always - ;; operation on top-level queries, works as expected. - ;; - ;; TODO - Like the MBQL stuff, `params.sql` should be fixed so it operates on any level instead of only top-level. - (let [wrapped (assoc outer-query :native (set/rename-keys m {:native :query})) - {expanded :native} (params.sql/expand (dissoc wrapped :parameters) parameters)] - ;; remove `:parameters` and `:template-tags` now that we've spliced them in to the query - (cond-> expanded - ;; rename `:query` back to `:native` if this was a native source query - is-source-query? (set/rename-keys {:query :native})))) +(defn- expand-native-params [_ m] + (params.native/expand-inner m)) (defn- expand-one - "Expand `:parameters` in one [inner-query style] map that contains them." + "Expand `:parameters` in one inner-query-style map that contains them." [outer-query {:keys [source-table source-query parameters], :as m}] ;; HACK - normalization does not yet operate on `:parameters` that aren't at the top level, so double-check that ;; they're normalized properly before proceeding. diff --git a/src/metabase/query_processor/middleware/parameters/native.clj b/src/metabase/query_processor/middleware/parameters/native.clj new file mode 100644 index 0000000000000000000000000000000000000000..3ce365ea4ca860c06e5c944e4dfd13ad4410d1c3 --- /dev/null +++ b/src/metabase/query_processor/middleware/parameters/native.clj @@ -0,0 +1,56 @@ +(ns metabase.query-processor.middleware.parameters.native + "Param substitution for *SQL* queries. + + This is a new implementation, fondly referred to as 'SQL parameters 2.0', written for v0.23.0. The new + implementation uses prepared statement args instead of substituting them directly into the query, and is much + better-organized and better-documented. + + The Basics: + + * Things like `{{x}}` (required params) get subsituted with the value of `:x`, which can be a literal used in a + clause (e.g. in a clause like `value = {{x}}`) or a \"field filter\" that handles adding the clause itself + (e.g. `{{timestamp}}` might become `timestamp BETWEEN ? AND ?`). + + * Things like `[[AND {{x}]]` are optional params. If the param (`:x`) isn't specified, the *entire* clause inside + `[[...]]` is replaced with an empty string; If it is specified, the value inside the curly brackets `{{x}}` is + replaced as usual and the rest of the clause (`AND ...`) is included in the query as-is + + Various `metabase.query-processor.middleware.parameters.native.*` namespaces implement different steps of this + process, which are as follows: + + 1. `values` parses `:parameters` passed in as arguments to the query and returns a map of param key -> value + + 2. `parse` takes a SQL query string and breaks it out into a series of string fragments interleaved with objects + representing optional and non-optional params + + 3. `substitute` (and the related namespace `substitution`) replace optional and param objects with appropriate SQL + snippets and prepared statement args, and combine the sequence of fragments back into a single SQL string." + (:require [metabase.driver :as driver] + [metabase.query-processor.middleware.parameters.native + [parse :as parse] + [substitute :as substitute] + [values :as values]])) + +(defn expand-inner + "Expand parameters inside an *inner* native `query`. Not recursive -- recursive transformations are handled in + the `middleware.parameters` functions that invoke this function." + [{:keys [parameters query native] :as inner-query}] + (if-not (driver/supports? driver/*driver* :native-parameters) + inner-query + ;; Totally ridiculous, but top-level native queries use the key `:query` for SQL or equivalent, while native + ;; source queries use `:native`. So we need to handle either case. + (let [query (or query native)] + ;; only SQL is officially supported rn! We can change this in the future. But we will probably want separate + ;; implementations of `parse` for other drivers, such as ones with JSON-based query languages. I think? + (if-not (string? query) + inner-query + (merge + (dissoc inner-query :parameters :template-tags) + (let [[query params] (-> query + parse/parse + (substitute/substitute (values/query->params-map inner-query)))] + (merge + (if (:query inner-query) + {:query query} + {:native query}) + {:params params}))))))) diff --git a/src/metabase/query_processor/middleware/parameters/native/interface.clj b/src/metabase/query_processor/middleware/parameters/native/interface.clj new file mode 100644 index 0000000000000000000000000000000000000000..0179ad5b4644c3ecfd732486fa4441cc7467602c --- /dev/null +++ b/src/metabase/query_processor/middleware/parameters/native/interface.clj @@ -0,0 +1,106 @@ +(ns metabase.query-processor.middleware.parameters.native.interface + "Various record types below are used as a convenience for differentiating the different param types." + (:require [metabase.util.schema :as su] + [potemkin.types :as p.types] + [pretty.core :refer [PrettyPrintable]] + [schema.core :as s])) + +;; "FieldFilter" is something that expands to a clause like "some_field BETWEEN 1 AND 10" +;; +;; `field` is a Field Toucan instance +;; +;; `value`" is either: +;; * `no-value` +;; * A map contianing the value and type info for the value, e.g. +;; +;; {:type :date/single +;; :value #inst "2019-09-20T19:52:00.000-07:00"} +;; +;; * A vector of maps like the one above (for multiple values) +(p.types/defrecord+ FieldFilter [field value] + PrettyPrintable + (pretty [this] + (list 'map->FieldFilter (into {} this)))) + +(defn FieldFilter? + "Is `x` an instance of the `FieldFilter` record type?" + [x] + (instance? FieldFilter x)) + +;; as in a literal date, defined by date-string S +;; `s` is a String +(p.types/defrecord+ Date [s] + PrettyPrintable + (pretty [_] + (list 'Date. s))) + +(p.types/defrecord+ DateRange [start end] + PrettyPrintable + (pretty [_] + (list 'DateRange. start end))) + +;; List of numbers to faciliate things like using params in a SQL `IN` clause. See the discussion in `value->number` +;; for more details. +;; `numbers` are a sequence of `[java.lang.Number]` +(p.types/defrecord+ CommaSeparatedNumbers [numbers] + PrettyPrintable + (pretty [_] + (list 'CommaSeperatedNumbers. numbers))) + +(def no-value + "Convenience for representing an *optional* parameter present in a query but whose value is unspecified in the param + values." + ::no-value) + +(def SingleValue + "Schema for a valid *single* value for a param. As of 0.28.0 params can either be single-value or multiple value." + (s/cond-pre (s/eq no-value) + CommaSeparatedNumbers + FieldFilter + Date + s/Num + s/Str + s/Bool)) + +(def ParamValue + "Schema for a parameter *value* during parsing by the `values` namespace, and also (confusingly) for the `:value` part + of a `FieldFilter`, which gets passed along to `substitution`. TODO - this is horribly confusing" + {:type s/Keyword ; TODO - what types are allowed? :text, ...? + (s/optional-key :target) s/Any + ;; not specified if the param has no value. TODO - make this stricter + (s/optional-key :value) s/Any + ;; The following are not used by the code in this namespace but may or may not be specified depending on what the + ;; code that constructs the query params is doing. We can go ahead and ignore these when present. + (s/optional-key :slug) su/NonBlankString + (s/optional-key :name) su/NonBlankString + (s/optional-key :default) s/Any + (s/optional-key :id) s/Any}) ; used internally by the frontend + +;; Sequence of multiple values for generating a SQL IN() clause. vales +;; `values` are a sequence of `[SingleValue]` +(p.types/defrecord+ MultipleValues [values] + PrettyPrintable + (pretty [_] + (list 'MultipleValues. values))) + +(p.types/defrecord+ Param [k] + PrettyPrintable + (pretty [_] + (list 'param k))) + +(p.types/defrecord+ Optional [args] + PrettyPrintable + (pretty [_] + (cons 'optional args))) + +;; `Param?` and `Optional?` exist mostly so you don't have to try to import the classes from this namespace which can +;; cause problems if the ns isn't loaded first +(defn Param? + "Is `x` an instance of the `Param` record type?" + [x] + (instance? Param x)) + +(defn Optional? + "Is `x` an instance of the `Optional` record type?" + [x] + (instance? Optional x)) diff --git a/src/metabase/query_processor/middleware/parameters/native/parse.clj b/src/metabase/query_processor/middleware/parameters/native/parse.clj new file mode 100644 index 0000000000000000000000000000000000000000..cc5b7287a662af67ef1c687bfcd879186cbec1de --- /dev/null +++ b/src/metabase/query_processor/middleware/parameters/native/parse.clj @@ -0,0 +1,93 @@ +(ns metabase.query-processor.middleware.parameters.native.parse + (:require [clojure.string :as str] + [metabase.query-processor.middleware.parameters.native.interface :as i] + [metabase.util.i18n :refer [tru]] + [schema.core :as s]) + (:import [metabase.query_processor.middleware.parameters.native.interface Optional Param])) + +(def ^:private StringOrToken (s/cond-pre s/Str (s/enum :optional-begin :param-begin :end))) + +(defn- split-on-token [^String s token-str token] + (when-let [index (str/index-of s token-str)] + (let [before (.substring s 0 index) + after (.substring s (+ index (count token-str)) (count s))] + [before token after]))) + +(defn- tokenize-one [s token-str token] + (loop [acc [], s s] + (if (empty? s) + acc + (if-let [[before token after] (split-on-token s token-str token)] + (recur (into acc [before token]) after) + (conj acc s))))) + +(s/defn ^:private tokenize :- [StringOrToken] + [s :- s/Str] + (reduce + (fn [strs [token-str token]] + (filter + (some-fn keyword? seq) + (mapcat + (fn [s] + (if-not (string? s) + [s] + (tokenize-one s token-str token))) + strs))) + [s] + [["[[" :optional-begin] + ["]]" :end] + ["{{" :param-begin] + ["}}" :end]])) + +(defn- param [& [k & more]] + (when (or (seq more) + (not (string? k))) + (throw (Exception. (tru "Invalid '{{...}}' clause: expected a param name")))) + (let [k (str/trim k)] + (when (empty? k) + (throw (Exception. (tru "'{{...}}' clauses cannot be empty.")))) + (i/->Param k))) + +(defn- optional [& parsed] + (when-not (some i/Param? parsed) + (throw (Exception. (tru "'[[...]]' clauses must contain at least one '{{...}}' clause.")))) + (i/->Optional parsed)) + +(def ^:private ParsedToken (s/cond-pre s/Str Param Optional)) + +(declare parse-tokens) + +(s/defn ^:private parse-tokens :- [(s/one [ParsedToken] "parsed tokens") (s/one [StringOrToken] "remaining tokens")] + ([tokens :- [StringOrToken]] + (parse-tokens tokens 0)) + + ([tokens :- [StringOrToken], level] + (loop [acc [], [token & more] tokens] + (condp = token + nil + (if (pos? level) + (throw + (IllegalArgumentException. (tru "Invalid query: found '[[' or '{{' with no matching ']]' or '}}'"))) + [acc nil]) + + :optional-begin + (let [[parsed more] (parse-tokens more (inc level))] + (recur (conj acc (apply optional parsed)) more)) + + :param-begin + (let [[parsed more] (parse-tokens more (inc level))] + (recur (conj acc (apply param parsed)) more)) + + :end + (if (zero? level) + (throw + (IllegalArgumentException. (tru "Invalid query: found ']]' or '}}' with no matching '[[' or '{{'"))) + [acc more]) + + (recur (conj acc token) more))))) + +(s/defn parse :- [(s/cond-pre s/Str Param Optional)] + "Attempts to parse SQL string `s`. Parses any optional clauses or parameters found, and returns a sequence of SQL + query fragments Strings (possibly) interposed with `Param` or `Optional` instances." + [s :- s/Str] + (-> s tokenize parse-tokens first)) diff --git a/src/metabase/query_processor/middleware/parameters/native/substitute.clj b/src/metabase/query_processor/middleware/parameters/native/substitute.clj new file mode 100644 index 0000000000000000000000000000000000000000..915ce843d4989aba6dfe82f4e7894df2aed56df7 --- /dev/null +++ b/src/metabase/query_processor/middleware/parameters/native/substitute.clj @@ -0,0 +1,66 @@ +(ns metabase.query-processor.middleware.parameters.native.substitute + (:require [clojure.string :as str] + [metabase.query-processor.middleware.parameters.native + [interface :as i] + [substitution :as substitution]] + [metabase.util.i18n :refer [tru]])) + +(defn- substitute-field-filter [param->value [sql args missing] in-optional? k {:keys [field value], :as v}] + (if (and (= i/no-value value) in-optional?) + ;; no-value field filters inside optional clauses are ignored, and eventually emitted entirely + [sql args (conj missing k)] + ;; otherwise no values get replaced with `1 = 1` and other values get replaced normally + (let [{:keys [replacement-snippet prepared-statement-args]} (substitution/->replacement-snippet-info v)] + [(str sql replacement-snippet) (concat args prepared-statement-args) missing]))) + +(defn- substitute-param [param->value [sql args missing] in-optional? {:keys [k]}] + (if-not (contains? param->value k) + [sql args (conj missing k)] + (let [v (get param->value k)] + (cond + (i/FieldFilter? v) + (substitute-field-filter param->value [sql args missing] in-optional? k v) + + (= i/no-value v) + [sql args (conj missing k)] + + :else + (let [{:keys [replacement-snippet prepared-statement-args]} (substitution/->replacement-snippet-info v)] + [(str sql replacement-snippet) (concat args prepared-statement-args) missing]))))) + +(declare substitute*) + +(defn- substitute-optional [param->value [sql args missing] in-optional? {subclauses :args}] + (let [[opt-sql opt-args opt-missing] (substitute* param->value subclauses true)] + (if (seq opt-missing) + [sql args missing] + [(str sql opt-sql) (concat args opt-args) missing]))) + +(defn- substitute* [param->value parsed in-optional?] + (reduce + (fn [[sql args missing] x] + (cond + (string? x) + [(str sql x) args missing] + + (i/Param? x) + (substitute-param param->value [sql args missing] in-optional? x) + + (i/Optional? x) + (substitute-optional param->value [sql args missing] in-optional? x))) + nil + parsed)) + +(defn substitute + "Substitute `Optional` and `Param` objects in a `parsed-query`, a sequence of parsed string fragments and tokens, with + the values from the map `param->value` (using logic from `substitution` to decide what replacement SQL should be + generated). + + (substitute [\"select * from foobars where bird_type = \" (param \"bird_type\")] + {\"bird_type\" \"Steller's Jay\"}) + ;; -> [\"select * from foobars where bird_type = ?\" [\"Steller's Jay\"]]" + [parsed-query param->value] + (let [[sql args missing] (substitute* param->value parsed-query false)] + (when (seq missing) + (throw (Exception. (tru "Cannot run query: missing required parameters: {0}" (set missing))))) + [(str/trim sql) args])) diff --git a/src/metabase/query_processor/middleware/parameters/native/substitution.clj b/src/metabase/query_processor/middleware/parameters/native/substitution.clj new file mode 100644 index 0000000000000000000000000000000000000000..874e82cdaa4a43f9914bca234942b7fe0cc5e527 --- /dev/null +++ b/src/metabase/query_processor/middleware/parameters/native/substitution.clj @@ -0,0 +1,188 @@ +(ns metabase.query-processor.middleware.parameters.native.substitution + "These functions take the info for a param fetched by the functions above and add additional info about how that param + should be represented as SQL. (Specifically, they return information in this format: + + {;; appropriate SQL that should be used to replace the param snippet, e.g. {{x}} + :replacement-snippet \"= ?\" + ;; ; any prepared statement args (values for `?` placeholders) needed for the replacement snippet + :prepared-statement-args [#inst \"2017-01-01\"]}" + (:require [clojure.string :as str] + [honeysql.core :as hsql] + [metabase.driver :as driver] + [metabase.driver.sql :as sql] + [metabase.driver.sql.query-processor :as sql.qp] + [metabase.query-processor.middleware.parameters.dates :as date-params] + [metabase.query-processor.middleware.parameters.native.interface :as i] + [metabase.util + [date :as du] + [schema :as su]] + [schema.core :as s]) + (:import clojure.lang.Keyword + honeysql.types.SqlCall + java.util.UUID + [metabase.query_processor.middleware.parameters.native.interface CommaSeparatedNumbers Date DateRange + FieldFilter MultipleValues])) + +(def ^:private ParamSnippetInfo + {(s/optional-key :replacement-snippet) s/Str ; allowed to be blank if this is an optional param + (s/optional-key :prepared-statement-args) [s/Any]}) + +(defmulti ->replacement-snippet-info + "Return information about how `value` should be converted to SQL, as a map with keys `:replacement-snippet` and + `:prepared-statement-args`. + + (->replacement-snippet-info \"ABC\") -> {:replacement-snippet \"?\", :prepared-statement-args \"ABC\"}" + {:arglists '([value])} + class) + +(defn- create-replacement-snippet [nil-or-obj] + (let [{:keys [sql-string param-values]} (sql/->prepared-substitution driver/*driver* nil-or-obj)] + {:replacement-snippet sql-string + :prepared-statement-args param-values})) + +(defmethod ->replacement-snippet-info nil + [this] + (create-replacement-snippet this)) + +(defmethod ->replacement-snippet-info Object + [this] + (create-replacement-snippet (str this))) + +(defmethod ->replacement-snippet-info Number + [this] + (create-replacement-snippet this)) + +(defmethod ->replacement-snippet-info Boolean + [this] + (create-replacement-snippet this)) + +(defmethod ->replacement-snippet-info Keyword + [this] + (if (= this i/no-value) + {:replacement-snippet ""} + (create-replacement-snippet this))) + +(defmethod ->replacement-snippet-info SqlCall + [this] + (create-replacement-snippet this)) + +(defmethod ->replacement-snippet-info UUID + [this] + {:replacement-snippet (format "CAST('%s' AS uuid)" (str this))}) + +(defmethod ->replacement-snippet-info CommaSeparatedNumbers + [{:keys [numbers]}] + {:replacement-snippet (str/join ", " numbers)}) + +(defmethod ->replacement-snippet-info MultipleValues + [{:keys [values]}] + (let [values (map ->replacement-snippet-info values)] + {:replacement-snippet (str/join ", " (map :replacement-snippet values)) + :prepared-statement-args (apply concat (map :prepared-statement-args values))})) + +(defmethod ->replacement-snippet-info Date + [{:keys [s]}] + (create-replacement-snippet (du/->Timestamp s))) + +(defn- prepared-ts-subs [operator date-str] + (let [{:keys [sql-string param-values]} (sql/->prepared-substitution driver/*driver* (du/->Timestamp date-str))] + {:replacement-snippet (str operator " " sql-string) + :prepared-statement-args param-values})) + +(defmethod ->replacement-snippet-info DateRange + [{:keys [start end]}] + (cond + (= start end) + (prepared-ts-subs \= start) + + (nil? start) + (prepared-ts-subs \< end) + + (nil? end) + (prepared-ts-subs \> start) + + :else + (let [params (map (comp #(sql/->prepared-substitution driver/*driver* %) du/->Timestamp) [start end])] + {:replacement-snippet (apply format "BETWEEN %s AND %s" (map :sql-string params)), + :prepared-statement-args (vec (mapcat :param-values params))}))) + + +;;; ------------------------------------- Field Filter replacement snippet info -------------------------------------- + +(s/defn ^:private combine-replacement-snippet-maps :- ParamSnippetInfo + "Combine multiple `replacement-snippet-maps` into a single map using a SQL `AND` clause." + [replacement-snippet-maps :- [ParamSnippetInfo]] + {:replacement-snippet (str \( (str/join " AND " (map :replacement-snippet replacement-snippet-maps)) \)) + :prepared-statement-args (reduce concat (map :prepared-statement-args replacement-snippet-maps))}) + +(defn- relative-date-param-type? [param-type] + (contains? #{:date/range :date/month-year :date/quarter-year :date/relative :date/all-options} param-type)) + +;; for relative dates convert the param to a `DateRange` record type and call `->replacement-snippet-info` on it +(s/defn ^:private relative-date-field-filter->replacement-snippet-info :- ParamSnippetInfo + [value] + ;; TODO - get timezone from query dict + (-> (date-params/date-string->range value (.getID du/*report-timezone*)) + i/map->DateRange + ->replacement-snippet-info)) + +(s/defn ^:private field-filter->equals-clause-sql :- ParamSnippetInfo + [value] + (-> (->replacement-snippet-info value) + (update :replacement-snippet (partial str "= ")))) + +(s/defn ^:private field-filter-multiple-values->in-clause-sql :- ParamSnippetInfo + [values] + (-> (i/map->MultipleValues {:values values}) + ->replacement-snippet-info + (update :replacement-snippet (partial format "IN (%s)")))) + +(s/defn ^:private field-filter->replacement-snippet-info :- ParamSnippetInfo + "Return `[replacement-snippet & prepared-statement-args]` appropriate for a field filter parameter." + [{param-type :type, value :value} :- i/ParamValue] + (cond + ;; convert relative dates to approprate date range representations + (relative-date-param-type? param-type) (relative-date-field-filter->replacement-snippet-info value) + ;; convert all other dates to `= <date>` + (date-params/date-type? param-type) (field-filter->equals-clause-sql (i/map->Date {:s value})) + ;; for sequences of multiple values we want to generate an `IN (...)` clause + (sequential? value) (field-filter-multiple-values->in-clause-sql value) + ;; convert everything else to `= <value>` + :else (field-filter->equals-clause-sql value))) + +(s/defn ^:private honeysql->replacement-snippet-info :- ParamSnippetInfo + "Convert `x` to a replacement snippet info map by passing it to HoneySQL's `format` function." + [x] + (let [[snippet & args] (hsql/format x, :quoting (sql.qp/quote-style driver/*driver*), :allow-dashed-names? true)] + {:replacement-snippet snippet + :prepared-statement-args args})) + +(s/defn ^:private field->identifier :- su/NonBlankString + "Return an approprate snippet to represent this `field` in SQL given its param type. + For non-date Fields, this is just a quoted identifier; for dates, the SQL includes appropriately bucketing based on + the `param-type`." + [field param-type] + (:replacement-snippet + (honeysql->replacement-snippet-info + (let [identifier (sql.qp/->honeysql driver/*driver* (sql.qp/field->identifier driver/*driver* field))] + (if (date-params/date-type? param-type) + (sql.qp/date driver/*driver* :day identifier) + identifier))))) + +(defmethod ->replacement-snippet-info FieldFilter + [{:keys [field value], :as field-filter}] + (cond + ;; otherwise if the value isn't present just put in something that will always be true, such as `1` (e.g. `WHERE 1 + ;; = 1`). This is only used for field filters outside of optional clauses + (= value i/no-value) {:replacement-snippet "1 = 1"} + ;; if we have a vector of multiple values recursively convert them to SQL and combine into an `AND` clause + ;; (This is multiple values in the sense that the frontend provided multiple maps with value values for the same + ;; FieldFilter, not in the sense that we have a single map with multiple values for `:value`.) + (vector? value) + (combine-replacement-snippet-maps (for [v value] + (->replacement-snippet-info (assoc field-filter :value v)))) + ;; otherwise convert single value to SQL. + ;; Convert the value to a replacement snippet info map and then tack on the field identifier to the front + :else + (update (field-filter->replacement-snippet-info value) + :replacement-snippet (partial str (field->identifier field (:type value)) " ")))) diff --git a/src/metabase/query_processor/middleware/parameters/native/values.clj b/src/metabase/query_processor/middleware/parameters/native/values.clj new file mode 100644 index 0000000000000000000000000000000000000000..1c12f18b448a553c83d8b900ffb3d8e4cbf8b846 --- /dev/null +++ b/src/metabase/query_processor/middleware/parameters/native/values.clj @@ -0,0 +1,226 @@ +(ns metabase.query-processor.middleware.parameters.native.values + "These functions build a map of information about the types and values of the params used in a query. (These functions + don't parse the query itself, but instead look at the values of `:template-tags` and `:parameters` passed along with + the query.) + + (query->params-map some-query) + ;; -> {\"checkin_date\" {:field {:name \"date\", :parent_id nil, :table_id 1375} + :param {:type \"date/range\" + :target [\"dimension\" [\"template-tag\" \"checkin_date\"]] + :value \"2015-01-01~2016-09-01\"}}}" + (:require [clojure.string :as str] + [metabase.models.field :refer [Field]] + [metabase.query-processor.middleware.parameters.native.interface :as i] + [metabase.util + [i18n :as ui18n :refer [tru]] + [schema :as su]] + [schema.core :as s] + [toucan.db :as db]) + (:import java.text.NumberFormat + java.util.UUID + [metabase.query_processor.middleware.parameters.native.interface CommaSeparatedNumbers FieldFilter + MultipleValues])) + +(def ^:private ParamType + (s/enum :number + :dimension ; Field Filter + :text + :date)) + +;; various schemas are used to check that various functions return things in expected formats + +;; TAGS in this case are simple params like {{x}} that get replaced with a single value ("ABC" or 1) as opposed to a +;; "FieldFilter" clause like FieldFilters +;; +;; Since 'FieldFilter' are considered their own `:type` (confusingly enough, called `:dimension`), to *actually* store +;; the type of a FieldFilter look at the key `:widget-type`. This applies to things like the default value for a +;; FieldFilter as well. +(def ^:private TagParam + "Schema for a tag parameter declaration, passed in as part of the `:template-tags` list." + {(s/optional-key :id) su/NonBlankString ; this is used internally by the frontend + :name su/NonBlankString + :display-name su/NonBlankString + :type ParamType + (s/optional-key :dimension) [s/Any] + (s/optional-key :widget-type) s/Keyword ; type of the [default] value if `:type` itself is `dimension` + (s/optional-key :required) s/Bool + (s/optional-key :default) s/Any}) + +(def ^:private ParsedParamValue + "Schema for valid param value(s). Params can have one or more values." + (s/named (s/maybe (s/cond-pre i/SingleValue MultipleValues)) + "Valid param value(s)")) + +(s/defn ^:private param-with-target + "Return the param in `params` with a matching `target`. `target` is something like: + + [:dimension [:template-tag <param-name>]] ; for FieldFilters (Field Filters) + [:variable [:template-tag <param-name>]] ; for other types of params" + [params :- (s/maybe [i/ParamValue]), target] + (when-let [matching-params (seq (for [param params + :when (= (:target param) target)] + param))] + ;; if there's only one matching param no need to nest it inside a vector. Otherwise return vector of params + ((if (= (count matching-params) 1) + first + vec) matching-params))) + + +;;; FieldFilter Params (Field Filters) (e.g. WHERE {{x}}) + +(s/defn ^:private default-value-for-field-filter + "Return the default value for a FieldFilter (`:type` = `:dimension`) param defined by the map `tag`, if one is set." + [tag :- TagParam] + (when (and (:required tag) (not (:default tag))) + (throw (Exception. (tru "''{0}'' is a required param." (:display-name tag))))) + (when-let [default (:default tag)] + {:type (:widget-type tag :dimension) ; widget-type is the actual type of the default value if set + :target [:dimension [:template-tag (:name tag)]] + :value default})) + +(s/defn ^:private field-filter->field-id :- su/IntGreaterThanZero + [field-filter] + (second field-filter)) + +(s/defn ^:private field-filter-value-for-tag :- (s/maybe (s/cond-pre FieldFilter (s/eq i/no-value))) + "Return the `FieldFilter` value of a param, if applicable. Field filters are referred to internally as `:dimension`s + for historic reasons." + [tag :- TagParam, params :- (s/maybe [i/ParamValue])] + (when-let [field-filter (:dimension tag)] + (i/map->FieldFilter + ;; TODO - shouldn't this use the QP Store? + {:field (or (db/select-one [Field :name :parent_id :table_id :base_type] + :id (field-filter->field-id field-filter)) + (throw (Exception. (tru "Can't find field with ID: {0}" + (field-filter->field-id field-filter))))) + :value (if-let [value-info-or-infos (or + ;; look in the sequence of params we were passed to see if there's anything + ;; that matches + (param-with-target params [:dimension [:template-tag (:name tag)]]) + ;; if not, check and see if we have a default param + (default-value-for-field-filter tag))] + ;; `value-info` will look something like after we remove `:target` which is not needed after this point + ;; + ;; {:type :date/single + ;; :value #inst "2019-09-20T19:52:00.000-07:00"} + ;; + ;; (or it will be a vector of these maps for multiple values) + (cond + (map? value-info-or-infos) (dissoc value-info-or-infos :target) + (sequential? value-info-or-infos) (mapv #(dissoc % :target) value-info-or-infos)) + i/no-value)}))) + + +;;; Non-FieldFilter Params (e.g. WHERE x = {{x}}) + +(s/defn ^:private param-value-for-tag [tag :- TagParam, params :- (s/maybe [i/ParamValue])] + (when (not= (:type tag) :dimension) + (:value (param-with-target params [:variable [:template-tag (:name tag)]])))) + +(s/defn ^:private default-value-for-tag + "Return the `:default` value for a param if no explicit values were passsed. This only applies to non-FieldFilter + params. Default values for FieldFilter (Field Filter) params are handled above in `default-value-for-field-filter`." + [{:keys [default display-name required]} :- TagParam] + (or default + (when required + (throw (Exception. (tru "''{0}'' is a required param." display-name)))))) + + +;;; Parsing Values + +(s/defn ^:private parse-number :- s/Num + "Parse a string like `1` or `2.0` into a valid number. Done mostly to keep people from passing in + things that aren't numbers, like SQL identifiers." + [s :- s/Str] + (.parse (NumberFormat/getInstance) ^String s)) + +(s/defn ^:private value->number :- (s/cond-pre s/Num CommaSeparatedNumbers) + "Parse a 'numeric' param value. Normally this returns an integer or floating-point number, but as a somewhat + undocumented feature it also accepts comma-separated lists of numbers. This was a side-effect of the old parameter + code that unquestioningly substituted any parameter passed in as a number directly into the SQL. This has long been + changed for security purposes (avoiding SQL injection), but since users have come to expect comma-separated numeric + values to work we'll allow that (with validation) and return an instance of `CommaSeperatedNumbers`. (That is + converted to SQL as a simple comma-separated list.)" + [value] + (cond + ;; if not a string it's already been parsed + (number? value) value + ;; same goes for an instance of CommaSeperated values + (instance? CommaSeparatedNumbers value) value + ;; if the value is a string, then split it by commas in the string. Usually there should be none. + ;; Parse each part as a number. + (string? value) + (let [parts (for [part (str/split value #",")] + (parse-number part))] + (if (> (count parts) 1) + ;; If there's more than one number return an instance of `CommaSeparatedNumbers` + (i/map->CommaSeparatedNumbers {:numbers parts}) + ;; otherwise just return the single number + (first parts))))) + +(s/defn ^:private parse-value-for-field-base-type :- s/Any + "Do special parsing for value for a (presumably textual) FieldFilter (`:type` = `:dimension`) param (i.e., attempt + to parse it as appropriate based on the base-type of the Field associated with it). These are special cases for + handling types that do not have an associated parameter type (such as `date` or `number`), such as UUID fields." + [base-type :- su/FieldType, value] + (cond + (isa? base-type :type/UUID) (UUID/fromString value) + (isa? base-type :type/Number) (value->number value) + :else value)) + +(s/defn ^:private parse-value-for-type :- ParsedParamValue + "Parse a `value` based on the type chosen for the param, such as `text` or `number`. (Depending on the type of param + created, `value` here might be a raw value or a map including information about the Field it references as well as a + value.) For numbers, dates, and the like, this will parse the string appropriately; for `text` parameters, this will + additionally attempt handle special cases based on the base type of the Field, for example, parsing params for UUID + base type Fields as UUIDs." + [param-type :- ParamType, value] + (cond + (= value i/no-value) + value + + (= param-type :number) + (value->number value) + + (= param-type :date) + (i/map->Date {:s value}) + + ;; Field Filters + (and (= param-type :dimension) + (= (get-in value [:value :type]) :number)) + (update-in value [:value :value] value->number) + + (sequential? value) + (i/map->MultipleValues {:values (for [v value] + (parse-value-for-type param-type v))}) + + (and (= param-type :dimension) + (get-in value [:field :base_type]) + (string? (get-in value [:value :value]))) + (update-in value [:value :value] (partial parse-value-for-field-base-type (get-in value [:field :base_type]))) + + :else + value)) + +(s/defn ^:private value-for-tag :- ParsedParamValue + "Given a map `tag` (a value in the `:template-tags` dictionary) return the corresponding value from the `params` + sequence. The `value` is something that can be compiled to SQL via `->replacement-snippet-info`." + [tag :- TagParam, params :- (s/maybe [i/ParamValue])] + (parse-value-for-type (:type tag) (or (param-value-for-tag tag params) + (field-filter-value-for-tag tag params) + (default-value-for-tag tag) + i/no-value))) + +(s/defn query->params-map :- {su/NonBlankString ParsedParamValue} + "Extract parameters info from `query`. Return a map of parameter name -> value. + + (query->params-map some-query) + -> + {:checkin_date #inst \"2019-09-19T23:30:42.233-07:00\"}" + [{tags :template-tags, params :parameters}] + (into {} (for [[k tag] tags + :let [v (value-for-tag tag params)] + :when v] + ;; TODO - if V is `nil` *on purpose* this still won't give us a query like `WHERE field = NULL`. That + ;; kind of query shouldn't be possible from the frontend anyway + {k v}))) diff --git a/src/metabase/query_processor/middleware/parameters/sql.clj b/src/metabase/query_processor/middleware/parameters/sql.clj deleted file mode 100644 index 9241ec8da4f43393aa7872d367205a7329013949..0000000000000000000000000000000000000000 --- a/src/metabase/query_processor/middleware/parameters/sql.clj +++ /dev/null @@ -1,593 +0,0 @@ -(ns metabase.query-processor.middleware.parameters.sql - "Param substitution for *SQL* queries. - This is a new implementation, fondly referred to as 'SQL parameters 2.0', written for v0.23.0. - The new implementation uses prepared statement args instead of substituting them directly into the query, - and is much better-organized and better-documented." - (:require [clojure.string :as str] - [honeysql.core :as hsql] - [medley.core :as m] - [metabase.driver :as driver] - [metabase.driver.sql :as sql] - [metabase.driver.sql.query-processor :as sql.qp] - [metabase.models.field :as field :refer [Field]] - [metabase.query-processor.middleware.parameters.dates :as date-params] - [metabase.util - [date :as du] - [i18n :as ui18n :refer [tru]] - [schema :as su]] - [schema.core :as s] - [toucan.db :as db]) - (:import clojure.lang.Keyword - honeysql.types.SqlCall - java.text.NumberFormat - java.util.regex.Pattern - java.util.UUID - metabase.models.field.FieldInstance)) - -;; The Basics: -;; -;; * Things like `{{x}}` (required params) get subsituted with the value of `:x`, which can be a literal used in a -;; clause (e.g. in a clause like `value = {{x}}`) or a "field filter" that handles adding the clause itself -;; (e.g. `{{timestamp}}` might become `timestamp BETWEEN ? AND ?`). -;; -;; * Things like `[[AND {{x}]]` are optional param. If the param (`:x`) isn't specified, the *entire* clause inside -;; `[[...]]` is replaced with an empty string; If it is specified, the value inside the curly brackets `{{x}}` is -;; replaced as usual and the rest of the clause (`AND ...`) is included in the query as-is -;; -;; See the various parts of this namespace below to see how it's done. - - -;;; +----------------------------------------------------------------------------------------------------------------+ -;;; | ETC | -;;; +----------------------------------------------------------------------------------------------------------------+ - -;; Various record types below are used as a convenience for differentiating the different param types. - -;; "Dimension" here means a "FIELD FILTER", e.g. something that expands to a clause like "some_field BETWEEN 1 AND 10" -(s/defrecord ^:private Dimension [field :- FieldInstance, param]) ; param is either single param or a vector of params - -;; as in a literal date, defined by date-string S -(s/defrecord ^:private Date [s :- s/Str]) - -(defrecord ^:private DateRange [start end]) - -;; List of numbers to faciliate things like using params in a SQL `IN` clause. See the discussion in `value->number` -;; for more details. -(s/defrecord ^:private CommaSeparatedNumbers [numbers :- [s/Num]]) - -;; convenience for representing an *optional* parameter present in a query but whose value is unspecified in the param -;; values. -(defrecord ^:private NoValue []) - -(defn- no-value? [x] - (instance? NoValue x)) - -(def ^:private ParamType - (s/enum :number :dimension :text :date)) - -;; various schemas are used to check that various functions return things in expected formats - -;; TAGS in this case are simple params like {{x}} that get replaced with a single value ("ABC" or 1) as opposed to a -;; "FieldFilter" clause like Dimensions -;; -;; Since 'Dimension' (Field Filters) are considered their own `:type`, to *actually* store the type of a Dimension -;; look at the key `:widget-type`. This applies to things like the default value for a Dimension as well. -(def ^:private TagParam - "Schema for values passed in as part of the `:template-tags` list." - {(s/optional-key :id) su/NonBlankString ; this is used internally by the frontend - :name su/NonBlankString - :display-name su/NonBlankString - :type ParamType - (s/optional-key :dimension) [s/Any] - (s/optional-key :widget-type) s/Keyword ; type of the [default] value if `:type` itself is `dimension` - (s/optional-key :required) s/Bool - (s/optional-key :default) s/Any}) - -(def ^:private DimensionValue - {:type s/Keyword ; TODO - what types are allowed? :text, ...? - :target s/Any - ;; not specified if the param has no value. TODO - make this stricter - (s/optional-key :value) s/Any - ;; The following are not used by the code in this namespace but may or may not be specified depending on what the - ;; code that constructs the query params is doing. We can go ahead and ignore these when present. - (s/optional-key :slug) su/NonBlankString - (s/optional-key :name) su/NonBlankString - (s/optional-key :default) s/Any - (s/optional-key :id) s/Any}) ; used internally by the frontend - -(def ^:private SingleValue - "Schema for a valid *single* value for a param. As of 0.28.0 params can either be single-value or multiple value." - (s/cond-pre NoValue - CommaSeparatedNumbers - Dimension - Date - s/Num - s/Str - s/Bool)) - -;; Sequence of multiple values for generating a SQL IN() clause. vales -(s/defrecord ^:private MultipleValues [values :- [SingleValue]]) - -(def ^:private ParamValue - "Schema for valid param value(s). Params can have one or more values." - (s/named (s/maybe (s/cond-pre SingleValue MultipleValues)) - "Valid param value(s)")) - -(def ^:private ParamValues - {su/NonBlankString ParamValue}) - -(def ^:private ParamSnippetInfo - {(s/optional-key :replacement-snippet) s/Str ; allowed to be blank if this is an optional param - (s/optional-key :prepared-statement-args) [s/Any]}) - -;;; +----------------------------------------------------------------------------------------------------------------+ -;;; | PARAM INFO RESOULTION | -;;; +----------------------------------------------------------------------------------------------------------------+ - -;; These functions build a map of information about the types and values of the params used in a query. -;; (These functions don't parse the query itself, but instead look at the values of `:template-tags` and `:parameters` -;; passed along with the query.) -;; -;; (query->params-map some-query) -;; ;; -> {"checkin_date" {:field {:name "\date"\, :parent_id nil, :table_id 1375} -;; :param {:type "\date/range"\ -;; :target ["\dimension"\ ["\template-tag"\ "\checkin_date"\]] -;; :value "\2015-01-01~2016-09-01"\}}} - -(s/defn ^:private param-with-target - "Return the param in `params` with a matching `target`. `target` is something like: - - [:dimension [:template-tag <param-name>]] ; for Dimensions (Field Filters) - [:variable [:template-tag <param-name>]] ; for other types of params" - [params :- (s/maybe [DimensionValue]), target] - (when-let [matching-params (seq (for [param params - :when (= (:target param) target)] - param))] - ;; if there's only one matching param no need to nest it inside a vector. Otherwise return vector of params - ((if (= (count matching-params) 1) - first - vec) matching-params))) - - -;;; Dimension Params (Field Filters) (e.g. WHERE {{x}}) - -(s/defn ^:private default-value-for-dimension :- (s/maybe DimensionValue) - "Return the default value for a Dimension (Field Filter) param defined by the map TAG, if one is set." - [tag :- TagParam] - (when (and (:required tag) (not (:default tag))) - (throw (Exception. (tru "''{0}'' is a required param." (:display-name tag))))) - (when-let [default (:default tag)] - {:type (:widget-type tag :dimension) ; widget-type is the actual type of the default value if set - :target [:dimension [:template-tag (:name tag)]] - :value default})) - -(s/defn ^:private dimension->field-id :- su/IntGreaterThanZero - [dimension] - (second dimension)) - -(s/defn ^:private dimension-value-for-tag :- (s/maybe Dimension) - "Return the \"Dimension\" value of a param, if applicable. \"Dimension\" here means what is called a \"Field - Filter\" in the Native Query Editor." - [tag :- TagParam, params :- (s/maybe [DimensionValue])] - (when-let [dimension (:dimension tag)] - (map->Dimension {:field (or (db/select-one [Field :name :parent_id :table_id :base_type], - :id (dimension->field-id dimension)) - (throw (Exception. (tru "Can't find field with ID: {0}" - (dimension->field-id dimension))))) - :param (or - ;; look in the sequence of params we were passed to see if there's anything that matches - (param-with-target params [:dimension [:template-tag (:name tag)]]) - ;; if not, check and see if we have a default param - (default-value-for-dimension tag))}))) - - -;;; Non-Dimension Params (e.g. WHERE x = {{x}}) - -(s/defn ^:private param-value-for-tag [tag :- TagParam, params :- (s/maybe [DimensionValue])] - (when (not= (:type tag) :dimension) - (:value (param-with-target params [:variable [:template-tag (:name tag)]])))) - -(s/defn ^:private default-value-for-tag - "Return the `:default` value for a param if no explicit values were passsed. This only applies to non-Dimension - (non-Field Filter) params. Default values for Dimension (Field Filter) params are handled above in - `default-value-for-dimension`." - [{:keys [default display-name required]} :- TagParam] - (or default - (when required - (throw (Exception. (tru "''{0}'' is a required param." display-name)))))) - - -;;; Parsing Values - -(s/defn ^:private parse-number :- s/Num - "Parse a string like `1` or `2.0` into a valid number. Done mostly to keep people from passing in - things that aren't numbers, like SQL identifiers." - [s :- s/Str] - (.parse (NumberFormat/getInstance) ^String s)) - -(s/defn ^:private value->number :- (s/cond-pre s/Num CommaSeparatedNumbers) - "Parse a 'numeric' param value. Normally this returns an integer or floating-point number, but as a somewhat - undocumented feature it also accepts comma-separated lists of numbers. This was a side-effect of the old parameter - code that unquestioningly substituted any parameter passed in as a number directly into the SQL. This has long been - changed for security purposes (avoiding SQL injection), but since users have come to expect comma-separated numeric - values to work we'll allow that (with validation) and return an instance of `CommaSeperatedNumbers`. (That is - converted to SQL as a simple comma-separated list.)" - [value] - (cond - ;; if not a string it's already been parsed - (number? value) value - ;; same goes for an instance of CommaSeperated values - (instance? CommaSeparatedNumbers value) value - ;; if the value is a string, then split it by commas in the string. Usually there should be none. - ;; Parse each part as a number. - (string? value) - (let [parts (for [part (str/split value #",")] - (parse-number part))] - (if (> (count parts) 1) - ;; If there's more than one number return an instance of `CommaSeparatedNumbers` - (strict-map->CommaSeparatedNumbers {:numbers parts}) - ;; otherwise just return the single number - (first parts))))) - -(s/defn ^:private parse-value-for-field-base-type :- s/Any - "Do special parsing for value for a (presumably textual) FieldFilter 'dimension' param (i.e., attempt to parse it as - appropriate based on the base-type of the Field associated with it). These are special cases for handling types that - do not have an associated parameter type (such as `date` or `number`), such as UUID fields." - [base-type :- su/FieldType, value] - (cond - (isa? base-type :type/UUID) (UUID/fromString value) - (isa? base-type :type/Number) (value->number value) - :else value)) - -(s/defn ^:private parse-value-for-type :- ParamValue - "Parse a `value` based on the type chosen for the param, such as `text` or `number`. (Depending on the type of param - created, `value` here might be a raw value or a map including information about the Field it references as well as a - value.) For numbers, dates, and the like, this will parse the string appropriately; for `text` parameters, this will - additionally attempt handle special cases based on the base type of the Field, for example, parsing params for UUID - base type Fields as UUIDs." - [param-type :- ParamType, value] - (cond - (no-value? value) - value - - (= param-type :number) - (value->number value) - - (= param-type :date) - (map->Date {:s value}) - - (and (= param-type :dimension) - (= (get-in value [:param :type]) :number)) - (update-in value [:param :value] value->number) - - (sequential? value) - (map->MultipleValues {:values (for [v value] - (parse-value-for-type param-type v))}) - - (and (= param-type :dimension) - (get-in value [:field :base_type]) - (string? (get-in value [:param :value]))) - (update-in value [:param :value] (partial parse-value-for-field-base-type (get-in value [:field :base_type]))) - - :else - value)) - -(s/defn ^:private value-for-tag :- ParamValue - "Given a map TAG (a value in the `:template-tags` dictionary) return the corresponding value from the `params` - sequence. The VALUE is something that can be compiled to SQL via `->replacement-snippet-info`." - [tag :- TagParam, params :- (s/maybe [DimensionValue])] - (parse-value-for-type (:type tag) (or (param-value-for-tag tag params) - (dimension-value-for-tag tag params) - (default-value-for-tag tag) - ;; TODO - what if value is specified but is `nil`? - (NoValue.)))) - -(s/defn ^:private query->params-map :- ParamValues - "Extract parameters info from `query`. Return a map of parameter name -> value. - - (query->params-map some-query) - -> - {:checkin_date {:field {:name \"date\", :parent_id nil, :table_id 1375} - :param {:type :date/range - :target [:dimension [:template-tag \"checkin_date\"]] - :value \"2015-01-01~2016-09-01\"}}}" - [{{tags :template-tags} :native, params :parameters}] - (into {} (for [[k tag] tags - :let [v (value-for-tag tag params)] - :when v] - ;; TODO - if V is `nil` *on purpose* this still won't give us a query like `WHERE field = NULL`. That - ;; kind of query shouldn't be possible from the frontend anyway - {k v}))) - - -;;; +----------------------------------------------------------------------------------------------------------------+ -;;; | PARAM->SQL SUBSTITUTION | -;;; +----------------------------------------------------------------------------------------------------------------+ - -;; These functions take the info for a param fetched by the functions above and add additional info about how that -;; param should be represented as SQL. (Specifically, they return information in this format: -;; -;; {:replacement-snippet "= ?" ; appropriate SQL that should be used to replace the param snippet, e.g. {{x}} -;; :prepared-statement-args [#inst "2017-01-01"]} ; any prepared statement args (values for `?` placeholders) needed for the replacement snippet - -(defprotocol ^:private ISQLParamSubstituion - "Protocol for specifying what SQL should be generated for parameters of various types." - (^:private ->replacement-snippet-info [this] - "Return information about how THIS should be converted to SQL, as a map with keys `:replacement-snippet` and - `:prepared-statement-args`. - - (->replacement-snippet-info \"ABC\") -> {:replacement-snippet \"?\", :prepared-statement-args \"ABC\"}")) - - -(defn- relative-date-param-type? [param-type] - (contains? #{:date/range :date/month-year :date/quarter-year :date/relative :date/all-options} param-type)) - -;; for relative dates convert the param to a `DateRange` record type and call `->replacement-snippet-info` on it -(s/defn ^:private relative-date-dimension-value->replacement-snippet-info :- ParamSnippetInfo - [value] - ;; TODO - get timezone from query dict - (-> (date-params/date-string->range value (.getID du/*report-timezone*)) - map->DateRange - ->replacement-snippet-info)) - -(s/defn ^:private dimension-value->equals-clause-sql :- ParamSnippetInfo - [value] - (-> (->replacement-snippet-info value) - (update :replacement-snippet (partial str "= ")))) - -(s/defn ^:private dimension-multiple-values->in-clause-sql :- ParamSnippetInfo - [values] - (-> (map->MultipleValues {:values values}) - ->replacement-snippet-info - (update :replacement-snippet (partial format "IN (%s)")))) - -(s/defn ^:private dimension->replacement-snippet-info :- ParamSnippetInfo - "Return `[replacement-snippet & prepared-statement-args]` appropriate for a `dimension` parameter." - [{param-type :type, value :value} :- DimensionValue] - (cond - ;; convert relative dates to approprate date range representations - (relative-date-param-type? param-type) (relative-date-dimension-value->replacement-snippet-info value) - ;; convert all other dates to `= <date>` - (date-params/date-type? param-type) (dimension-value->equals-clause-sql (map->Date {:s value})) - ;; for sequences of multiple values we want to generate an `IN (...)` clause - (sequential? value) (dimension-multiple-values->in-clause-sql value) - ;; convert everything else to `= <value>` - :else (dimension-value->equals-clause-sql value))) - -(s/defn ^:private honeysql->replacement-snippet-info :- ParamSnippetInfo - "Convert `x` to a replacement snippet info map by passing it to HoneySQL's `format` function." - [x] - (let [[snippet & args] (hsql/format x, :quoting (sql.qp/quote-style driver/*driver*), :allow-dashed-names? true)] - {:replacement-snippet snippet - :prepared-statement-args args})) - -(s/defn ^:private field->identifier :- su/NonBlankString - "Return an approprate snippet to represent this `field` in SQL given its param type. - For non-date Fields, this is just a quoted identifier; for dates, the SQL includes appropriately bucketing based on - the `param-type`." - [field param-type] - (:replacement-snippet - (honeysql->replacement-snippet-info - (let [identifier (sql.qp/->honeysql driver/*driver* (sql.qp/field->identifier driver/*driver* field))] - (if (date-params/date-type? param-type) - (sql.qp/date driver/*driver* :day identifier) - identifier))))) - -(s/defn ^:private combine-replacement-snippet-maps :- ParamSnippetInfo - "Combine multiple `replacement-snippet-maps` into a single map using a SQL `AND` clause." - [replacement-snippet-maps :- [ParamSnippetInfo]] - {:replacement-snippet (str \( (str/join " AND " (map :replacement-snippet replacement-snippet-maps)) \)) - :prepared-statement-args (reduce concat (map :prepared-statement-args replacement-snippet-maps))}) - -(defn- create-replacement-snippet [nil-or-obj] - (let [{:keys [sql-string param-values]} (sql/->prepared-substitution driver/*driver* nil-or-obj)] - {:replacement-snippet sql-string - :prepared-statement-args param-values})) - -(defn- prepared-ts-subs [operator date-str] - (let [{:keys [sql-string param-values]} (sql/->prepared-substitution driver/*driver* (du/->Timestamp date-str))] - {:replacement-snippet (str operator " " sql-string) - :prepared-statement-args param-values})) - -(extend-protocol ISQLParamSubstituion - nil (->replacement-snippet-info [this] (create-replacement-snippet this)) - Object (->replacement-snippet-info [this] (create-replacement-snippet (str this))) - Number (->replacement-snippet-info [this] (create-replacement-snippet this)) - Boolean (->replacement-snippet-info [this] (create-replacement-snippet this)) - Keyword (->replacement-snippet-info [this] (create-replacement-snippet this)) - SqlCall (->replacement-snippet-info [this] (create-replacement-snippet this)) - UUID (->replacement-snippet-info [this] {:replacement-snippet (format "CAST('%s' AS uuid)" (str this))}) - NoValue (->replacement-snippet-info [_] {:replacement-snippet ""}) - - CommaSeparatedNumbers - (->replacement-snippet-info [{:keys [numbers]}] - {:replacement-snippet (str/join ", " numbers)}) - - MultipleValues - (->replacement-snippet-info [{:keys [values]}] - (let [values (map ->replacement-snippet-info values)] - {:replacement-snippet (str/join ", " (map :replacement-snippet values)) - :prepared-statement-args (apply concat (map :prepared-statement-args values))})) - - Date - (->replacement-snippet-info [{:keys [s]}] - (create-replacement-snippet (du/->Timestamp s))) - - DateRange - (->replacement-snippet-info [{:keys [start end]}] - (cond - (= start end) - (prepared-ts-subs \= start) - - (nil? start) - (prepared-ts-subs \< end) - - (nil? end) - (prepared-ts-subs \> start) - - :else - (let [params (map (comp #(sql/->prepared-substitution driver/*driver* %) du/->Timestamp) [start end])] - {:replacement-snippet (apply format "BETWEEN %s AND %s" (map :sql-string params)), - :prepared-statement-args (vec (mapcat :param-values params))}))) - - ;; TODO - clean this up if possible! - Dimension - (->replacement-snippet-info [{:keys [field param], :as dimension}] - (cond - ;; if the param is `nil` just put in something that will always be true, such as `1` (e.g. `WHERE 1 = 1`) - (nil? param) {:replacement-snippet "1 = 1"} - ;; if we have a vector of multiple params recursively convert them to SQL and combine into an `AND` clause - ;; (This is multiple params in the sense that the frontend provided multiple maps with param values for the same - ;; Dimension, not in the sense that we have a single map with multiple values for `:value`.) - (vector? param) - (combine-replacement-snippet-maps (for [p param] - (->replacement-snippet-info (assoc dimension :param p)))) - ;; otherwise convert single param to SQL. - ;; Convert the value to a replacement snippet info map and then tack on the field identifier to the front - :else - (update (dimension->replacement-snippet-info param) - :replacement-snippet (partial str (field->identifier field (:type param)) " "))))) - - -;;; +----------------------------------------------------------------------------------------------------------------+ -;;; | PARSING THE SQL TEMPLATE | -;;; +----------------------------------------------------------------------------------------------------------------+ - -(defrecord ^:private Param [param-key sql-value prepared-statement-args]) - -(defn- param? [maybe-param] - (instance? Param maybe-param)) - -(defn- merge-query-map [query-map node] - (cond - (string? node) - (update query-map :query str node) - - (param? node) - (-> query-map - (update :query str (:sql-value node)) - (update :params concat (:prepared-statement-args node))) - - :else - (-> query-map - (update :query str (:query node)) - (update :params concat (:params node))))) - -(def ^:private empty-query-map {:query "" :params []}) - -(defn- no-value-param? [maybe-param] - (and (param? maybe-param) - (no-value? (:sql-value maybe-param)))) - -(defn- quoted-re-pattern [s] - (-> s Pattern/quote re-pattern)) - -(defn- split-delimited-string - "Interesting parts of the SQL string (vs. parts that are just passed through) are delimited, i.e. {{something}}. This - function takes a `delimited-begin` and `delimited-end` regex and uses that to separate the string. Returns a map - with the prefix (the string leading up to the first `delimited-begin`) and `:delimited-strings` as a seq of maps - where `:delimited-body` is what's in-between the delimited marks (i.e. foo in {{foo}} and then a suffix, which is - the characters after the trailing delimiter but before the next occurrence of the `delimited-end`." - [delimited-begin delimited-end s] - (let [begin-pattern (quoted-re-pattern delimited-begin) - end-pattern (quoted-re-pattern delimited-end) - [prefix & segmented-strings] (str/split s begin-pattern)] - (when-let [^String msg (and (seq segmented-strings) - (not-every? #(str/index-of % delimited-end) segmented-strings) - (tru "Found ''{0}'' with no terminating ''{1}'' in query ''{2}''" - delimited-begin delimited-end s))] - (throw (IllegalArgumentException. msg))) - {:prefix prefix - :delimited-strings (for [segmented-string segmented-strings - :let [[token-str & rest-of-segment] (str/split segmented-string end-pattern)]] - {:delimited-body token-str - :suffix (apply str rest-of-segment)})})) - -(s/defn ^:private token->param :- Param - "Given a `token` and `param-key->value` return a `Param`. If no parameter value is found, return a `NoValue` param" - [token :- su/NonBlankString, param-key->value :- ParamValues] - (let [val (get param-key->value token (NoValue.)) - {:keys [replacement-snippet - prepared-statement-args]} (->replacement-snippet-info val)] - (map->Param (merge {:param-key token} - (if (no-value? val) - {:sql-value val, :prepared-statement-args []} - {:sql-value replacement-snippet - :prepared-statement-args prepared-statement-args}))))) - -(s/defn ^:private parse-params - "Parse `s` for any parameters. Returns a seq of strings and `Param` instances" - [s :- s/Str, param-key->value :- ParamValues] - (let [{:keys [prefix delimited-strings]} (split-delimited-string "{{" "}}" s)] - (cons prefix - (mapcat (fn [{:keys [delimited-body suffix]}] - [(-> delimited-body - str/trim - (token->param param-key->value)) - suffix]) - delimited-strings)))) - -(s/defn ^:private parse-params-or-throw - "Same as `parse-params` but will throw an exception if there are any `NoValue` parameters" - [s :- s/Str, param-key->value :- ParamValues] - (let [results (parse-params s param-key->value)] - (if-let [{:keys [param-key]} (m/find-first no-value-param? results)] - (throw (ui18n/ex-info (tru "Unable to substitute ''{0}'': param not specified.\nFound: {1}" - (name param-key) (pr-str (map name (keys param-key->value)))) - {:status-code 400})) - results))) - -(def ^:private ParseTemplateResponse - {:query s/Str - :params [s/Any]}) - -(defn- parse-one-optional-param - "Parse a single optional param." - [param-key->value {:keys [delimited-body suffix]}] - (let [optional-clause (parse-params delimited-body param-key->value)] - (if (some no-value-param? optional-clause) - (parse-params-or-throw suffix param-key->value) - (concat optional-clause (parse-params-or-throw suffix param-key->value))))) - -(s/defn ^:private parse-optional-params :- ParseTemplateResponse - "Attempts to parse SQL parameter string `s`. Parses any optional clauses or parameters found, returns a query map." - [s :- s/Str, param-key->value :- ParamValues] - (let [{:keys [prefix delimited-strings]} (split-delimited-string "[[" "]]" s) - parsed (apply - concat - (parse-params-or-throw prefix param-key->value) - (for [parsed-delimited-string delimited-strings] - (parse-one-optional-param param-key->value parsed-delimited-string)))] - (reduce merge-query-map empty-query-map parsed))) - -(s/defn ^:private parse-template :- ParseTemplateResponse - [sql :- s/Str, param-key->value :- ParamValues] - (-> sql - (parse-optional-params param-key->value) - (update :query str/trim))) - - -;;; +----------------------------------------------------------------------------------------------------------------+ -;;; | PUTTING IT ALL TOGETHER | -;;; +----------------------------------------------------------------------------------------------------------------+ - -(s/defn ^:private expand-query-params - [{sql :query, :as native} :- {:query s/Str, s/Keyword s/Any}, param-key->value :- ParamValues] - (-> native - (merge (parse-template sql param-key->value)) - (dissoc :template-tags))) - -(s/defn expand - "Expand parameters inside a *SQL* `query`." - ([query :- {:native su/Map, s/Keyword s/Any}] - (if (driver/supports? driver/*driver* :native-parameters) - (update query :native expand-query-params (query->params-map query)) - query)) - - ;; HACK - all this code is written expecting `:parameters` to be a top-level key; to support parameters in source - ;; queries (especially for GTAPs) we need `:parameters` to be in the same level as the query they affect; so move - ;; passed parameters to the top-level until we get a chance to fix this. - ([query parameters] - (-> (assoc query :parameters parameters) - expand - (dissoc query :parameters)))) diff --git a/test/metabase/query_processor/middleware/parameters/native/parse_test.clj b/test/metabase/query_processor/middleware/parameters/native/parse_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..1441331f97c35af8e93a05e41b5a325ea9c8b7ba --- /dev/null +++ b/test/metabase/query_processor/middleware/parameters/native/parse_test.clj @@ -0,0 +1,100 @@ +(ns metabase.query-processor.middleware.parameters.native.parse-test + (:require [expectations :refer [expect]] + [metabase.query-processor.middleware.parameters.native + [interface :as i] + [parse :as parse]])) + +(def ^:private param (var-get #'parse/param)) +(def ^:private optional (var-get #'parse/optional)) + +;; tests for tokenization +(expect + [:param-begin "num_toucans" :end] + (#'parse/tokenize "{{num_toucans}}")) + +(expect + [:optional-begin "AND num_toucans > " :param-begin "num_toucans" :end :end] + (#'parse/tokenize "[[AND num_toucans > {{num_toucans}}]]")) + +(expect + [:end :param-begin :end :optional-begin] + (#'parse/tokenize "}}{{]][[")) + +(expect + ["SELECT * FROM toucanneries WHERE TRUE " :optional-begin "AND num_toucans > " :param-begin "num_toucans" :end :end] + (#'parse/tokenize "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]")) + +(expect + ["SELECT * FROM toucanneries WHERE TRUE " :optional-begin "AND num_toucans > " :param-begin "num_toucans" :end :end + " " :optional-begin "AND total_birds > " :param-begin "total_birds" :end :end] + (#'parse/tokenize + "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]")) + +(expect + ["select * from foo where bar=1"] + (parse/parse "select * from foo where bar=1")) + +(expect + ["select * from foo where bar=" (i/->Param "baz")] + (parse/parse "select * from foo where bar={{baz}}")) + +(expect + ["select * from foo " (i/->Optional ["where bar = " (i/->Param "baz") " "])] + (parse/parse "select * from foo [[where bar = {{baz}} ]]")) + +;; Multiple optional clauses +(expect + ["select * from foo where bar1 = " (param "baz") " " + (optional "and bar2 = " (param "baz")) " " + (optional "and bar3 = " (param "baz")) " " + (optional "and bar4 = " (param "baz"))] + (parse/parse (str "select * from foo where bar1 = {{baz}} " + "[[and bar2 = {{baz}}]] " + "[[and bar3 = {{baz}}]] " + "[[and bar4 = {{baz}}]]"))) + +(expect + ["select * from foobars " + (optional " where foobars.id in (string_to_array(" (param "foobar_id") ", ',')::integer[]) ")] + (parse/parse "select * from foobars [[ where foobars.id in (string_to_array({{foobar_id}}, ',')::integer[]) ]]")) + +;; single square brackets shouldn't get parsed +(let [query (str "SELECT [test_data.checkins.venue_id] AS [venue_id], " + " [test_data.checkins.user_id] AS [user_id], " + " [test_data.checkins.id] AS [checkins_id] " + "FROM [test_data.checkins] " + "LIMIT 2")] + (expect + [query] + (parse/parse query))) + +(expect + ["SELECT * FROM toucanneries WHERE TRUE " + (optional "AND num_toucans > " (param "num_toucans")) + " " + (optional "AND total_birds > " (param "total_birds"))] + (parse/parse + "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]")) + +;; Valid syntax in PG -- shouldn't get parsed +(let [query "SELECT array_dims(1 || '[0:1]={2,3}'::int[])"] + (expect + [query] + (parse/parse query))) + +;; Testing that invalid/unterminated template params/clauses throw an exception +(expect + IllegalArgumentException + (parse/parse "select * from foo [[where bar = {{baz}} ")) + +(expect + IllegalArgumentException + (parse/parse "select * from foo [[where bar = {{baz]]")) + +(expect + IllegalArgumentException + (parse/parse "select * from foo {{bar}} {{baz")) + +(expect + IllegalArgumentException + (parse/parse "select * from foo [[clause 1 {{bar}}]] [[clause 2")) diff --git a/test/metabase/query_processor/middleware/parameters/native/substitute_test.clj b/test/metabase/query_processor/middleware/parameters/native/substitute_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..d0f3bc381a557ccbb25bfeae4df65a79c872276a --- /dev/null +++ b/test/metabase/query_processor/middleware/parameters/native/substitute_test.clj @@ -0,0 +1,122 @@ +(ns metabase.query-processor.middleware.parameters.native.substitute-test + (:require [expectations :refer [expect]] + [metabase.driver :as driver] + [metabase.models.field :refer [Field]] + [metabase.query-processor.middleware.parameters.native + [interface :as i] + [substitute :as substitute]] + [metabase.test.data :as data])) + +(defn- optional [& args] (i/->Optional args)) +(defn- param [param-name] (i/->Param param-name)) + +(defn- substitute [parsed param->value] + (driver/with-driver :h2 + (substitute/substitute parsed param->value))) + +;; normal substitution +(expect + ["select * from foobars where bird_type = ?" ["Steller's Jay"]] + (substitute + ["select * from foobars where bird_type = " (param "bird_type")] + {"bird_type" "Steller's Jay"})) + +;; make sure falsey values are substituted correctly +;; `nil` should get substituted as `NULL` +(expect + ["select * from foobars where bird_type = NULL" []] + (substitute + ["select * from foobars where bird_type = " (param "bird_type")] + {"bird_type" nil})) + +;; `false` should get substituted as `false` +(expect + ["select * from foobars where bird_type = FALSE" []] + (substitute + ["select * from foobars where bird_type = " (param "bird_type")] + {"bird_type" false})) + +;; optional substitution -- param present +(expect + ;; should preserve whitespace inside optional params + ["select * from foobars where bird_type = ?" ["Steller's Jay"]] + (substitute + ["select * from foobars " (optional " where bird_type = " (param "bird_type"))] + {"bird_type" "Steller's Jay"})) + +;; optional substitution -- param not present +(expect + ["select * from foobars" nil] + (substitute + ["select * from foobars " (optional " where bird_type = " (param "bird_type"))] + {})) + +;; optional -- multiple params -- all present +(expect + ["select * from foobars where bird_type = ? AND color = ?" ["Steller's Jay" "Blue"]] + (substitute + ["select * from foobars " (optional " where bird_type = " (param "bird_type") " AND color = " (param "bird_color"))] + {"bird_type" "Steller's Jay", "bird_color" "Blue"})) + +;; optional -- multiple params -- some present +(expect + ["select * from foobars" nil] + (substitute + ["select * from foobars " (optional " where bird_type = " (param "bird_type") " AND color = " (param "bird_color"))] + {"bird_type" "Steller's Jay"})) + +;; nested optionals -- all present +(expect + ["select * from foobars where bird_type = ? AND color = ?" ["Steller's Jay" "Blue"]] + (substitute + ["select * from foobars " (optional " where bird_type = " (param "bird_type") + (optional " AND color = " (param "bird_color")))] + {"bird_type" "Steller's Jay", "bird_color" "Blue"})) + +;; nested optionals -- some present +(expect + ["select * from foobars where bird_type = ?" ["Steller's Jay"]] + (substitute + ["select * from foobars " (optional " where bird_type = " (param "bird_type") + (optional " AND color = " (param "bird_color")))] + {"bird_type" "Steller's Jay"})) + +;;; ------------------------------------------------- Field Filters -------------------------------------------------- + +(defn- date-field-filter-value + "Field filter 'values' returned by the `values` namespace are actualy `FieldFilter` record types that contain information about" + [] + (i/map->FieldFilter + {:field (Field (data/id :checkins :date)) + :value {:type :date/single + :value #inst "2019-09-20T19:52:00.000-07:00"}})) + +;; field filter -- non-optional + present +(expect + ["select * from checkins where CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) = ?" + [#inst "2019-09-20T19:52:00.000-07:00"]] + (substitute + ["select * from checkins where " (param "date")] + {"date" (date-field-filter-value)})) + +;; field filter -- non-optional + missing -- should be replaced with 1 = 1 +(expect + ["select * from checkins where 1 = 1" []] + (substitute + ["select * from checkins where " (param "date")] + {"date" (assoc (date-field-filter-value) :value i/no-value)})) + +;; field filter -- optional + present +(expect + ["select * from checkins where CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) = ?" + [#inst "2019-09-20T19:52:00.000-07:00"]] + (substitute + ["select * from checkins " (optional "where " (param "date"))] + {"date" (date-field-filter-value)})) + +;; field filter -- optional + missing -- should be omitted entirely +(expect + ["select * from checkins" nil] + (substitute + ["select * from checkins " (optional "where " (param "date"))] + {"date" (assoc (date-field-filter-value) :value i/no-value)})) diff --git a/test/metabase/query_processor/middleware/parameters/native/substitution_test.clj b/test/metabase/query_processor/middleware/parameters/native/substitution_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..2a91b453a927434f0598cd1fddfc40b6b4d17b53 --- /dev/null +++ b/test/metabase/query_processor/middleware/parameters/native/substitution_test.clj @@ -0,0 +1,11 @@ +(ns metabase.query-processor.middleware.parameters.native.substitution-test + (:require [expectations :refer [expect]] + [metabase.driver :as driver] + [metabase.query-processor.middleware.parameters.native.substitution :as substitution])) + +;; make sure we handle quotes inside names correctly! +(expect + {:replacement-snippet "\"test-data\".\"PUBLIC\".\"checkins\".\"date\"" + :prepared-statement-args nil} + (driver/with-driver :h2 + (#'substitution/honeysql->replacement-snippet-info :test-data.PUBLIC.checkins.date))) diff --git a/test/metabase/query_processor/middleware/parameters/native/values_test.clj b/test/metabase/query_processor/middleware/parameters/native/values_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..9db24b3306dae30a70612825a7b3b61850e15b0b --- /dev/null +++ b/test/metabase/query_processor/middleware/parameters/native/values_test.clj @@ -0,0 +1,119 @@ +(ns metabase.query-processor.middleware.parameters.native.values-test + (:require [expectations :refer [expect]] + [metabase.query-processor.middleware.parameters.native + [interface :as i] + [values :as values]] + [metabase.test.data :as data])) + +;; variable -- specified +(expect + "2" + (#'values/value-for-tag + {:name "id", :display-name "ID", :type :text, :required true, :default "100"} + [{:type :category, :target [:variable [:template-tag "id"]], :value "2"}])) + +;; variable -- unspecified +(expect + i/no-value + (#'values/value-for-tag + {:name "id", :display-name "ID", :type :text} nil)) + +;; variable -- default +(expect + "100" + (#'values/value-for-tag + {:name "id", :display-name "ID", :type :text, :required true, :default "100"} nil)) + +;; Field Filter -- specified +(expect + {:field {:name "DATE" + :parent_id nil + :table_id (data/id :checkins) + :base_type :type/Date} + :value {:type :date/range + :value "2015-04-01~2015-05-01"}} + (into {} (#'values/value-for-tag + {:name "checkin_date" + :display-name "Checkin Date" + :type :dimension + :dimension [:field-id (data/id :checkins :date)]} + [{:type :date/range, :target [:dimension [:template-tag "checkin_date"]], :value "2015-04-01~2015-05-01"}]))) + +;; Field Filter -- unspecified +(expect + {:field {:name "DATE" + :parent_id nil + :table_id (data/id :checkins) + :base_type :type/Date} + :value i/no-value} + (into {} (#'values/value-for-tag + {:name "checkin_date" + :display-name "Checkin Date" + :type :dimension + :dimension [:field-id (data/id :checkins :date)]} + nil))) +;; Field Filter -- id requiring casting +(expect + {:field {:name "ID" + :parent_id nil + :table_id (data/id :checkins) + :base_type :type/BigInteger} + :value {:type :id + :value 5}} + (into {} (#'values/value-for-tag + {:name "id", :display-name "ID", :type :dimension, :dimension [:field-id (data/id :checkins :id)]} + [{:type :id, :target [:dimension [:template-tag "id"]], :value "5"}]))) + +;; Field Filter -- required but unspecified +(expect Exception + (into {} (#'values/value-for-tag + {:name "checkin_date", :display-name "Checkin Date", :type "dimension", :required true, + :dimension ["field-id" (data/id :checkins :date)]} + nil))) + +;; Field Filter -- required and default specified +(expect + {:field {:name "DATE" + :parent_id nil + :table_id (data/id :checkins) + :base_type :type/Date} + :value {:type :dimension + :value "2015-04-01~2015-05-01"}} + (into {} (#'values/value-for-tag + {:name "checkin_date" + :display-name "Checkin Date" + :type :dimension + :required true + :default "2015-04-01~2015-05-01", + :dimension [:field-id (data/id :checkins :date)]} + nil))) + + +;; multiple values for the same tag should return a vector with multiple params instead of a single param +(expect + {:field {:name "DATE" + :parent_id nil + :table_id (data/id :checkins) + :base_type :type/Date} + :value [{:type :date/range + :value "2015-01-01~2016-09-01"} + {:type :date/single + :value "2015-07-01"}]} + (into {} (#'values/value-for-tag + {:name "checkin_date", :display-name "Checkin Date", :type :dimension, :dimension [:field-id (data/id :checkins :date)]} + [{:type :date/range, :target [:dimension [:template-tag "checkin_date"]], :value "2015-01-01~2016-09-01"} + {:type :date/single, :target [:dimension [:template-tag "checkin_date"]], :value "2015-07-01"}]))) + +;; Make sure defaults values get picked up for field filter clauses +(expect + {:field {:name "DATE", :parent_id nil, :table_id (data/id :checkins), :base_type :type/Date} + :value {:type :date/all-options + :value "past5days"}} + (#'values/field-filter-value-for-tag + {:name "checkin_date" + :display-name "Checkin Date" + :type :dimension + :dimension [:field-id (data/id :checkins :date)] + :default "past5days" + :widget-type :date/all-options} + nil)) diff --git a/test/metabase/query_processor/middleware/parameters/sql_test.clj b/test/metabase/query_processor/middleware/parameters/native_test.clj similarity index 65% rename from test/metabase/query_processor/middleware/parameters/sql_test.clj rename to test/metabase/query_processor/middleware/parameters/native_test.clj index 4911bc5a69350fd91809d7b52f4eb014102f7de1..1bdb3a8746845c2bbae0a79d15bf75c5cd99ee1b 100644 --- a/test/metabase/query_processor/middleware/parameters/sql_test.clj +++ b/test/metabase/query_processor/middleware/parameters/native_test.clj @@ -1,5 +1,5 @@ -(ns metabase.query-processor.middleware.parameters.sql-test - "Tests for parameters in native SQL queries, which are of the `{{param}}` form." +(ns metabase.query-processor.middleware.parameters.native-test + "E2E tests for SQL param substitution." (:require [clj-time.core :as t] [expectations :refer [expect]] [metabase @@ -7,7 +7,10 @@ [query-processor :as qp] [query-processor-test :as qp.test]] [metabase.mbql.normalize :as normalize] - [metabase.query-processor.middleware.parameters.sql :as sql] + [metabase.query-processor.middleware.parameters.native :as native] + [metabase.query-processor.middleware.parameters.native + [parse :as parse] + [substitute :as substitute]] [metabase.test [data :as data] [util :as tu]] @@ -17,119 +20,31 @@ [schema :as su]] [schema.core :as s])) -;;; ----------------------------------------------- basic parser tests ----------------------------------------------- - -(defn- parse-template - ([sql] - (parse-template sql {})) - - ([sql param-key->value] - (driver/with-driver :h2 - (#'sql/parse-template sql param-key->value)))) - -(expect - {:query "select * from foo where bar=1" - :params []} - (parse-template "select * from foo where bar=1")) - -(expect - {:query "select * from foo where bar=?" - :params ["foo"]} - (parse-template "select * from foo where bar={{baz}}" {"baz" "foo"})) - -(expect - {:query "select * from foo where bar = ?" - :params ["foo"]} - (parse-template "select * from foo [[where bar = {{baz}} ]]" {"baz" "foo"})) - -;; Multiple optional clauses, all present -(expect - {:query "select * from foo where bar1 = ? and bar2 = ? and bar3 = ? and bar4 = ?" - :params (repeat 4 "foo")} - (parse-template (str "select * from foo where bar1 = {{baz}} " - "[[and bar2 = {{baz}}]] " - "[[and bar3 = {{baz}}]] " - "[[and bar4 = {{baz}}]]") - {"baz" "foo"})) - -;; Multiple optional clauses, none present -(expect - {:query "select * from foo where bar1 = ?" - :params ["foo"]} - (parse-template (str "select * from foo where bar1 = {{baz}} " - "[[and bar2 = {{none}}]] " - "[[and bar3 = {{none}}]] " - "[[and bar4 = {{none}}]]") - {"baz" "foo"})) - -(expect - {:query "select * from foobars where foobars.id in (string_to_array(?, ',')::integer[])" - :params ["foo"]} - (parse-template "select * from foobars [[ where foobars.id in (string_to_array({{foobar_id}}, ',')::integer[]) ]]" - {"foobar_id" "foo"})) - -(expect - {:query (str "SELECT [test_data.checkins.venue_id] AS [venue_id], " - " [test_data.checkins.user_id] AS [user_id], " - " [test_data.checkins.id] AS [checkins_id] " - "FROM [test_data.checkins] " - "LIMIT 2") - :params []} - (parse-template (str "SELECT [test_data.checkins.venue_id] AS [venue_id], " - " [test_data.checkins.user_id] AS [user_id], " - " [test_data.checkins.id] AS [checkins_id] " - "FROM [test_data.checkins] " - "LIMIT 2"))) - -;; Valid syntax in PG -(expect - {:query "SELECT array_dims(1 || '[0:1]={2,3}'::int[])" - :params []} - (parse-template "SELECT array_dims(1 || '[0:1]={2,3}'::int[])")) - -;; Testing that invalid/unterminated template params/clauses throw an exception -(expect - IllegalArgumentException - (parse-template "select * from foo [[where bar = {{baz}} " {"baz" "foo"})) - -(expect - IllegalArgumentException - (parse-template "select * from foo [[where bar = {{baz]]" {"baz" "foo"})) - -(expect - IllegalArgumentException - (parse-template "select * from foo {{bar}} {{baz" {"bar" "foo", "baz" "foo"})) - -(expect - IllegalArgumentException - (parse-template "select * from foo [[clause 1 {{bar}}]] [[clause 2" {"bar" "foo"})) - - ;;; ------------------------------------------ simple substitution -- {{x}} ------------------------------------------ -(defn- substitute {:style/indent 1} [sql params] - ;; apparently you can still bind private dynamic vars - (driver/with-driver :h2 - (#'sql/expand-query-params {:query sql} (into {} params)))) +(defn- substitute-e2e {:style/indent 1} [sql params] + (let [[query params] (driver/with-driver :h2 + (#'substitute/substitute (parse/parse sql) (into {} params)))] + {:query query, :params (vec params)})) (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE" :params []} - (substitute "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}}" + (substitute-e2e "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}}" {"toucans_are_cool" true})) (expect Exception - (substitute "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}}" + (substitute-e2e "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}}" nil)) (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE AND bird_type = ?" :params ["toucan"]} - (substitute "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = {{bird_type}}" + (substitute-e2e "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = {{bird_type}}" {"toucans_are_cool" true, "bird_type" "toucan"})) (expect Exception - (substitute "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = {{bird_type}}" + (substitute-e2e "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = {{bird_type}}" {"toucans_are_cool" true})) ;;; ---------------------------------- optional substitution -- [[ ... {{x}} ... ]] ---------------------------------- @@ -137,232 +52,144 @@ (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE" :params []} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}}]]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}}]]" {"toucans_are_cool" true})) (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE" :params []} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{ toucans_are_cool }}]]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{ toucans_are_cool }}]]" {"toucans_are_cool" true})) (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE" :params []} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool }}]]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool }}]]" {"toucans_are_cool" true})) (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE" :params []} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{ toucans_are_cool}}]]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{ toucans_are_cool}}]]" {"toucans_are_cool" true})) (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE" :params []} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool_2}}]]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool_2}}]]" {"toucans_are_cool_2" true})) (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE AND bird_type = 'toucan'" :params []} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = 'toucan']]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = 'toucan']]" {"toucans_are_cool" true})) ;; Two parameters in an optional (expect {:query "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE AND bird_type = ?" :params ["toucan"]} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = {{bird_type}}]]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = {{bird_type}}]]" {"toucans_are_cool" true, "bird_type" "toucan"})) (expect {:query "SELECT * FROM bird_facts" :params []} - (substitute "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}}]]" + (substitute-e2e "SELECT * FROM bird_facts [[WHERE toucans_are_cool = {{toucans_are_cool}}]]" nil)) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > 5" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" {"num_toucans" 5})) -;; make sure nil gets substituted in as `NULL` +;; make sure nil gets substitute-e2ed in as `NULL` (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > NULL" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" {"num_toucans" nil})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > TRUE" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" {"num_toucans" true})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > FALSE" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" {"num_toucans" false})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > ?" :params ["abc"]} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" {"num_toucans" "abc"})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > ?" :params ["yo' mama"]} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" {"num_toucans" "yo' mama"})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]]" nil)) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > 2 AND total_birds > 5" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" {"num_toucans" 2, "total_birds" 5})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND total_birds > 5" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" {"total_birds" 5})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > 3" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" {"num_toucans" 3})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" nil)) (expect {:query "SELECT * FROM toucanneries WHERE bird_type = ? AND num_toucans > 2 AND total_birds > 5" :params ["toucan"]} - (substitute "SELECT * FROM toucanneries WHERE bird_type = {{bird_type}} [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE bird_type = {{bird_type}} [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" {"bird_type" "toucan", "num_toucans" 2, "total_birds" 5})) (expect Exception - (substitute "SELECT * FROM toucanneries WHERE bird_type = {{bird_type}} [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE bird_type = {{bird_type}} [[AND num_toucans > {{num_toucans}}]] [[AND total_birds > {{total_birds}}]]" {"num_toucans" 2, "total_birds" 5})) (expect {:query "SELECT * FROM toucanneries WHERE TRUE AND num_toucans > 5 AND num_toucans < 5" :params []} - (substitute "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND num_toucans < {{num_toucans}}]]" + (substitute-e2e "SELECT * FROM toucanneries WHERE TRUE [[AND num_toucans > {{num_toucans}}]] [[AND num_toucans < {{num_toucans}}]]" {"num_toucans" 5})) ;; Make sure that substiutions still work if the subsitution contains brackets inside it (#3657) (expect {:query "select * from foobars where foobars.id in (string_to_array(100, ',')::integer[])" :params []} - (substitute "select * from foobars [[ where foobars.id in (string_to_array({{foobar_id}}, ',')::integer[]) ]]" + (substitute-e2e "select * from foobars [[ where foobars.id in (string_to_array({{foobar_id}}, ',')::integer[]) ]]" {"foobar_id" 100})) -;;; -------------------------------------------- tests for value-for-tag --------------------------------------------- - -;; variable -- specified -(expect - "2" - (#'sql/value-for-tag {:name "id", :display-name "ID", :type :text, :required true, :default "100"} - [{:type :category, :target [:variable [:template-tag "id"]], :value "2"}])) - -;; variable -- unspecified -(expect - #metabase.query_processor.middleware.parameters.sql.NoValue{} - (#'sql/value-for-tag {:name "id", :display-name "ID", :type :text} nil)) - -;; variable -- default -(expect - "100" - (#'sql/value-for-tag {:name "id", :display-name "ID", :type :text, :required true, :default "100"} nil)) - -;; dimension -- specified -(expect - {:field {:name "DATE" - :parent_id nil - :table_id (data/id :checkins) - :base_type :type/Date} - :param {:type :date/range - :target [:dimension [:template-tag "checkin_date"]] - :value "2015-04-01~2015-05-01"}} - (into {} (#'sql/value-for-tag {:name "checkin_date", :display-name "Checkin Date", :type :dimension, :dimension [:field-id (data/id :checkins :date)]} - [{:type :date/range, :target [:dimension [:template-tag "checkin_date"]], :value "2015-04-01~2015-05-01"}]))) - -;; dimension -- unspecified -(expect - {:field {:name "DATE" - :parent_id nil - :table_id (data/id :checkins) - :base_type :type/Date} - :param nil} - (into {} (#'sql/value-for-tag {:name "checkin_date", :display-name "Checkin Date", :type :dimension, :dimension [:field-id (data/id :checkins :date)]} - nil))) -;; dimension -- id requiring casting -(expect - {:field {:name "ID" - :parent_id nil - :table_id (data/id :checkins) - :base_type :type/BigInteger} - :param {:type :id - :target [:dimension [:template-tag "id"]] - :value 5}} - (into {} (#'sql/value-for-tag {:name "id", :display-name "ID", :type :dimension, :dimension [:field-id (data/id :checkins :id)]} - [{:type :id, :target [:dimension [:template-tag "id"]], :value "5"}]))) - -;; dimension -- required but unspecified -(expect Exception - (into {} (#'sql/value-for-tag {:name "checkin_date", :display-name "Checkin Date", :type "dimension", :required true, - :dimension ["field-id" (data/id :checkins :date)]} - nil))) - -;; dimension -- required and default specified -(expect - {:field {:name "DATE" - :parent_id nil - :table_id (data/id :checkins) - :base_type :type/Date} - :param {:type :dimension - :target [:dimension [:template-tag "checkin_date"]] - :value "2015-04-01~2015-05-01"}} - (into {} (#'sql/value-for-tag {:name "checkin_date", :display-name "Checkin Date", :type :dimension, :required true, :default "2015-04-01~2015-05-01", - :dimension [:field-id (data/id :checkins :date)]} - nil))) - - -;; multiple values for the same tag should return a vector with multiple params instead of a single param -(expect - {:field {:name "DATE" - :parent_id nil - :table_id (data/id :checkins) - :base_type :type/Date} - :param [{:type :date/range - :target [:dimension [:template-tag "checkin_date"]] - :value "2015-01-01~2016-09-01"} - {:type :date/single - :target [:dimension [:template-tag "checkin_date"]] - :value "2015-07-01"}]} - (into {} (#'sql/value-for-tag {:name "checkin_date", :display-name "Checkin Date", :type :dimension, :dimension [:field-id (data/id :checkins :date)]} - [{:type :date/range, :target [:dimension [:template-tag "checkin_date"]], :value "2015-01-01~2016-09-01"} - {:type :date/single, :target [:dimension [:template-tag "checkin_date"]], :value "2015-07-01"}]))) - - ;;; ------------------------------------------- expansion tests: variables ------------------------------------------- (defmacro ^:private with-h2-db-timezone @@ -375,12 +202,19 @@ :id 1} ~@body)) +(defn- expand** + "Expand parameters inside a top-level native `query`. Not recursive. " + [{:keys [parameters], inner :native, :as query}] + (let [inner' (native/expand-inner (update inner :parameters #(concat parameters %)))] + (assoc query :native inner'))) + (defn- expand* [query] (-> (with-h2-db-timezone (driver/with-driver :h2 - (sql/expand (normalize/normalize query)))) + (expand** (normalize/normalize query)))) :native - (select-keys [:query :params :template-tags]))) + (select-keys [:query :params :template-tags]) + (update :params vec))) ;; unspecified optional param (expect @@ -393,9 +227,9 @@ ;; unspecified *required* param (expect Exception - (sql/expand {:native {:query "SELECT * FROM orders [[WHERE id = {{id}}]];" - :template-tags {"id" {:name "id", :display-name "ID", :type :number, :required true}}} - :parameters []})) + (expand** {:native {:query "SELECT * FROM orders [[WHERE id = {{id}}]];" + :template-tags {"id" {:name "id", :display-name "ID", :type :number, :required true}}} + :parameters []})) ;; default value (expect @@ -430,139 +264,152 @@ :parameters [{:type "category", :target [:variable [:template-tag "category"]], :value "Gizmo"}]})) -;;; ------------------------------------------ expansion tests: dimensions ------------------------------------------- +;;; ----------------------------------------- expansion tests: field filters ----------------------------------------- -(defn- expand-with-dimension-param [dimension-param] - (with-redefs [t/now (constantly (t/date-time 2016 06 07 12 0 0))] - (-> {:native {:query "SELECT * FROM checkins WHERE {{date}};" - :template-tags {"date" {:name "date" - :display-name "Checkin Date" - :type :dimension - :dimension [:field-id (data/id :checkins :date)]}}} - :parameters (when dimension-param - [(merge {:target [:dimension [:template-tag "date"]]} - dimension-param)])} - expand* - (dissoc :template-tags)))) +(defn- expand-with-field-filter-param + ([field-filter-param] + (expand-with-field-filter-param "SELECT * FROM checkins WHERE {{date}};" field-filter-param)) + + ([sql field-filter-param] + (with-redefs [t/now (constantly (t/date-time 2016 06 07 12 0 0))] + (-> {:native {:query sql + :template-tags {"date" {:name "date" + :display-name "Checkin Date" + :type :dimension + :dimension [:field-id (data/id :checkins :date)]}}} + :parameters (when field-filter-param + [(merge {:target [:dimension [:template-tag "date"]]} + field-filter-param)])} + expand* + (dissoc :template-tags))))) ;; dimension (date/single) (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) = ?;" :params [#inst "2016-07-01T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/single, :value "2016-07-01"})) + (expand-with-field-filter-param {:type :date/single, :value "2016-07-01"})) ;; dimension (date/range) (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-07-01T00:00:00.000000000-00:00" #inst "2016-08-01T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "2016-07-01~2016-08-01"})) + (expand-with-field-filter-param {:type :date/range, :value "2016-07-01~2016-08-01"})) ;; dimension (date/month-year) (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-07-01T00:00:00.000000000-00:00" #inst "2016-07-31T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/month-year, :value "2016-07"})) + (expand-with-field-filter-param {:type :date/month-year, :value "2016-07"})) ;; dimension (date/quarter-year) (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-01-01T00:00:00.000000000-00:00" #inst "2016-03-31T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/quarter-year, :value "Q1-2016"})) + (expand-with-field-filter-param {:type :date/quarter-year, :value "Q1-2016"})) ;; dimension (date/all-options, before) (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) < ?;" :params [#inst "2016-07-01T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/all-options, :value "~2016-07-01"})) + (expand-with-field-filter-param {:type :date/all-options, :value "~2016-07-01"})) ;; dimension (date/all-options, after) (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) > ?;" :params [#inst "2016-07-01T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/all-options, :value "2016-07-01~"})) + (expand-with-field-filter-param {:type :date/all-options, :value "2016-07-01~"})) ;; relative date -- "yesterday" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) = ?;" :params [#inst "2016-06-06T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "yesterday"})) + (expand-with-field-filter-param {:type :date/range, :value "yesterday"})) ;; relative date -- "past7days" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-05-31T00:00:00.000000000-00:00" #inst "2016-06-06T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "past7days"})) + (expand-with-field-filter-param {:type :date/range, :value "past7days"})) ;; relative date -- "past30days" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-05-08T00:00:00.000000000-00:00" #inst "2016-06-06T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "past30days"})) + (expand-with-field-filter-param {:type :date/range, :value "past30days"})) ;; relative date -- "thisweek" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-06-05T00:00:00.000000000-00:00" #inst "2016-06-11T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "thisweek"})) + (expand-with-field-filter-param {:type :date/range, :value "thisweek"})) ;; relative date -- "thismonth" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-06-01T00:00:00.000000000-00:00" #inst "2016-06-30T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "thismonth"})) + (expand-with-field-filter-param {:type :date/range, :value "thismonth"})) ;; relative date -- "thisyear" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-01-01T00:00:00.000000000-00:00" #inst "2016-12-31T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "thisyear"})) + (expand-with-field-filter-param {:type :date/range, :value "thisyear"})) ;; relative date -- "lastweek" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-05-29T00:00:00.000000000-00:00" #inst "2016-06-04T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "lastweek"})) + (expand-with-field-filter-param {:type :date/range, :value "lastweek"})) ;; relative date -- "lastmonth" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2016-05-01T00:00:00.000000000-00:00" #inst "2016-05-31T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "lastmonth"})) + (expand-with-field-filter-param {:type :date/range, :value "lastmonth"})) ;; relative date -- "lastyear" (expect {:query "SELECT * FROM checkins WHERE CAST(\"PUBLIC\".\"CHECKINS\".\"DATE\" AS date) BETWEEN ? AND ?;" :params [#inst "2015-01-01T00:00:00.000000000-00:00" #inst "2015-12-31T00:00:00.000000000-00:00"]} - (expand-with-dimension-param {:type :date/range, :value "lastyear"})) + (expand-with-field-filter-param {:type :date/range, :value "lastyear"})) ;; dimension with no value -- just replace with an always true clause (e.g. "WHERE 1 = 1") (expect {:query "SELECT * FROM checkins WHERE 1 = 1;" :params []} - (expand-with-dimension-param nil)) + (expand-with-field-filter-param nil)) -;; dimension -- number +;; dimension -- number -- should get parsed to Number (expect {:query "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" = 100;" :params []} - (expand-with-dimension-param {:type :number, :value "100"})) + (expand-with-field-filter-param {:type :number, :value "100"})) ;; dimension -- text (expect {:query "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" = ?;" :params ["100"]} - (expand-with-dimension-param {:type :text, :value "100"})) + (expand-with-field-filter-param {:type :text, :value "100"})) + +;; *OPTIONAL* Field Filter params should not get replaced with 1 = 1 if the param is not present (#5541, #9489). +;; *Optional params should be emitted entirely. +(expect + {:query "SELECT * FROM ORDERS WHERE TOTAL > 100 AND CREATED_AT < now()" + :params []} + (expand-with-field-filter-param + "SELECT * FROM ORDERS WHERE TOTAL > 100 [[AND {{created}} #]] AND CREATED_AT < now()" + nil)) ;;; -------------------------------------------- "REAL" END-TO-END-TESTS --------------------------------------------- @@ -671,16 +518,6 @@ :template-tags {"date" {:name "date" :display-name "Date" :type :date}}} :parameters [{:type :date/single :target [:variable [:template-tag "date"]] :value "2018-04-18"}])))) - -;;; -------------------------------------------- SQL PARAMETERS 2.0 TESTS -------------------------------------------- - -;; make sure we handle quotes inside names correctly! -(expect - {:replacement-snippet "\"test-data\".\"PUBLIC\".\"checkins\".\"date\"", - :prepared-statement-args nil} - (driver/with-driver :postgres - (#'sql/honeysql->replacement-snippet-info :test-data.PUBLIC.checkins.date))) - ;; Some random end-to-end param expansion tests added as part of the SQL Parameters 2.0 rewrite (expect @@ -763,21 +600,6 @@ :target [:dimension [:template-tag "checkin_date"]] :value "past5days"}]}))) -;; Make sure defaults values get picked up for field filter clauses -(expect - {:field {:name "DATE", :parent_id nil, :table_id (data/id :checkins), :base_type :type/Date} - :param {:type :date/all-options - :target [:dimension [:template-tag "checkin_date"]] - :value "past5days"}} - (#'sql/dimension-value-for-tag - {:name "checkin_date" - :display-name "Checkin Date" - :type :dimension - :dimension [:field-id (data/id :checkins :date)] - :default "past5days" - :widget-type :date/all-options} - nil)) - ;; Make sure we can specify the type of a default value for a "Dimension" (Field Filter) by setting the ;; `:widget-type` key. Check that it works correctly with relative dates... (expect