Skip to content
Snippets Groups Projects
Unverified Commit bca22d97 authored by Jeff Evans's avatar Jeff Evans Committed by GitHub
Browse files

Fix BigQuery Percentile and Median expressions not using correct backtick quoting (#17979)

 Fix BigQuery Percentile and Median expressions not using correct backtick quoting

Ensure the identifier components are quoted by reifying `ToSql` for the `APPROX_QUANTILES` expression

Add test to ensure query is created properly

Mark `:percentile-aggregations` as being supported in the new driver
parent 0bcf8712
No related branches found
No related tags found
No related merge requests found
......@@ -286,7 +286,7 @@
;;; | Other Driver Method Impls |
;;; +----------------------------------------------------------------------------------------------------------------+
(defmethod driver/supports? [:bigquery-cloud-sdk :percentile-aggregations] [_ _] false)
(defmethod driver/supports? [:bigquery-cloud-sdk :percentile-aggregations] [_ _] true)
(defmethod driver/supports? [:bigquery-cloud-sdk :expressions] [_ _] true)
......
......@@ -23,6 +23,7 @@
[metabase.util.date-2 :as u.date]
[metabase.util.honeysql-extensions :as hx]
[metabase.util.i18n :refer [tru]]
[pretty.core :refer [PrettyPrintable]]
[schema.core :as s]
[toucan.db :as db])
(:import [com.google.cloud.bigquery Field$Mode FieldValue]
......@@ -415,13 +416,26 @@
[(Math/round x) (Math/round (Math/pow 10 power))]
(recur (* 10 x) (inc power)))))
(defn- approx-quantiles
"HoneySQL form for the APPROX_QUANTILES invocation. The [OFFSET(...)] part after the function call is odd and
needs special treatment."
[driver expr offset quantiles]
(let [expr-hsql (sql.qp/->honeysql driver expr)]
(reify
hformat/ToSql
(to-sql [_]
(format "APPROX_QUANTILES(%s, %s)[OFFSET(%s)]"
(hformat/to-sql expr-hsql)
quantiles
offset))
PrettyPrintable
(pretty [_]
(format "APPROX_QUANTILES(%s, %s)[OFFSET(%s)]" (pr-str expr) (pr-str quantiles) (pr-str offset))))))
(defmethod sql.qp/->honeysql [:bigquery-cloud-sdk :percentile]
[driver [_ arg p]]
(let [[offset quantiles] (percentile->quantile p)]
(hsql/raw (format "APPROX_QUANTILES(%s, %s)[OFFSET(%s)]"
(hformat/to-sql (sql.qp/->honeysql driver arg))
quantiles
offset))))
(approx-quantiles driver arg offset quantiles)))
(defmethod sql.qp/->honeysql [:bigquery-cloud-sdk :median]
[driver [_ arg]]
......
......@@ -821,3 +821,18 @@
:source-query {:source-table (mt/id :checkins)
:aggregation [[:count]]
:breakout [!month.date]}})))))))
(deftest custom-expression-args-quoted
(mt/test-driver :bigquery-cloud-sdk
(mt/dataset sample-dataset
(testing "Arguments to custom aggregation expression functions have backticks applied properly"
(is (= {:mbql? true
:params nil
:table-name "orders"
:query (str "SELECT APPROX_QUANTILES(`v3_sample_dataset.orders`.`quantity`, 10)[OFFSET(5)] AS `CE`"
" FROM `v3_sample_dataset.orders` LIMIT 10")}
(qp/query->native (mt/mbql-query orders
{:aggregation [[:aggregation-options
[:percentile $orders.quantity 0.5]
{:name "CE", :display-name "CE"}]]
:limit 10}))))))))
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