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

Retain source alias for aggregates (#23771)

parent 956bfd7b
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/query_processor_test.clj
+ 19
− 0
View file @ 6858dcef
......@@ -854,3 +854,22 @@
(let [t [:absolute-datetime #t "2022-04-22T18:27:00-08:00"]
hsql-form (sql.qp/add-interval-honeysql-form :bigquery-cloud-sdk t 3 :month)]
(sql.qp/format-honeysql :bigquery-cloud-sdk hsql-form))))))
(deftest custom-expression-with-space-in-having
(mt/test-driver :bigquery-cloud-sdk
(mt/dataset avian-singles
(testing "Custom expressions with spaces are matched properly (#22310)"
(let [name-with-spaces "sum id diff"
sql-query (-> (mt/mbql-query messages
{:filter [:> [:field name-with-spaces {:base-type :type/Float}] 5]
:source-query {:source-table $$messages
:aggregation [[:aggregation-options
[:sum [:- $sender_id $receiver_id]]
{:name name-with-spaces
:display-name name-with-spaces}]]
:breakout [$text]}
:limit 1})
qp/compile
:query)]
(is (not (str/includes? sql-query name-with-spaces))
(format "Query `%s' should not contain `%s'" sql-query name-with-spaces)))))))
src/metabase/query_processor/util/add_alias_info.clj
+ 12
− 7
View file @ 6858dcef
......@@ -186,7 +186,7 @@
(defn- exports [query]
(into #{} (mbql.u/match (dissoc query :source-query :source-metadata :joins)
[(_ :guard #{:field :expression :aggregation}) _ (_ :guard (every-pred map? ::position))])))
[(_ :guard #{:field :expression :aggregation-options}) _ (_ :guard (every-pred map? ::position))])))
(defn- join-with-alias [{:keys [joins]} join-alias]
(some (fn [join]
......@@ -216,12 +216,16 @@
(when-let [field-name (let [[_ id-or-name] field-clause]
(when (string? id-or-name)
id-or-name))]
(when-let [expression-exports (not-empty (filter (partial mbql.u/is-clause? :expression) all-exports))]
(some
(fn [[_ expression-name :as expression-clause]]
(when (= expression-name field-name)
expression-clause))
expression-exports))))))
(or (some
(fn [[_ expression-name :as expression-clause]]
(when (= expression-name field-name)
expression-clause))
(filter (partial mbql.u/is-clause? :expression) all-exports))
(some
(fn [[_ _ opts :as aggregation-options-clause]]
(when (= (::source-alias opts) field-name)
aggregation-options-clause))
(filter (partial mbql.u/is-clause? :aggregation-options) all-exports)))))))
(defn- matching-field-in-join-at-this-level
"If `field-clause` is the result of a join *at this level* with a `:source-query`, return the 'source' `:field` clause
......@@ -347,6 +351,7 @@
unique-alias (unique-alias-fn position original-ag-name)]
[:aggregation-options wrapped-ag-clause (assoc opts
:name unique-alias
::source-alias original-ag-name
::position position
::desired-alias unique-alias)]))
......
test/metabase/query_processor/util/add_alias_info_test.clj
+ 61
− 1
View file @ 6858dcef
......@@ -187,6 +187,7 @@
{:aggregation [[:aggregation-options
[:count]
{:name "count"
::add/source-alias "count"
::add/desired-alias "count"
::add/position 0}]]
:filter [:!=
......@@ -211,16 +212,19 @@
:aggregation [[:aggregation-options
[:count]
{:name "count"
::add/source-alias "count"
::add/desired-alias "count"
::add/position 0}]
[:aggregation-options
[:count]
{:name "count_2"
::add/source-alias "count_2"
::add/desired-alias "count_2"
::add/position 1}]
[:aggregation-options
[:count]
{:name "count_3"
::add/source-alias "count_3"
::add/desired-alias "count_3"
::add/position 2}]]}
:fields [[:field "count" {:base-type :type/BigInteger
......@@ -305,6 +309,7 @@
[:avg [:field %reviews.rating {::add/source-table $$reviews
::add/source-alias "RATING"}]]
{:name "avg"
::add/source-alias "avg"
::add/desired-alias "avg"
::add/position 1}]]
:breakout [[:field %products.category {:join-alias "P2"
......@@ -370,9 +375,12 @@
(driver/register! ::custom-escape :parent :h2)
(defn- prefix-alias [alias]
(str "COOL." alias))
(defmethod driver/escape-alias ::custom-escape
[_driver field-alias]
(str "COOL." field-alias))
(prefix-alias field-alias))
(deftest custom-escape-alias-test
(let [db (mt/db)]
......@@ -399,6 +407,7 @@
::add/position 0}]]
{:aggregation [[:aggregation-options [:count] {:name "COOL.count"
::add/position 1
::add/source-alias "count"
::add/desired-alias "COOL.count"}]]
:breakout [double-price]
:order-by [[:asc double-price]]})))
......@@ -413,6 +422,55 @@
add-alias-info
:query)))))))
(deftest custom-escape-alias-filtering-aggregation-test
(let [db (mt/db)]
(driver/with-driver ::custom-escape
(mt/with-db db
(is (query= (mt/$ids venues
(let [price [:field %price {::add/source-table $$venues
::add/source-alias "PRICE"
::add/desired-alias "COOL.PRICE"
::add/position 0}]
outer-price (-> price
(assoc-in [2 ::add/source-table] ::add/source)
(update-in [2 ::add/source-alias] prefix-alias)
(update-in [2 ::add/desired-alias] prefix-alias))
count-opts {:name "COOL.strange count"
::add/source-alias "strange count"
::add/desired-alias "COOL.strange count"
::add/position 1}
outer-count-opts (-> count-opts
(dissoc :name)
(assoc :base-type :type/BigInteger
::add/source-table ::add/source)
(update ::add/source-alias prefix-alias)
(update ::add/desired-alias prefix-alias))]
{:source-query
{:source-table $$venues
:breakout [price]
:aggregation [[:aggregation-options [:count] count-opts]]
:order-by [[:asc price]]}
:fields [outer-price
[:field
"strange count"
outer-count-opts]]
:filter [:<
[:field
"strange count"
outer-count-opts]
[:value 10 {:base_type :type/BigInteger}]]
:limit 1}))
(-> (mt/mbql-query venues
{:source-query {:source-table $$venues
:aggregation [[:aggregation-options
[:count]
{:name "strange count"}]]
:breakout [$price]}
:filter [:< [:field "strange count" {:base-type :type/BigInteger}] 10]
:limit 1})
add-alias-info
:query)))))))
(driver/register! ::custom-escape-spaces-to-underscores :parent :h2)
(defmethod driver/escape-alias ::custom-escape-spaces-to-underscores
......@@ -555,6 +613,7 @@
::add/source-alias "USER_ID"}]]
{:name "sum"
::add/position 0
::add/source-alias "sum"
::add/desired-alias "sum"}]]
:order-by [[:asc [:aggregation 0 {::add/desired-alias "sum"
::add/position 0}]]]})
......@@ -569,6 +628,7 @@
:breakout [[:expression "count" {::add/desired-alias "count"
::add/position 0}]]
:aggregation [[:aggregation-options [:count] {:name "count_2"
::add/source-alias "count"
::add/desired-alias "count_2"
::add/position 1}]]
:order-by [[:asc [:expression "count" {::add/desired-alias "count"
......
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