Skip to content
Snippets Groups Projects
Unverified Commit 2591fb3d authored by Ryan Senior's avatar Ryan Senior Committed by GitHub
Browse files

Revert "Switch sql templates to an instaparse grammar"

parent c12fa036
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
project.clj
+ 0
− 1
View file @ 2591fb3d
......@@ -64,7 +64,6 @@
[hiccup "1.0.5"] ; HTML templating
[honeysql "0.8.2"] ; Transform Clojure data structures to SQL
[io.crate/crate-jdbc "2.1.6"] ; Crate JDBC driver
[instaparse "1.4.0"] ; Insaparse parser generator
[kixi/stats "0.3.10" ; Various statistic measures implemented as transducers
:exclusions [org.clojure/test.check ; test.check and AVL trees are used in kixi.stats.random. Remove exlusion if using.
org.clojure/data.avl]]
......
src/metabase/query_processor/middleware/parameters/sql.clj
+ 115
− 89
View file @ 2591fb3d
......@@ -6,7 +6,6 @@
(:require [clojure.string :as str]
[clojure.tools.logging :as log]
[honeysql.core :as hsql]
[instaparse.core :as insta]
[metabase.models.field :as field :refer [Field]]
[metabase.query-processor.middleware.parameters.dates :as date-params]
[metabase.query-processor.middleware.expand :as ql]
......@@ -63,8 +62,6 @@
;; values.
(defrecord ^:private NoValue [])
(defn- no-value? [x]
(instance? NoValue x))
;; various schemas are used to check that various functions return things in expected formats
......@@ -111,7 +108,11 @@
{s/Keyword ParamValue})
(def ^:private ParamSnippetInfo
{(s/optional-key :replacement-snippet) s/Str ; allowed to be blank if this is an optional param
{(s/optional-key :param-key) s/Keyword
(s/optional-key :original-snippet) su/NonBlankString
(s/optional-key :variable-snippet) su/NonBlankString
(s/optional-key :optional-snippet) (s/maybe su/NonBlankString)
(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]})
;;; +----------------------------------------------------------------------------------------------------------------+
......@@ -222,7 +223,7 @@
(s/defn ^:private parse-value-for-type :- ParamValue
[param-type value]
(cond
(no-value? value) value
(instance? NoValue value) value
(= param-type "number") (value->number value)
(= param-type "date") (map->Date {:s value})
(and (= param-type "dimension")
......@@ -388,98 +389,123 @@
:else
(update (dimension->replacement-snippet-info param) :replacement-snippet (partial str (field->identifier field (:type param)) " ")))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | QUERY PARSING / PARAM SNIPPETS |
;;; +----------------------------------------------------------------------------------------------------------------+
;; These functions parse a query and look for param snippets, which look like:
;;
;; * {{...}} (required)
;; * [[...{{...}}...]] (optional)
;;
;; and creates a list of these snippets, keeping the original order.
;;
;; The details maps returned have the format:
;;
;; {:param-key :timestamp ; name of the param being replaced
;; :original-snippet "[[AND timestamp < {{timestamp}}]]" ; full text of the snippet to be replaced
;; :optional-snippet "AND timestamp < {{timestamp}}" ; portion of the snippet inside [[optional]] brackets, or `nil` if the snippet isn't optional
;; :variable-snippet "{{timestamp}}"} ; portion of the snippet referencing the variable itself, e.g. {{x}}
(s/defn ^:private param-snippet->param-name :- s/Keyword
"Return the keyword name of the param being referenced inside PARAM-SNIPPET.
(param-snippet->param-name \"{{x}}\") -> :x"
[param-snippet :- su/NonBlankString]
(keyword (second (re-find #"\{\{\s*(\w+)\s*\}\}" param-snippet))))
(s/defn ^:private sql->params-snippets-info :- [ParamSnippetInfo]
"Return a sequence of maps containing information about the param snippets found by paring SQL."
[sql :- su/NonBlankString]
(for [[param-snippet optional-replacement-snippet] (re-seq #"(?:\[\[(.+?)\]\])|(?:\{\{\s*\w+\s*\}\})" sql)]
{:param-key (param-snippet->param-name param-snippet)
:original-snippet param-snippet
:variable-snippet (re-find #"\{\{\s*\w+\s*\}\}" param-snippet)
:optional-snippet optional-replacement-snippet}))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | PARSING THE SQL TEMPLATE |
;;; | PARAMS DETAILS LIST |
;;; +----------------------------------------------------------------------------------------------------------------+
(def ^:private sql-template-parser
(insta/parser
"SQL := (ANYTHING_BUT_BRACE | PARAM | OPTIONAL)*
;; These functions combine the info from the other 3 stages (Param Info Resolution, Param->SQL Substitution, and Query
;; Parsing) and create a sequence of maps containing param details that has all the information needed to do SQL
;; substituion. This sequence is returned in the same order as params encountered in the
;;
;; original query, making passing prepared statement args simple
;;
;; The details maps returned have the format:
;;
;; {:original-snippet "[[AND timestamp < {{timestamp}}]]" ; full text of the snippet to be replaced
;; :replacement-snippet "AND timestamp < ?" ; full text that the snippet should be replaced with
;; :prepared-statement-args [#inst "2016-01-01"]} ; prepared statement args needed by the replacement snippet
;;
;; (Basically these functions take `:param-key`, `:optional-snippet`, and `:variable-snippet` from the Query Parsing stage and the info from the other stages
;; to add the appropriate info for `:replacement-snippet` and `:prepared-statement-args`.)
(s/defn ^:private snippet-value :- ParamValue
"Fetch the value from PARAM-KEY->VALUE for SNIPPET-INFO.
If no value is specified, return `NoValue` if the snippet is optional; otherwise throw an Exception."
[{:keys [param-key optional-snippet]} :- ParamSnippetInfo, param-key->value :- ParamValues]
(u/prog1 (get param-key->value param-key (NoValue.))
;; if ::no-value was specified an the param is not [[optional]], throw an exception
(when (and (instance? NoValue <>)
(not optional-snippet))
(throw (ex-info (format "Unable to substitute '%s': param not specified.\nFound: %s" param-key (keys param-key->value))
{:status-code 400})))))
(s/defn ^:private handle-optional-snippet :- ParamSnippetInfo
"Create the approprate `:replacement-snippet` for PARAM, combining the value of REPLACEMENT-SNIPPET from the Param->SQL Substitution phase
with the OPTIONAL-SNIPPET, if any."
[{:keys [variable-snippet optional-snippet replacement-snippet prepared-statement-args], :as snippet-info} :- ParamSnippetInfo]
(assoc snippet-info
:replacement-snippet (cond
(not optional-snippet) replacement-snippet ; if there is no optional-snippet return replacement as-is
(seq replacement-snippet) (str/replace optional-snippet variable-snippet replacement-snippet) ; if replacement-snippet is non blank splice into optional-snippet
:else "") ; otherwise return blank replacement (i.e. for NoValue)
;; for every thime the `variable-snippet` occurs in the `optional-snippet` we need to supply an additional set of `prepared-statment-args`
;; e.g. [[ AND ID = {{id}} OR USER_ID = {{id}} ]] should have *2* sets of the prepared statement args for {{id}} since it occurs twice
:prepared-statement-args (if-let [occurances (u/occurances-of-substring optional-snippet variable-snippet)]
(apply concat (repeat occurances prepared-statement-args))
prepared-statement-args)))
(s/defn ^:private add-replacement-snippet-info :- [ParamSnippetInfo]
"Add `:replacement-snippet` and `:prepared-statement-args` info to the maps in PARAMS-SNIPPETS-INFO by looking at
PARAM-KEY->VALUE and using the Param->SQL substituion functions."
[params-snippets-info :- [ParamSnippetInfo], param-key->value :- ParamValues]
(for [snippet-info params-snippets-info]
(handle-optional-snippet
(merge snippet-info
(s/validate ParamSnippetInfo (->replacement-snippet-info (snippet-value snippet-info param-key->value)))))))
(* optional clauses don't support nesting, but can contains SQL text and params *)
OPTIONAL := <'[['> (ANYTHING_BUT_BRACE | PARAM)* <']]'>
<ANYTHING_BUT_BRACE> := (SQL_CHARS SINGLE_BRACE? SINGLE_BRACKET?)*
(* Any characters other than {,},[,]. Those need to be handled separately. *)
<SQL_CHARS> := #'[\\W\\w&&[^\\{\\}\\[\\]]]*'
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | SUBSTITUITION |
;;; +----------------------------------------------------------------------------------------------------------------+
(* Single braces/brackets can occur in SQL and should not be recognized as a param/optional *)
<SINGLE_BRACE> := '{' !'{' | '}' !'}'
<SINGLE_BRACKET> := '[' !'[' | ']' !']'
PARAM = <'{{'> <WHITESPACE*> TOKEN <WHITESPACE*> <'}}'>
;; These functions take the information about parameters from the Params Details List functions and then convert the
;; original SQL Query into a SQL query with appropriate subtitutions and a sequence of prepared statement args
(* Any word character is a valid token [a-zA-Z_0-9] *)
<TOKEN> := #'(\\w)+'
WHITESPACE := #'\\s'"))
(s/defn ^:private substitute-one
[sql :- su/NonBlankString, {:keys [original-snippet replacement-snippet]} :- ParamSnippetInfo]
(str/replace-first sql original-snippet replacement-snippet))
(defrecord ^:private Param [param-key sql-value prepared-statement-args])
(defn- param? [maybe-param]
(instance? Param maybe-param))
(s/defn ^:private substitute :- {:query su/NonBlankString, :params [s/Any]}
"Using the PARAM-SNIPPET-INFO built from the stages above, replace the snippets in SQL and return a vector of
`[sql & prepared-statement-params]`."
{:style/indent 1}
[sql :- su/NonBlankString, param-snippets-info :- [ParamSnippetInfo]]
(log/debug (format "PARAM INFO: %s\n%s" (u/emoji "🔥") (u/pprint-to-str 'yellow param-snippets-info)))
(loop [sql sql, prepared-statement-args [], [snippet-info & more] param-snippets-info]
(if-not snippet-info
{:query (str/trim sql), :params (for [arg prepared-statement-args]
((resolve 'metabase.driver.generic-sql/prepare-sql-param) *driver* arg))}
(recur (substitute-one sql snippet-info)
(concat prepared-statement-args (:prepared-statement-args snippet-info))
more))))
(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- transform-sql
"Returns the combined query-map from all of the parameters, optional clauses etc. At this point there should not be
a NoValue leaf. If so, it's an error (i.e. missing a required parameter."
[param-key->value]
(fn [& nodes]
(doseq [maybe-param nodes
:when (no-value-param? maybe-param)]
(throw (ex-info (format "Unable to substitute '%s': param not specified.\nFound: %s"
(:param-name maybe-param) (keys param-key->value))
{:status-code 400})))
(-> (reduce merge-query-map empty-query-map nodes)
(update :query str/trim))))
(defn- transform-optional
"Converts the `OPTIONAL` clause to a query map. If one or more parameters are not populated for this optional
clause, it will return an empty-query-map, which will be omitted from the query."
[& nodes]
(if (some no-value-param? nodes)
empty-query-map
(reduce merge-query-map empty-query-map nodes)))
(defn- transform-param
"Converts a `PARAM` parse leaf to a query map that includes the SQL snippet to replace the `{{param}}` value and the
param itself for the prepared statement"
[param-key->value]
(fn [token]
(let [val (get param-key->value (keyword token) (NoValue.))]
(if (no-value? val)
(map->Param {:param-key token, :sql-value val, :prepared-statement-args []})
(let [{:keys [replacement-snippet prepared-statement-args]} (->replacement-snippet-info val)]
(map->Param {:param-key token
:sql-value replacement-snippet
:prepared-statement-args prepared-statement-args}))))))
(defn- parse-transform-map
"Instaparse returns things like [:SQL token token token...]. This map will be used when crawling the parse tree from
the bottom up. When encountering the a `:PARAM` node, it will invoke the included function, invoking the function
with each item in the list as arguments "
[param-key->value]
{:SQL (transform-sql param-key->value)
:OPTIONAL transform-optional
:PARAM (transform-param param-key->value)})
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | PUTTING IT ALL TOGETHER |
......@@ -487,8 +513,8 @@
(s/defn ^:private expand-query-params
[{sql :query, :as native}, param-key->value :- ParamValues]
(let [parsed-template (insta/parse sql-template-parser sql)]
(merge native (insta/transform (parse-transform-map param-key->value) parsed-template))))
(merge native (when-let [param-snippets-info (seq (add-replacement-snippet-info (sql->params-snippets-info sql) param-key->value))]
(substitute sql param-snippets-info))))
(defn expand
"Expand parameters inside a *SQL* QUERY."
......
test/metabase/driver/generic_sql/native_test.clj
+ 2
− 2
View file @ 2591fb3d
......@@ -17,7 +17,7 @@
[99]]
:columns ["ID"]
:cols [(merge col-defaults {:name "ID", :display_name "ID", :base_type :type/Integer})]
:native_form {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2;", :params []}}}
:native_form {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2;"}}}
(-> (qp/process-query {:native {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2;"}
:type :native
:database (id)})
......@@ -34,7 +34,7 @@
[{:name "ID", :display_name "ID", :base_type :type/Integer}
{:name "NAME", :display_name "Name", :base_type :type/Text}
{:name "CATEGORY_ID", :display_name "Category ID", :base_type :type/Integer}])
:native_form {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2;", :params []}}}
:native_form {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2;"}}}
(-> (qp/process-query {:native {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2;"}
:type :native
:database (id)})
......
test/metabase/query_processor/middleware/parameters/sql_test.clj
+ 1
− 7
View file @ 2591fb3d
......@@ -43,6 +43,7 @@
(substitute "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}} AND bird_type = {{bird_type}}"
{:toucans_are_cool true}))
;;; ---------------------------------- optional substitution -- [[ ... {{x}} ... ]] ----------------------------------
(expect
......@@ -81,13 +82,6 @@
(substitute "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}}]]"
{:toucans_are_cool true, :bird_type "toucan"}))
(expect
{:query "SELECT * FROM bird_facts"
:params []}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment