From 628578400e0c590df73fdd25f8448ec5b3e27beb Mon Sep 17 00:00:00 2001
From: Case Nelson <case@metabase.com>
Date: Tue, 27 Jun 2023 17:04:20 -0600
Subject: [PATCH] [MLv2] Native query manipulation functions for raw query and
template tags (#31836)
* [MLv2] Native query manipulation functions for raw query and template tags
* Remove circular dependency
* Check extract, rather than recognize
* Copy schema from #31789, fix tests for schema
* Fix test
* Update src/metabase/lib/js.cljs
Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
* Update src/metabase/lib/native.cljc
Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
* Convert template-tag dimension field to mlv2
* Update src/metabase/lib/schema/template_tag.cljc
Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
* Include ::id/snippet
* Add assert for modifying native queries
* sort namespace
---------
Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
---
.../src/metabase-lib/queries/NativeQuery.ts | 2 +-
src/metabase/lib/convert.cljc | 4 +
src/metabase/lib/core.cljc | 9 +-
src/metabase/lib/js.cljs | 55 ++--
src/metabase/lib/native.cljc | 78 +++++-
src/metabase/lib/query.cljc | 21 --
src/metabase/lib/schema.cljc | 17 +-
src/metabase/lib/schema/id.cljc | 3 +
src/metabase/lib/schema/template_tag.cljc | 133 ++++++++++
src/metabase/metabot.clj | 2 +-
test/metabase/lib/convert_test.cljc | 37 ++-
test/metabase/lib/native_test.cljc | 244 +++++++++++-------
test/metabase/lib/query_test.cljc | 14 -
test/metabase/metabot/metabot_util_test.clj | 2 +-
14 files changed, 460 insertions(+), 161 deletions(-)
create mode 100644 src/metabase/lib/schema/template_tag.cljc
diff --git a/frontend/src/metabase-lib/queries/NativeQuery.ts b/frontend/src/metabase-lib/queries/NativeQuery.ts
index 7dacef1cf4a..cd6f701006f 100644
--- a/frontend/src/metabase-lib/queries/NativeQuery.ts
+++ b/frontend/src/metabase-lib/queries/NativeQuery.ts
@@ -437,7 +437,7 @@ export default class NativeQuery extends AtomicQuery {
*/
private _getUpdatedTemplateTags(queryText: string): TemplateTags {
return queryText && this.supportsNativeParameters()
- ? ML.template_tags(queryText, this.templateTagsMap())
+ ? ML.extract_template_tags(queryText, this.templateTagsMap())
: {};
}
diff --git a/src/metabase/lib/convert.cljc b/src/metabase/lib/convert.cljc
index 123e7aa733a..b81886a58b4 100644
--- a/src/metabase/lib/convert.cljc
+++ b/src/metabase/lib/convert.cljc
@@ -161,6 +161,10 @@
(cond-> stage
(:joins stage) (update :joins deduplicate-join-aliases))))))
+(defmethod ->pMBQL :mbql.stage/native
+ [stage]
+ (m/update-existing stage :template-tags update-vals (fn [tag] (m/update-existing tag :dimension ->pMBQL))))
+
(defmethod ->pMBQL :mbql/join
[join]
(let [join (-> join
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index 81f23c0d111..8120328a7ee 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -191,8 +191,12 @@
[lib.native
#?@(:cljs [->TemplateTags
TemplateTags->])
- recognize-template-tags
- template-tags]
+ native-query
+ raw-native-query
+ with-native-query
+ template-tags
+ with-template-tags
+ extract-template-tags]
[lib.order-by
change-direction
order-by
@@ -202,7 +206,6 @@
[lib.normalize
normalize]
[lib.query
- native-query
query
saved-question-query]
[lib.ref
diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs
index 54d92aac906..d24335dcedf 100644
--- a/src/metabase/lib/js.cljs
+++ b/src/metabase/lib/js.cljs
@@ -27,31 +27,24 @@
;; conversion for incoming args and outgoing return values. I'm imagining something like
;; `(mu/js-export lib.core/recognize-template-tags)` where that function has a Malli schema and it works like
;; `metabase.shared.util.namespaces/import-fn` plus wrapping it with conversion for all args and the return value.
-(defn ^:export recognize-template-tags
- "Given the text of a native query, extract a possibly-empty set of template tag strings from it.
-
- These looks like mustache templates. For variables, we only allow alphanumeric characters, eg. `{{foo}}`.
- For snippets they start with `snippet:`, eg. `{{ snippet: arbitrary text here }}`.
- And for card references either `{{ #123 }}` or with the optional human label `{{ #123-card-title-slug }}`.
-
- Invalid patterns are simply ignored, so something like `{{&foo!}}` is just disregarded."
- [query-text]
- (-> query-text
- lib.core/recognize-template-tags
- clj->js))
-
-(defn ^:export template-tags
+(defn ^:export extract-template-tags
"Extract the template tags from a native query's text.
If the optional map of existing tags previously parsed is given, this will reuse the existing tags where
they match up with the new one (in particular, it will preserve the UUIDs).
- See [[recognize-template-tags]] for how the tags are parsed."
- ([query-text] (template-tags query-text {}))
+ Given the text of a native query, extract a possibly-empty set of template tag strings from it.
+
+ These look like mustache templates. For variables, we only allow alphanumeric characters, eg. `{{foo}}`.
+ For snippets they start with `snippet:`, eg. `{{ snippet: arbitrary text here }}`.
+ And for card references either `{{ #123 }}` or with the optional human label `{{ #123-card-title-slug }}`.
+
+ Invalid patterns are simply ignored, so something like `{{&foo!}}` is just disregarded."
+ ([query-text] (extract-template-tags query-text {}))
([query-text existing-tags]
(->> existing-tags
lib.core/->TemplateTags
- (lib.core/template-tags query-text)
+ (lib.core/extract-template-tags query-text)
lib.core/TemplateTags->)))
(defn ^:export suggestedName
@@ -534,3 +527,31 @@
#js {:operator operator
:options (clj->js options)
:args (to-array args)}))
+
+(defn ^:export native-query
+ "Create a new native query.
+
+ Native in this sense means a pMBQL query with a first stage that is a native query."
+ [database-id metadata inner-query]
+ (lib.core/native-query (metadataProvider database-id metadata) inner-query))
+
+(defn ^:export with-native-query
+ "Update the raw native query, the first stage must already be a native type.
+ Replaces templates tags"
+ [a-query inner-query]
+ (lib.core/with-native-query a-query inner-query))
+
+(defn ^:export with-template-tags
+ "Updates the native query's template tags."
+ [a-query tags]
+ (lib.core/with-template-tags a-query (lib.core/->TemplateTags tags)))
+
+(defn ^:export raw-native-query
+ "Returns the native query string"
+ [a-query]
+ (lib.core/raw-native-query a-query))
+
+(defn ^:export template-tags
+ "Returns the native query's template tags"
+ [a-query]
+ (lib.core/TemplateTags-> (lib.core/template-tags a-query)))
diff --git a/src/metabase/lib/native.cljc b/src/metabase/lib/native.cljc
index dd7e486ea3c..ab7e7470ecf 100644
--- a/src/metabase/lib/native.cljc
+++ b/src/metabase/lib/native.cljc
@@ -1,13 +1,19 @@
(ns metabase.lib.native
"Functions for working with native queries."
(:require
+ #?@(:cljs ([metabase.domain-entities.converters :as converters]))
[clojure.set :as set]
[clojure.string :as str]
[medley.core :as m]
+ [metabase.lib.metadata :as lib.metadata]
+ [metabase.lib.options :as lib.options]
+ [metabase.lib.query :as lib.query]
+ [metabase.lib.schema :as lib.schema]
[metabase.lib.schema.common :as common]
+ [metabase.lib.util :as lib.util]
+ [metabase.shared.util.i18n :as i18n]
[metabase.util.humanization :as u.humanization]
- [metabase.util.malli :as mu]
- #?@(:cljs ([metabase.domain-entities.converters :as converters]))))
+ [metabase.util.malli :as mu]))
(def ^:private TemplateTag
[:map
@@ -35,7 +41,7 @@
(def ^:private tag-regexes
[variable-tag-regex snippet-tag-regex card-tag-regex])
-(mu/defn recognize-template-tags :- [:set ::common/non-blank-string]
+(mu/defn ^:private recognize-template-tags :- [:set ::common/non-blank-string]
"Given the text of a native query, extract a possibly-empty set of template tag strings from it."
[query-text :- ::common/non-blank-string]
(into #{}
@@ -95,15 +101,21 @@
(m/index-by :name (map fresh-tag new-tags))))]
(update-vals tags finish-tag)))
-(mu/defn ^:export template-tags :- TemplateTags
+(mu/defn extract-template-tags :- TemplateTags
"Extract the template tags from a native query's text.
If the optional map of existing tags previously parsed is given, this will reuse the existing tags where
they match up with the new one (in particular, it will preserve the UUIDs).
- See [[recognize-template-tags]] for how the tags are parsed."
+ Given the text of a native query, extract a possibly-empty set of template tag strings from it.
+
+ These looks like mustache templates. For variables, we only allow alphanumeric characters, eg. `{{foo}}`.
+ For snippets they start with `snippet:`, eg. `{{ snippet: arbitrary text here }}`.
+ And for card references either `{{ #123 }}` or with the optional human label `{{ #123-card-title-slug }}`.
+
+ Invalid patterns are simply ignored, so something like `{{&foo!}}` is just disregarded."
([query-text :- ::common/non-blank-string]
- (template-tags query-text nil))
+ (extract-template-tags query-text nil))
([query-text :- ::common/non-blank-string
existing-tags :- [:maybe TemplateTags]]
(let [query-tag-names (not-empty (recognize-template-tags query-text))
@@ -123,3 +135,57 @@
(def TemplateTags->
"Converter from a map of `TemplateTag`s keyed by their string names to vanilla JS."
(converters/outgoing TemplateTags))))
+
+(mu/defn native-query :- ::lib.schema/query
+ "Create a new native query.
+
+ Native in this sense means a pMBQL query with a first stage that is a native query."
+ ([metadata-providerable :- lib.metadata/MetadataProviderable
+ inner-query :- ::common/non-blank-string]
+ (native-query metadata-providerable nil inner-query))
+
+ ([metadata-providerable :- lib.metadata/MetadataProviderable
+ results-metadata :- [:maybe lib.metadata/StageMetadata]
+ inner-query :- ::common/non-blank-string]
+ (let [tags (extract-template-tags inner-query)]
+ (lib.query/query-with-stages metadata-providerable
+ [(-> {:lib/type :mbql.stage/native
+ :lib/stage-metadata results-metadata
+ :template-tags tags
+ :native inner-query}
+ lib.options/ensure-uuid)]))))
+
+(mu/defn with-native-query :- ::lib.schema/query
+ "Update the raw native query, the first stage must already be a native type.
+ Replaces templates tags"
+ [query :- ::lib.schema/query
+ inner-query :- ::common/non-blank-string]
+ (lib.util/update-query-stage
+ query 0
+ (fn [{existing-tags :template-tags stage-type :lib/type :as stage}]
+ (assert (= stage-type :mbql.stage/native) (i18n/tru "Must be a native query"))
+ (assoc stage
+ :native inner-query
+ :template-tags (extract-template-tags inner-query existing-tags)))))
+
+(mu/defn with-template-tags :- ::lib.schema/query
+ "Updates the native query's template tags."
+ [query :- ::lib.schema/query
+ tags :- TemplateTags]
+ (lib.util/update-query-stage
+ query 0
+ (fn [{existing-tags :template-tags stage-type :lib/type :as stage}]
+ (assert (= stage-type :mbql.stage/native) (i18n/tru "Must be a native query"))
+ (let [valid-tags (keys existing-tags)]
+ (assoc stage :template-tags
+ (m/deep-merge existing-tags (select-keys tags valid-tags)))))))
+
+(mu/defn raw-native-query :- ::common/non-blank-string
+ "Returns the native query string"
+ [query :- ::lib.schema/query]
+ (:native (lib.util/query-stage query 0)))
+
+(mu/defn template-tags :- TemplateTags
+ "Returns the native query's template tags"
+ [query :- ::lib.schema/query]
+ (:template-tags (lib.util/query-stage query 0)))
diff --git a/src/metabase/lib/query.cljc b/src/metabase/lib/query.cljc
index e3cd672105f..dbb15a1bc3c 100644
--- a/src/metabase/lib/query.cljc
+++ b/src/metabase/lib/query.cljc
@@ -7,7 +7,6 @@
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.normalize :as lib.normalize]
- [metabase.lib.options :as lib.options]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.id :as lib.schema.id]
[metabase.lib.util :as lib.util]
@@ -91,26 +90,6 @@
x]
(->query metadata-providerable x))
-;;; TODO -- the stuff below will probably change in the near future, please don't read too much in to it.
-(mu/defn native-query :- ::lib.schema/query
- "Create a new native query.
-
- Native in this sense means a pMBQL query with a first stage that is a native query."
- ([metadata-providerable :- lib.metadata/MetadataProviderable
- inner-query]
- (native-query metadata-providerable nil inner-query))
-
- ;; TODO not sure if `results-metadata` should be StageMetadata (i.e., a map roughly matching the shape you get from
- ;; the QP) or a sequence of ColumnMetadatas, like what would be saved in Card `result_metadata`.
- ([metadata-providerable :- lib.metadata/MetadataProviderable
- results-metadata :- lib.metadata/StageMetadata
- inner-query]
- (query-with-stages metadata-providerable
- [(-> {:lib/type :mbql.stage/native
- :lib/stage-metadata results-metadata
- :native inner-query}
- lib.options/ensure-uuid)])))
-
(mu/defn saved-question-query :- ::lib.schema/query
"Convenience for creating a query from a Saved Question (i.e., a Card)."
[metadata-providerable :- lib.metadata/MetadataProviderable
diff --git a/src/metabase/lib/schema.cljc b/src/metabase/lib/schema.cljc
index 4cf08e64c57..70743606c7f 100644
--- a/src/metabase/lib/schema.cljc
+++ b/src/metabase/lib/schema.cljc
@@ -8,6 +8,7 @@
future we can deprecate that namespace and eventually do away with it entirely."
(:require
[metabase.lib.schema.aggregation :as aggregation]
+ [metabase.lib.schema.common :as common]
[metabase.lib.schema.expression :as expression]
[metabase.lib.schema.expression.arithmetic]
[metabase.lib.schema.expression.conditional]
@@ -19,6 +20,7 @@
[metabase.lib.schema.literal]
[metabase.lib.schema.order-by :as order-by]
[metabase.lib.schema.ref :as ref]
+ [metabase.lib.schema.template-tag :as template-tag]
[metabase.lib.schema.util :as lib.schema.util]
[metabase.mbql.util.match :as mbql.match]
[metabase.util.malli.registry :as mr]))
@@ -33,8 +35,19 @@
(mr/def ::stage.native
[:map
[:lib/type [:= :mbql.stage/native]]
- [:native any?]
- [:args {:optional true} [:sequential any?]]])
+ ;; the actual native query, depends on the underlying database. Could be a raw SQL string or something like that.
+ ;; Only restriction is that it is non-nil.
+ [:native some?]
+ ;; any parameters that should be passed in along with the query to the underlying query engine, e.g. for JDBC these
+ ;; are the parameters we pass in for a `PreparedStatement` for `?` placeholders. These can be anything, including
+ ;; nil.
+ [:args {:optional true} [:sequential any?]]
+ ;; the Table/Collection/etc. that this query should be executed against; currently only used for MongoDB, where it
+ ;; is required.
+ [:collection {:optional true} ::common/non-blank-string]
+ ;; optional template tag declarations. Template tags are things like `{{x}}` in the query (the value of the
+ ;; `:native` key), but their definition lives under this key.
+ [:template-tags {:optional true} [:ref ::template-tag/template-tag-map]]])
(mr/def ::breakouts
[:sequential {:min 1} [:ref ::ref/ref]])
diff --git a/src/metabase/lib/schema/id.cljc b/src/metabase/lib/schema/id.cljc
index a0cd7fd2676..0f88c06bb67 100644
--- a/src/metabase/lib/schema/id.cljc
+++ b/src/metabase/lib/schema/id.cljc
@@ -29,3 +29,6 @@
(mr/def ::metric
::common/int-greater-than-or-equal-to-zero)
+
+(mr/def ::snippet
+ ::common/int-greater-than-or-equal-to-zero)
diff --git a/src/metabase/lib/schema/template_tag.cljc b/src/metabase/lib/schema/template_tag.cljc
new file mode 100644
index 00000000000..3e434d11326
--- /dev/null
+++ b/src/metabase/lib/schema/template_tag.cljc
@@ -0,0 +1,133 @@
+(ns metabase.lib.schema.template-tag
+ (:require
+ [malli.core :as mc]
+ [metabase.lib.schema.common :as common]
+ [metabase.lib.schema.id :as id]
+ [metabase.mbql.schema :as mbql.s]
+ [metabase.util.malli.registry :as mr]))
+
+;; Schema for valid values of `:widget-type` for a [[TemplateTag:FieldFilter]].
+(mr/def ::widget-type
+ (into
+ [:enum
+ ;; this will be a nicer error message than Malli trying to list every single possible allowed type.
+ {:error/message "Valid template tag :widget-type"}
+ :none]
+ (keys mbql.s/parameter-types)))
+
+;; Schema for valid values of template tag `:type`.
+(mr/def ::type
+ [:enum :snippet :card :dimension :number :text :date])
+
+;;; Things required by all template tag types.
+(mr/def ::common
+ [:map
+ [:name ::common/non-blank-string]
+ [:display-name ::common/non-blank-string]
+ ;; TODO -- `:id` is actually 100% required but we have a lot of tests that don't specify it because this constraint
+ ;; wasn't previously enforced; we need to go in and fix those tests and make this non-optional
+ [:id {:optional true} [:or ::common/non-blank-string :uuid]]])
+
+;;; Stuff shared between the Field filter and raw value template tag schemas.
+(mr/def ::value.common
+ [:merge
+ [:ref ::common]
+ [:map
+ ;; default value for this parameter
+ [:default {:optional true} any?]
+ ;; whether or not a value for this parameter is required in order to run the query
+ [:required {:optional true} :boolean]]])
+
+;; Example:
+;;
+;; {:id "c20851c7-8a80-0ffa-8a99-ae636f0e9539"
+;; :name "date"
+;; :display-name "Date"
+;; :type :dimension,
+;; :dimension [:field 4 nil]
+;; :widget-type :date/all-options}
+(mr/def ::field-filter
+ [:merge
+ [:ref ::value.common]
+ [:map
+ [:type [:= :dimension]]
+ [:dimension [:ref :mbql.clause/field]]
+ ;; which type of widget the frontend should show for this Field Filter; this also affects which parameter types
+ ;; are allowed to be specified for it.
+ [:widget-type [:ref ::widget-type]]
+ ;; optional map to be appended to filter clause
+ [:options {:optional true} :map]]])
+
+;; Example:
+;;
+;; {:id "c2fc7310-44eb-4f21-c3a0-63806ffb7ddd"
+;; :name "snippet: select"
+;; :display-name "Snippet: select"
+;; :type :snippet
+;; :snippet-name "select"
+;; :snippet-id 1}
+(mr/def ::snippet
+ [:merge
+ [:ref ::common]
+ [:map
+ [:type [:= :snippet]]
+ [:snippet-name ::common/non-blank-string]
+ [:snippet-id ::id/snippet]
+ ;; database to which this Snippet belongs. Doesn't always seem to be specified.
+ [:database {:optional true} ::id/database]]])
+
+;; Example:
+;;
+;; {:id "fc5e14d9-7d14-67af-66b2-b2a6e25afeaf"
+;; :name "#1635"
+;; :display-name "#1635"
+;; :type :card
+;; :card-id 1635}
+(mr/def ::source-query
+ [:merge
+ [:ref ::common]
+ [:map
+ [:type [:= :card]]
+ [:card-id ::id/card]]])
+
+;; Valid values of `:type` for raw value template tags.
+(mr/def ::raw-value.type
+ (into [:enum] mbql.s/raw-value-template-tag-types))
+
+;; Example:
+;;
+;; {:id "35f1ecd4-d622-6d14-54be-750c498043cb"
+;; :name "id"
+;; :display-name "Id"
+;; :type :number
+;; :required true
+;; :default "1"}
+(mr/def ::raw-value
+ [:merge
+ [:ref ::value.common]
+ ;; `:type` is used be the FE to determine which type of widget to display for the template tag, and to determine
+ ;; which types of parameters are allowed to be passed in for this template tag.
+ [:map
+ [:type [:ref ::raw-value.type]]]])
+
+(mr/def ::template-tag
+ [:and
+ [:map
+ [:type [:ref ::type]]]
+ [:multi {:dispatch :type}
+ [:dimension [:ref ::field-filter]]
+ [:snippet [:ref ::snippet]]
+ [:card [:ref ::source-query]]
+ ;; :number, :text, :date
+ [::mc/default [:ref ::raw-value]]]])
+
+(mr/def ::template-tag-map
+ [:and
+ [:map-of ::common/non-blank-string [:ref ::template-tag]]
+ ;; make sure people don't try to pass in a `:name` that's different from the actual key in the map.
+ [:fn
+ {:error/message "keys in template tag map must match the :name of their values"}
+ (fn [m]
+ (every? (fn [[tag-name tag-definition]]
+ (= tag-name (:name tag-definition)))
+ m))]])
diff --git a/src/metabase/metabot.clj b/src/metabase/metabot.clj
index f71b5b0be33..e144d4f5457 100644
--- a/src/metabase/metabot.clj
+++ b/src/metabase/metabot.clj
@@ -45,7 +45,7 @@
(:id model)
user_prompt
final-sql)
- template-tags (lib-native/template-tags inner_query)
+ template-tags (lib-native/extract-template-tags inner_query)
dataset {:dataset_query {:database database_id
:type "native"
:native {:query final-sql
diff --git a/test/metabase/lib/convert_test.cljc b/test/metabase/lib/convert_test.cljc
index 15f726b1e02..a47da2d7d34 100644
--- a/test/metabase/lib/convert_test.cljc
+++ b/test/metabase/lib/convert_test.cljc
@@ -1,10 +1,11 @@
(ns metabase.lib.convert-test
(:require
+ #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))
[clojure.test :refer [are deftest is testing]]
+ [malli.core :as mc]
[metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
- [metabase.lib.test-metadata :as meta]
- #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
+ [metabase.lib.test-metadata :as meta]))
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
@@ -88,6 +89,25 @@
:strategy :left-join
:fk-field-id (meta/id :venues :category-id)}]}}))))
+(deftest ^:parallel ->pMBQL-native-query-test
+ (testing "template tag dimensions are converted"
+ (let [original {:type :native,
+ :native
+ {:query "SELECT count(*) AS count FROM PUBLIC.PEOPLE WHERE true [[AND {{NAME}}]]",
+ :template-tags
+ {"NAME"
+ {:name "NAME",
+ :display-name "Name",
+ :type :dimension,
+ :dimension [:field 866 nil],
+ :widget-type :string/=,
+ :default nil}}},
+ :database 76}
+ converted (lib.convert/->pMBQL original)]
+ (is (=? {:stages [{:template-tags {"NAME" {:dimension [:field {:lib/uuid string?} 866]}}}]}
+ converted))
+ (is (mc/validate :metabase.lib.schema/query converted)))))
+
(deftest ^:parallel ->pMBQL-joins-default-alias-test
(let [original {:database (meta/id)
:type :query
@@ -332,6 +352,19 @@
:value [:param-value]}],
:type :native}
+ {:type :native,
+ :native
+ {:query "SELECT count(*) AS count FROM PUBLIC.PEOPLE WHERE true [[AND {{NAME}}]]",
+ :template-tags
+ {"NAME"
+ {:name "NAME",
+ :display-name "Name",
+ :type :dimension,
+ :dimension [:field 866 nil],
+ :widget-type :string/=,
+ :default nil}}},
+ :database 76}
+
{:database 1
:type :query
:query {:source-table 224
diff --git a/test/metabase/lib/native_test.cljc b/test/metabase/lib/native_test.cljc
index f9da568fd8a..9b1ba482750 100644
--- a/test/metabase/lib/native_test.cljc
+++ b/test/metabase/lib/native_test.cljc
@@ -1,13 +1,17 @@
(ns metabase.lib.native-test
(:require
+ #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]
+ [metabase.test.util.js :as test.js]))
[clojure.test :refer [are deftest is testing]]
+ [medley.core :as m]
+ [metabase.lib.core :as lib]
+ [metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.native :as lib.native]
- #?@(:clj ([medley.core :as m]
- [metabase.util.humanization :as u.humanization])
- :cljs ([metabase.test.util.js :as test.js]))))
+ [metabase.lib.test-metadata :as meta]
+ [metabase.util.humanization :as u.humanization]))
(deftest ^:parallel variable-tag-test
- (are [exp input] (= exp (lib.native/recognize-template-tags input))
+ (are [exp input] (= exp (set (keys (lib.native/extract-template-tags input))))
#{"foo"} "SELECT * FROM table WHERE {{foo}} AND some_field IS NOT NULL"
#{"foo" "bar"} "SELECT * FROM table WHERE {{foo}} AND some_field = {{bar}}"
;; Duplicates are flattened.
@@ -16,7 +20,7 @@
#{} "SELECT * FROM table WHERE {{&foo}}"))
(deftest ^:parallel snippet-tag-test
- (are [exp input] (= exp (lib.native/recognize-template-tags input))
+ (are [exp input] (= exp (set (keys (lib.native/extract-template-tags input))))
#{"snippet: foo "} "SELECT * FROM table WHERE {{snippet: foo }} AND some_field IS NOT NULL"
#{"snippet: foo *#&@"} "SELECT * FROM table WHERE {{snippet: foo *#&@}}"
;; TODO: This logic should trim the whitespace and unify these two snippet names.
@@ -24,7 +28,7 @@
#{"snippet: foo" "snippet:foo"} "SELECT * FROM table WHERE {{snippet: foo}} AND {{snippet:foo}}"))
(deftest ^:parallel card-tag-test
- (are [exp input] (= exp (lib.native/recognize-template-tags input))
+ (are [exp input] (= exp (set (keys (lib.native/extract-template-tags input))))
#{"#123"} "SELECT * FROM table WHERE {{ #123 }} AND some_field IS NOT NULL"
;; TODO: This logic should trim the whitespace and unify these two card tags.
;; I think this is a bug in the original code but am aiming to reproduce it exactly for now.
@@ -32,97 +36,94 @@
#{"#123"} "SELECT * FROM table WHERE {{ #not-this }} AND {{#123}}"
#{} "{{ #123foo }}"))
-#?(:clj
- ;; TODO: This is only CLJ-only because =? from Hawk is not available in CLJS currently.
- ;; I don't think there's any reason why it can't work there too.
- (deftest ^:parallel template-tags-test
- (testing "snippet tags"
- (is (=? {"snippet:foo" {:type :snippet
- :name "snippet:foo"
- :snippet-name "foo"
- :id uuid?}}
- (lib.native/template-tags "SELECT * FROM table WHERE {{snippet:foo}}")))
- (is (=? {"snippet:foo" {:type :snippet
- :name "snippet:foo"
- :snippet-name "foo"
- :id uuid?}
- "snippet: foo" {:type :snippet
- :name "snippet: foo"
- :snippet-name "foo"
- :id uuid?}}
- ;; TODO: This should probably be considered a bug - whitespace matters for the name.
- (lib.native/template-tags "SELECT * FROM {{snippet: foo}} WHERE {{snippet:foo}}"))))
+(deftest ^:parallel template-tags-test
+ (testing "snippet tags"
+ (is (=? {"snippet:foo" {:type :snippet
+ :name "snippet:foo"
+ :snippet-name "foo"
+ :id uuid?}}
+ (lib.native/extract-template-tags "SELECT * FROM table WHERE {{snippet:foo}}")))
+ (is (=? {"snippet:foo" {:type :snippet
+ :name "snippet:foo"
+ :snippet-name "foo"
+ :id uuid?}
+ "snippet: foo" {:type :snippet
+ :name "snippet: foo"
+ :snippet-name "foo"
+ :id uuid?}}
+ ;; TODO: This should probably be considered a bug - whitespace matters for the name.
+ (lib.native/extract-template-tags "SELECT * FROM {{snippet: foo}} WHERE {{snippet:foo}}"))))
- (testing "renaming a variable"
- (let [old-tag {:type :text
- :name "foo"
- :display-name "Foo"
- :id (m/random-uuid)}]
- (testing "changes display-name if the original is not customized"
- (is (=? {"bar" {:type :text
- :name "bar"
- :display-name "Bar"
- :id (:id old-tag)}}
- (lib.native/template-tags "SELECT * FROM {{bar}}"
- {"foo" old-tag}))))
- (testing "keeps display-name if it's customized"
- (is (=? {"bar" {:type :text
- :name "bar"
- :display-name "Custom Name"
- :id (:id old-tag)}}
- (lib.native/template-tags "SELECT * FROM {{bar}}"
- {"foo" (assoc old-tag :display-name "Custom Name")}))))
+ (testing "renaming a variable"
+ (let [old-tag {:type :text
+ :name "foo"
+ :display-name "Foo"
+ :id (m/random-uuid)}]
+ (testing "changes display-name if the original is not customized"
+ (is (=? {"bar" {:type :text
+ :name "bar"
+ :display-name "Bar"
+ :id (:id old-tag)}}
+ (lib.native/extract-template-tags "SELECT * FROM {{bar}}"
+ {"foo" old-tag}))))
+ (testing "keeps display-name if it's customized"
+ (is (=? {"bar" {:type :text
+ :name "bar"
+ :display-name "Custom Name"
+ :id (:id old-tag)}}
+ (lib.native/extract-template-tags "SELECT * FROM {{bar}}"
+ {"foo" (assoc old-tag :display-name "Custom Name")}))))
- (testing "works with other variables present, if they don't change"
- (let [other {:type :text
- :name "other"
- :display-name "Some Var"
- :id (m/random-uuid)}]
- (is (=? {"other" other
- "bar" {:type :text
- :name "bar"
- :display-name "Bar"
- :id (:id old-tag)}}
- (lib.native/template-tags "SELECT * FROM {{bar}} AND field = {{other}}"
- {"foo" old-tag
- "other" other})))))))
+ (testing "works with other variables present, if they don't change"
+ (let [other {:type :text
+ :name "other"
+ :display-name "Some Var"
+ :id (m/random-uuid)}]
+ (is (=? {"other" other
+ "bar" {:type :text
+ :name "bar"
+ :display-name "Bar"
+ :id (:id old-tag)}}
+ (lib.native/extract-template-tags "SELECT * FROM {{bar}} AND field = {{other}}"
+ {"foo" old-tag
+ "other" other})))))))
- (testing "general case, add and remove"
- (let [mktag (fn [base]
- (merge {:type :text
- :display-name (u.humanization/name->human-readable-name :simple (:name base))
- :id uuid?}
- base))
- v1 (mktag {:name "foo"})
- v2 (mktag {:name "bar"})
- v3 (mktag {:name "baz"})
- s1 (mktag {:name "snippet:first snippet"
- :snippet-name "first snippet"
- :type :snippet})
- s2 (mktag {:name "snippet:another snippet"
- :snippet-name "another snippet"
- :type :snippet})
+ (testing "general case, add and remove"
+ (let [mktag (fn [base]
+ (merge {:type :text
+ :display-name (u.humanization/name->human-readable-name :simple (:name base))
+ :id uuid?}
+ base))
+ v1 (mktag {:name "foo"})
+ v2 (mktag {:name "bar"})
+ v3 (mktag {:name "baz"})
+ s1 (mktag {:name "snippet:first snippet"
+ :snippet-name "first snippet"
+ :type :snippet})
+ s2 (mktag {:name "snippet:another snippet"
+ :snippet-name "another snippet"
+ :type :snippet})
- c1 (mktag {:name "#123-card-1"
- :type :card
- :card-id 123})
- c2 (mktag {:name "#321"
- :type :card
- :card-id 321})]
- (is (=? {"foo" v1
- "#123-card-1" c1
- "snippet:first snippet" s1}
- (lib.native/template-tags
- "SELECT * FROM {{#123-card-1}} WHERE {{foo}} AND {{ snippet:first snippet}}")))
- (is (=? {"bar" v2
- "baz" v3
- "snippet:another snippet" s2
- "#321" c2}
- (lib.native/template-tags
- "SELECT * FROM {{#321}} WHERE {{baz}} AND {{bar}} AND {{snippet:another snippet}}"
- {"foo" (assoc v1 :id (random-uuid))
- "#123-card-1" (assoc c1 :id (random-uuid))
- "snippet:first snippet" (assoc s1 :id (random-uuid))})))))))
+ c1 (mktag {:name "#123-card-1"
+ :type :card
+ :card-id 123})
+ c2 (mktag {:name "#321"
+ :type :card
+ :card-id 321})]
+ (is (=? {"foo" v1
+ "#123-card-1" c1
+ "snippet:first snippet" s1}
+ (lib.native/extract-template-tags
+ "SELECT * FROM {{#123-card-1}} WHERE {{foo}} AND {{ snippet:first snippet}}")))
+ (is (=? {"bar" v2
+ "baz" v3
+ "snippet:another snippet" s2
+ "#321" c2}
+ (lib.native/extract-template-tags
+ "SELECT * FROM {{#321}} WHERE {{baz}} AND {{bar}} AND {{snippet:another snippet}}"
+ {"foo" (assoc v1 :id (random-uuid))
+ "#123-card-1" (assoc c1 :id (random-uuid))
+ "snippet:first snippet" (assoc s1 :id (random-uuid))}))))))
#?(:cljs
(deftest converters-test
@@ -161,3 +162,60 @@
(testing "round trips work"
(is (= clj-tags (-> clj-tags (#'lib.native/TemplateTags->) (#'lib.native/->TemplateTags))))
(is (test.js/= js-tags (-> js-tags (#'lib.native/->TemplateTags) (#'lib.native/TemplateTags->))))))))
+
+(deftest ^:parallel native-query-test
+ (is (=? {:lib/type :mbql/query
+ :database (meta/id)
+ :stages [{:lib/type :mbql.stage/native
+ :lib/options {:lib/uuid string?}
+ :native "SELECT * FROM VENUES;"}]}
+ (lib/native-query meta/metadata-provider meta/qp-results-metadata "SELECT * FROM VENUES;"))))
+
+(deftest ^:parallel native-query-suggested-name-test
+ (let [query (lib/native-query meta/metadata-provider meta/qp-results-metadata "SELECT * FROM VENUES;")]
+ (is (= "Native query"
+ (lib.metadata.calculation/describe-query query)))
+ (is (nil? (lib.metadata.calculation/suggested-name query)))))
+
+(deftest ^:parallel native-query-building
+ (let [query (lib/native-query meta/metadata-provider "select * from venues where id = {{myid}}")]
+ (testing "Updating query keeps template tags in sync"
+ (is (=? ["select * from venues where id = {{myid}}"
+ {"myid" {:type :text,
+ :name "myid",
+ :id uuid?
+ :display-name "Myid"}}]
+ ((juxt lib/raw-native-query lib/template-tags) query)))
+ (is (=? ["select * from venues where id = {{myid}} and x = {{y}}"
+ {"myid" {} "y" {}}]
+ (-> query
+ (lib/with-native-query "select * from venues where id = {{myid}} and x = {{y}}")
+ ((juxt lib/raw-native-query lib/template-tags)))))
+ (is (=? ["select * from venues where id = {{myrenamedid}}"
+ {"myrenamedid" {}}]
+ (-> query
+ (lib/with-native-query "select * from venues where id = {{myrenamedid}}")
+ ((juxt lib/raw-native-query lib/template-tags)))))
+ (is (empty?
+ (-> query
+ (lib/with-native-query "select * from venues")
+ lib/template-tags))))))
+
+(deftest ^:parallel with-template-tags-test
+ (let [query (lib/native-query meta/metadata-provider "select * from venues where id = {{myid}}")
+ original-tags (lib/template-tags query)]
+ (is (= (assoc-in original-tags ["myid" :display-name] "My ID")
+ (-> query
+ (lib/with-template-tags {"myid" (assoc (get original-tags "myid") :display-name "My ID")})
+ lib/template-tags)))
+ (testing "Changing query keeps updated template tags"
+ (is (= (assoc-in original-tags ["myid" :display-name] "My ID")
+ (-> query
+ (lib/with-template-tags {"myid" (assoc (get original-tags "myid") :display-name "My ID")})
+ (lib/with-native-query "select * from venues where category_id = {{myid}}")
+ lib/template-tags))))
+ (testing "Doesn't introduce garbage"
+ (is (= original-tags
+ (-> query
+ (lib/with-template-tags {"garbage" (assoc (get original-tags "myid") :display-name "Foobar")})
+ lib/template-tags))))))
diff --git a/test/metabase/lib/query_test.cljc b/test/metabase/lib/query_test.cljc
index c06dee255c5..e866fdd5d79 100644
--- a/test/metabase/lib/query_test.cljc
+++ b/test/metabase/lib/query_test.cljc
@@ -29,20 +29,6 @@
(lib.metadata.calculation/describe-query query)
(lib.metadata.calculation/suggested-name query)))))
-(deftest ^:parallel native-query-test
- (is (=? {:lib/type :mbql/query
- :database (meta/id)
- :stages [{:lib/type :mbql.stage/native
- :lib/options {:lib/uuid string?}
- :native "SELECT * FROM VENUES;"}]}
- (lib/native-query meta/metadata-provider meta/qp-results-metadata "SELECT * FROM VENUES;"))))
-
-(deftest ^:parallel native-query-suggested-name-test
- (let [query (lib/native-query meta/metadata-provider meta/qp-results-metadata "SELECT * FROM VENUES;")]
- (is (= "Native query"
- (lib.metadata.calculation/describe-query query)))
- (is (nil? (lib.metadata.calculation/suggested-name query)))))
-
(deftest ^:parallel card-source-query-test
(is (=? {:lib/type :mbql/query
:database (meta/id)
diff --git a/test/metabase/metabot/metabot_util_test.clj b/test/metabase/metabot/metabot_util_test.clj
index cac23fc4536..d75d9da7700 100644
--- a/test/metabase/metabot/metabot_util_test.clj
+++ b/test/metabase/metabot/metabot_util_test.clj
@@ -180,7 +180,7 @@
:type "native"
:native {:query sql
:template-tags (update-vals
- (lib-native/template-tags inner_query)
+ (lib-native/extract-template-tags inner_query)
(fn [m] (update m :id str)))}})]
(is (some? (seq (get-in results [:data :rows]))))))))))
--
GitLab