Skip to content
Snippets Groups Projects
Unverified Commit 3f5168c6 authored by Cal Herries's avatar Cal Herries Committed by GitHub
Browse files

Don't search the text of native queries (#30779)

* Remove dataset_query from search

* More removal of dataset_query

* Fix test

* Remove test

* Fix kondo

* Fix kondo
parent 1c94a5ce
Branches
Tags
No related merge requests found
......@@ -93,9 +93,7 @@
:initial_sync_status :text
;; returned for Action
:model_id :integer
:model_name :text
;; returned for Card and Action
:dataset_query :text))
:model_name :text))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Shared Query Logic |
......@@ -191,26 +189,19 @@
(str "%" s "%"))
(defn- search-string-clause
[model query searchable-columns]
[query searchable-columns]
(when query
(into [:or]
(for [column searchable-columns
token (search-util/tokenize (search-util/normalize query))]
(if (and (= model "card") (= column (keyword (name (model->alias model)) "dataset_query")))
[:and
[:= (keyword (name (model->alias model)) "query_type") "native"]
[:like
[:lower column]
(wildcard-match token)]]
[:like
[:lower column]
(wildcard-match token)])))))
[:like
[:lower column]
(wildcard-match token)]))))
(s/defn ^:private base-where-clause-for-model :- [(s/one (s/enum :and := :inline) "type") s/Any]
[model :- SearchableModel, {:keys [search-string archived?]} :- SearchContext]
(let [archived-clause (archived-where-clause model archived?)
search-clause (search-string-clause model
search-string
search-clause (search-string-clause search-string
(map (let [model-alias (name (model->alias model))]
(fn [column]
(keyword model-alias (name column))))
......
(ns metabase.search.config
(:require
[cheshire.core :as json]
[metabase.models
:refer [Action Card Collection Dashboard Database Metric Segment Table]]
[metabase.models.setting :refer [defsetting]]
......@@ -75,7 +74,6 @@
(defmethod searchable-columns-for-model "card"
[_]
[:name
:dataset_query
:description])
(defmethod searchable-columns-for-model "dataset"
......@@ -135,12 +133,11 @@
[:model.collection_id :collection_id]
[:model.id :model_id]
[:model.name :model_name]
[:query_action.database_id :database_id]
[:query_action.dataset_query :dataset_query]))
[:query_action.database_id :database_id]))
(defmethod columns-for-model "card"
[_]
(conj default-columns :collection_id :collection_position :dataset_query
(conj default-columns :collection_id :collection_position
[:collection.name :collection_name]
[:collection.authority_level :collection_authority_level]
[{:select [:status]
......@@ -204,10 +201,3 @@
(defmethod column->string :default
[value _ _]
value)
(defmethod column->string [:card :dataset_query]
[value _ _]
(let [query (json/parse-string value true)]
(if (= "native" (:type query))
(-> query :native :query)
"")))
(ns metabase.search.scoring
(:require
[cheshire.core :as json]
[clojure.string :as str]
[java-time :as t]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.public-settings.premium-features :refer [defenterprise]]
[metabase.search.config :as search-config]
[metabase.search.util :as search-util]
......@@ -209,7 +207,6 @@
:name collection_name
:authority_level collection_authority_level}
:scores all-scores)
(update :dataset_query #(some-> % json/parse-string mbql.normalize/normalize))
(dissoc
:collection_id
:collection_name
......
......@@ -51,7 +51,6 @@
:context nil
:dashboardcard_count nil
:database_id false
:dataset_query nil
:description nil
:id true
:initial_sync_status nil
......@@ -100,10 +99,9 @@
(sorted-results
[(make-result "dashboard test dashboard", :model "dashboard", :bookmark false)
test-collection
(make-result "card test card", :model "card", :bookmark false, :dataset_query nil, :dashboardcard_count 0)
(make-result "dataset test dataset", :model "dataset", :bookmark false, :dataset_query nil, :dashboardcard_count 0)
(make-result "action test action", :model "action", :model_name (:name action-model-params), :model_id true,
:dataset_query (update (mt/query venues) :type name), :database_id true)
(make-result "card test card", :model "card", :bookmark false, :dashboardcard_count 0)
(make-result "dataset test dataset", :model "dataset", :bookmark false, :dashboardcard_count 0)
(make-result "action test action", :model "action", :model_name (:name action-model-params), :model_id true, :database_id true)
(merge
(make-result "metric test metric", :model "metric", :description "Lookin' for a blueberry")
(table-search-results))
......@@ -233,7 +231,6 @@
[:like [:lower :table_name] "%foo%"] [:inline 0]
[:like [:lower :table_description] "%foo%"] [:inline 0]
[:like [:lower :model_name] "%foo%"] [:inline 0]
[:like [:lower :dataset_query] "%foo%"] [:inline 0]
:else [:inline 1]]]
(api.search/order-clause "Foo")))))
......@@ -296,7 +293,7 @@
(def ^:private dashboard-count-results
(letfn [(make-card [dashboard-count]
(make-result (str "dashboard-count " dashboard-count) :dashboardcard_count dashboard-count,
:model "card", :bookmark false, :dataset_query nil))]
:model "card", :bookmark false))]
(set [(make-card 5)
(make-card 3)
(make-card 0)])))
......@@ -650,21 +647,6 @@
(t2/update! Pulse (:id pulse) {:dashboard_id (:id dashboard)})
(is (= nil (search-for-pulses pulse))))))))))
(deftest card-dataset-query-test
(testing "Search results should match a native query's dataset_query column, but not an MBQL query's one."
;; https://github.com/metabase/metabase/issues/24132
(let [native-card {:name "Another SQL query"
:query_type "native"
:dataset_query (mt/native-query {:query "SELECT COUNT(1) AS aggregation FROM venues"})}]
(mt/with-temp* [Card [_mbql-card {:name "Venues Count"
:query_type "query"
:dataset_query (mt/mbql-query venues {:aggregation [[:count]]})}]
Card [_native-card native-card]
Card [_dataset (assoc native-card :name "Dataset" :dataset true)]]
(is (= ["Another SQL query" "Dataset"]
(->> (search-request-data :rasta :q "aggregation")
(map :name))))))))
(deftest search-db-call-count-test
(t2.with-temp/with-temp
[Card _ {:name "card db count test 1"}
......
(ns metabase.search.scoring-test
(:require
[cheshire.core :as json]
[clojure.test :refer :all]
[java-time :as t]
[metabase.search.config :as search-config]
......@@ -273,20 +272,6 @@
{:weight 100 :score 0 :name "Some other score type"}])]
(is (= 0 (:score (scoring/score-and-result "" {:name "racing yo" :model "card"})))))))
(deftest ^:parallel serialize-test
(testing "It normalizes dataset queries from strings"
(let [query {:type :query
:query {:source-query {:source-table 1}}
:database 1}
result {:name "card"
:model "card"
:dataset_query (json/generate-string query)}]
(is (= query (-> result (#'scoring/serialize {} {}) :dataset_query)))))
(testing "Doesn't error on other models without a query"
(is (nil? (-> {:name "dash" :model "dashboard"}
(#'scoring/serialize {} {})
:dataset_query)))))
(deftest force-weight-test
(is (= [{:weight 10}]
(scoring/force-weight [{:weight 1}] 10)))
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment