Skip to content
Snippets Groups Projects
Unverified Commit e587a20c authored by metamben's avatar metamben Committed by GitHub
Browse files

Keep filter and fields clauses in trivial nested queries (#28590)

parent eab2af99
No related branches found
No related tags found
No related merge requests found
Expand all Hide whitespace changes
Inline Side-by-side
modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
+ 1
− 8
View file @ e587a20c
......@@ -217,14 +217,7 @@
(mt/dataset sample-dataset
(is (= '{:select [source.CATEGORY_2 AS CATEGORY_2
COUNT (*) AS count]
:from [{:select [products.id AS id
products.ean AS ean
products.title AS title
products.category AS category
products.vendor AS vendor
products.price AS price
products.rating AS rating
products.created_at AS created_at
:from [{:select [products.category AS category
products.category || ? AS CATEGORY_2]
:from [products]}
AS source]
......
src/metabase/query_processor/util/nest_query.clj
+ 18
− 3
View file @ e587a20c
......@@ -32,9 +32,20 @@
(seq joined-fields) (update :fields (fn [fields]
(m/distinct-by add/normalize-clause (concat fields joined-fields))))))
(defn- keep-source+alias-props [field]
(update field 2 select-keys [::add/source-alias ::add/source-table :join-alias]))
(defn- remove-unused-fields [inner-query source]
(let [used-fields (-> #{}
(into (map keep-source+alias-props) (mbql.u/match inner-query :field))
(into (map keep-source+alias-props) (mbql.u/match inner-query :expression)))]
(update source :fields #(filterv (comp used-fields keep-source+alias-props) %))))
(defn- nest-source [inner-query]
(classloader/require 'metabase.query-processor)
(let [source (as-> (select-keys inner-query [:source-table :source-query :source-metadata :joins :expressions]) source
(let [filter-clause (:filter inner-query)
keep-filter? (nil? (mbql.u/match-one filter-clause :expression))
source (as-> (select-keys inner-query [:source-table :source-query :source-metadata :joins :expressions]) source
;; preprocess this without a current user context so it's not subject to permissions checks. To get
;; here in the first place we already had to do perms checks to make sure the query we're transforming
;; is itself ok, so we don't need to run another check
......@@ -45,10 +56,14 @@
(add/add-alias-info source)
(:query source)
(dissoc source :limit)
(add-joined-fields-to-fields (joined-fields inner-query) source))]
(add-joined-fields-to-fields (joined-fields inner-query) source)
(remove-unused-fields inner-query source)
(cond-> source
keep-filter? (assoc :filter filter-clause)))]
(-> inner-query
(dissoc :source-table :source-metadata :joins)
(assoc :source-query source))))
(assoc :source-query source)
(cond-> keep-filter? (dissoc :filter)))))
(defn- raise-source-query-expression-ref
"Convert an `:expression` reference from a source query into an appropriate `:field` clause for use in the surrounding
......
test/metabase/driver/sql/query_processor_test.clj
+ 13
− 47
View file @ e587a20c
......@@ -433,15 +433,9 @@
DATE_TRUNC ("year" source.CREATED_AT) AS CREATED_AT
source.pivot-grouping AS pivot-grouping
COUNT (*) AS count]
:from [{:select [ORDERS.ID AS ID
ORDERS.USER_ID AS USER_ID
:from [{:select [ORDERS.USER_ID AS USER_ID
ORDERS.PRODUCT_ID AS PRODUCT_ID
ORDERS.SUBTOTAL AS SUBTOTAL
ORDERS.TAX AS TAX
ORDERS.TOTAL AS TOTAL
ORDERS.DISCOUNT AS DISCOUNT
ORDERS.CREATED_AT AS CREATED_AT
ORDERS.QUANTITY AS QUANTITY
ABS (0) AS pivot-grouping
;; TODO -- I'm not sure if the order here is deterministic
PRODUCTS__via__PRODUCT_ID.CATEGORY AS PRODUCTS__via__PRODUCT_ID__CATEGORY
......@@ -452,15 +446,15 @@
:left-join [PRODUCTS AS PRODUCTS__via__PRODUCT_ID
ON ORDERS.PRODUCT_ID = PRODUCTS__via__PRODUCT_ID.ID
PEOPLE AS PEOPLE__via__USER_ID
ON ORDERS.USER_ID = PEOPLE__via__USER_ID.ID]}
ON ORDERS.USER_ID = PEOPLE__via__USER_ID.ID]
:where [((PEOPLE__via__USER_ID.SOURCE = ?) OR (PEOPLE__via__USER_ID.SOURCE = ?))
AND
((PRODUCTS__via__PRODUCT_ID.CATEGORY = ?) OR (PRODUCTS__via__PRODUCT_ID.CATEGORY = ?))
AND
(ORDERS.CREATED_AT >= DATE_TRUNC ("year" DATEADD ("year" CAST (-2 AS long) CAST (NOW () AS datetime))))
AND
(ORDERS.CREATED_AT < DATE_TRUNC ("year" NOW ()))]}
AS source]
:where [((source.PEOPLE__via__USER_ID__SOURCE = ?) OR (source.PEOPLE__via__USER_ID__SOURCE = ?))
AND
((source.PRODUCTS__via__PRODUCT_ID__CATEGORY = ?) OR (source.PRODUCTS__via__PRODUCT_ID__CATEGORY = ?))
AND
(source.CREATED_AT >= DATE_TRUNC ("year" DATEADD ("year" CAST (-2 AS long) CAST (NOW () AS datetime))))
AND
(source.CREATED_AT < DATE_TRUNC ("year" NOW ()))]
:group-by [source.PRODUCTS__via__PRODUCT_ID__CATEGORY
source.PEOPLE__via__USER_ID__SOURCE
DATE_TRUNC ("year" source.CREATED_AT)
......@@ -585,12 +579,7 @@
(testing "Generated SQL"
(is (= '{:select [source.PRICE AS PRICE
source.test AS test]
:from [{:select [VENUES.ID AS ID
VENUES.NAME AS NAME
VENUES.CATEGORY_ID AS CATEGORY_ID
VENUES.LATITUDE AS LATITUDE
VENUES.LONGITUDE AS LONGITUDE
TIMESTAMPADD ("second" VENUES.PRICE timestamp "1970-01-01T00:00:00Z") AS PRICE
:from [{:select [TIMESTAMPADD ("second" VENUES.PRICE timestamp "1970-01-01T00:00:00Z") AS PRICE
1 * 1 AS test]
:from [VENUES]}
AS source]
......@@ -732,14 +721,7 @@
(mt/dataset sample-dataset
(is (= '{:select [source.CATEGORY_2 AS CATEGORY_2
COUNT (*) AS count]
:from [{:select [PRODUCTS.ID AS ID
PRODUCTS.EAN AS EAN
PRODUCTS.TITLE AS TITLE
PRODUCTS.CATEGORY AS CATEGORY
PRODUCTS.VENDOR AS VENDOR
PRODUCTS.PRICE AS PRICE
PRODUCTS.RATING AS RATING
PRODUCTS.CREATED_AT AS CREATED_AT
:from [{:select [PRODUCTS.CATEGORY AS CATEGORY
CONCAT (PRODUCTS.CATEGORY ?) AS CATEGORY_2]
:from [PRODUCTS]}
AS source]
......@@ -819,10 +801,6 @@
(testing "Make sure FLOATING POINT division is done when dividing by expressions/fields"
(is (= '{:select [source.my_cool_new_field AS my_cool_new_field]
:from [{:select [VENUES.ID AS ID
VENUES.NAME AS NAME
VENUES.CATEGORY_ID AS CATEGORY_ID
VENUES.LATITUDE AS LATITUDE
VENUES.LONGITUDE AS LONGITUDE
VENUES.PRICE AS PRICE
VENUES.PRICE + 2 AS big_price
CAST
......@@ -849,24 +827,12 @@
(testing (format "hx/*honey-sql-version* = %d" hx/*honey-sql-version*)
(is (= (case hx/*honey-sql-version*
1 '{:select [source.my_cool_new_field AS my_cool_new_field]
:from [{:select [VENUES.ID AS ID
VENUES.NAME AS NAME
VENUES.CATEGORY_ID AS CATEGORY_ID
VENUES.LATITUDE AS LATITUDE
VENUES.LONGITUDE AS LONGITUDE
VENUES.PRICE AS PRICE
2.0 / 4.0 AS my_cool_new_field]
:from [{:select [2.0 / 4.0 AS my_cool_new_field]
:from [VENUES]}
AS source]
:limit [1]}
2 '{:select [source.my_cool_new_field AS my_cool_new_field]
:from [{:select [VENUES.ID AS ID
VENUES.NAME AS NAME
VENUES.CATEGORY_ID AS CATEGORY_ID
VENUES.LATITUDE AS LATITUDE
VENUES.LONGITUDE AS LONGITUDE
VENUES.PRICE AS PRICE
2.0 / 4.0 AS my_cool_new_field]
:from [{:select [2.0 / 4.0 AS my_cool_new_field]
:from [VENUES]}
AS source]
:limit [1]})
......
test/metabase/query_processor/util/nest_query_test.clj
+ 520
− 636
View file @ e587a20c
This diff is collapsed.
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