Skip to content
Snippets Groups Projects
Unverified Commit 38269668 authored by Pawit Pornkitprasan's avatar Pawit Pornkitprasan Committed by GitHub
Browse files

Fix slow query when using nested native question with many rows (#18023)

`query->native` works by having `nativef` throws an exception
which causes `mbql->native` to set native query to `nil` which
then cause the rest of the execution to be skipped.

However, because the existing code skips overriding the native
query for queries already native, the execution is not skipped.

This causes bad performance for nested native question because
`query->native` is called for the sub-query which causes the
sub-query to be executed on its own, potentially without any
limit or filtering.
parent deeebec9
No related branches found
No related tags found
No related merge requests found
......@@ -17,15 +17,16 @@
"Middleware that handles conversion of MBQL queries to native (by calling driver QP methods) so the queries
can be executed. For queries that are already native, this function is effectively a no-op."
[qp]
(fn [{query-type :type, :as query} rff context]
(fn [query rff context]
(let [query (context/preprocessedf query context)
native-query (context/nativef (query->native-form query) context)]
(log/trace (u/format-color 'yellow "\nPreprocessed:\n%s" (u/pprint-to-str query)))
(log/trace (u/format-color 'green "Native form: \n%s" (u/pprint-to-str native-query)))
(qp
(cond-> query
(= query-type :query)
(assoc :native native-query))
;; For queries already native, this is normally no-op except during `qp/query->native`
;; where `:native` will be set to `nil` (result from `context/nativef`) to prevent
;; execution in the next step.
(assoc query :native native-query)
(fn [metadata]
(rff (assoc metadata :native_form native-query)))
context))))
......@@ -5,6 +5,7 @@
[metabase.models.permissions :as perms]
[metabase.query-processor :as qp]
[metabase.test :as mt]
[metabase.util :as u]
[schema.core :as s]))
(deftest query->native-test
......@@ -29,7 +30,16 @@
:type :native
:native {:query "SELECT * FROM VENUES [[WHERE price = {{price}}]];"
:template-tags {"price" {:name "price", :display-name "Price", :type :number, :required false}}}
:parameters [{:type "category", :target [:variable [:template-tag "price"]], :value "3"}]})))))
:parameters [{:type "category", :target [:variable [:template-tag "price"]], :value "3"}]}))))
(testing "If query is already native, `query->native` should not execute the query (metabase#13572)"
;; 1000,000,000 rows, no way this will finish in 2 seconds if executed
(let [long-query "SELECT CHECKINS.* FROM CHECKINS LEFT JOIN CHECKINS C2 ON 1=1 LEFT JOIN CHECKINS C3 ON 1=1"]
(u/with-timeout 2000
(is (= {:query long-query}
(qp/query->native
{:database (mt/id)
:type :native
:native {:query long-query}})))))))
;; If user permissions are bound, we should do permissions checking when you call `query->native`; you should need
;; native query execution permissions for the DB in question plus the perms needed for the original query in order to
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment