From 50bc7818707c732efb50f4846363781f2f9bbd7b Mon Sep 17 00:00:00 2001
From: Anoop K <40392769+anoopk19@users.noreply.github.com>
Date: Wed, 28 Aug 2019 15:27:28 -0400
Subject: [PATCH] Fixes the error while sorting group by results (#10712)
* Fixes the error while sorting group by results
* Fixed formatting issues
* Add tests
---
.../metabase/driver/druid/query_processor.clj | 6 ++-
.../driver/druid/query_processor_test.clj | 48 ++++++++++++++++++-
.../druid/test/metabase/driver/druid_test.clj | 24 ++++++++++
3 files changed, 75 insertions(+), 3 deletions(-)
diff --git a/modules/drivers/druid/src/metabase/driver/druid/query_processor.clj b/modules/drivers/druid/src/metabase/driver/druid/query_processor.clj
index 658fbfb099c..368a0de13fd 100644
--- a/modules/drivers/druid/src/metabase/driver/druid/query_processor.clj
+++ b/modules/drivers/druid/src/metabase/driver/druid/query_processor.clj
@@ -84,7 +84,8 @@
:distinct___count
(= ag-type :aggregation-options)
- (recur (second ag))
+ (let [[_ wrapped-ag options] ag]
+ (or (:name options) (recur wrapped-ag)))
ag-type
ag-type
@@ -1010,7 +1011,8 @@
(defmethod handle-limit ::groupBy
[_ {limit :limit} updated-query]
(if-not limit
- updated-query
+ (-> updated-query
+ (assoc-in [:query :limitSpec :type] :default))
(-> updated-query
(assoc-in [:query :limitSpec :type] :default)
(assoc-in [:query :limitSpec :limit] limit))))
diff --git a/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj b/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj
index 6c4ab2ab94e..4ec7fb87e8f 100644
--- a/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj
+++ b/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj
@@ -47,6 +47,52 @@
{:aggregation [[:* [:count $id] 10]]
:breakout [$venue_price]}))
+(datasets/expect-with-driver :druid
+ {:projections [:venue_category_name :user_name :__count_0]
+ :query {:queryType :groupBy
+ :granularity :all
+ :dataSource "checkins"
+ :dimensions ["venue_category_name", "user_name"]
+ :context {:timeout 60000, :queryId "<Query ID>"}
+ :intervals ["1900-01-01/2100-01-01"]
+ :aggregations [{:type :cardinality
+ :name "__count_0"
+ :fieldNames ["venue_name"]}]
+ :limitSpec {:type :default
+ :columns [{:dimension "__count_0", :direction :descending}
+ {:dimension "venue_category_name", :direction :ascending}
+ {:dimension "user_name", :direction :ascending}]}}
+ :query-type ::druid.qp/groupBy
+ :mbql? true}
+ (query->native
+ {:aggregation [[:aggregation-options [:distinct [:field-id (data/id :checkins :venue_name)]] {:name "__count_0"}]]
+ :breakout [$venue_category_name $user_name]
+ :order-by [[:desc [:aggregation 0]] [:asc [:field-id (data/id :checkins :venue_category_name)]]]}))
+
+(datasets/expect-with-driver :druid
+ {:projections [:venue_category_name :user_name :__count_0]
+ :query {:queryType :groupBy
+ :granularity :all
+ :dataSource "checkins"
+ :dimensions ["venue_category_name", "user_name"]
+ :context {:timeout 60000, :queryId "<Query ID>"}
+ :intervals ["1900-01-01/2100-01-01"]
+ :aggregations [{:type :cardinality
+ :name "__count_0"
+ :fieldNames ["venue_name"]}]
+ :limitSpec {:type :default
+ :columns [{:dimension "__count_0", :direction :descending}
+ {:dimension "venue_category_name", :direction :ascending}
+ {:dimension "user_name", :direction :ascending}]
+ :limit 5}}
+ :query-type ::druid.qp/groupBy
+ :mbql? true}
+ (query->native
+ {:aggregation [[:aggregation-options [:distinct [:field-id (data/id :checkins :venue_name)]] {:name "__count_0"}]]
+ :breakout [$venue_category_name $user_name]
+ :order-by [[:desc [:aggregation 0]] [:asc [:field-id (data/id :checkins :venue_category_name)]]]
+ :limit 5}))
+
(datasets/expect-with-driver :druid
{:projections [:venue_category_name :__count_0]
:query {:queryType :topN
@@ -66,4 +112,4 @@
(query->native
{:aggregation [[:aggregation-options [:distinct [:field-id (data/id :checkins :venue_name)]] {:name "__count_0"}]]
:breakout [$venue_category_name]
- :order-by [[:desc [:aggregation 0]] [:asc [:field-id (data/id :checkins :venue_category_name)]]]}))
\ No newline at end of file
+ :order-by [[:desc [:aggregation 0]] [:asc [:field-id (data/id :checkins :venue_category_name)]]]}))
diff --git a/modules/drivers/druid/test/metabase/driver/druid_test.clj b/modules/drivers/druid/test/metabase/driver/druid_test.clj
index 609023cfb39..e94f1fb7f31 100644
--- a/modules/drivers/druid/test/metabase/driver/druid_test.clj
+++ b/modules/drivers/druid/test/metabase/driver/druid_test.clj
@@ -439,3 +439,27 @@
(u/pprint-to-str 'red results))
{:cols (->> results :data :cols (map :name))
:rows (-> results :data :rows)})))
+
+(datasets/expect-with-driver :druid
+ [["Bar" "Felipinho Asklepios" 8.015665809687173]
+ ["Bar" "Spiros Teofil" 8.015665809687173]
+ ["Japanese" "Felipinho Asklepios" 7.011990219885757]
+ ["Japanese" "Frans Hevel" 7.011990219885757]
+ ["Mexican" "Shad Ferdynand" 7.011990219885757]]
+ (druid-query-returning-rows
+ {:aggregation [[:aggregation-options [:distinct [:field-id (data/id :checkins :venue_name)]] {:name "__count_0"}]]
+ :breakout [$venue_category_name $user_name]
+ :order-by [[:desc [:aggregation 0]] [:asc [:field-id (data/id :checkins :venue_category_name)]]]
+ :limit 5}))
+
+(datasets/expect-with-driver :druid
+ [["American" "Rüstem Hebel" 1.0002442201269182]
+ ["Artisan" "Broen Olujimi" 1.0002442201269182]
+ ["Artisan" "Conchúr Tihomir" 1.0002442201269182]
+ ["Artisan" "Dwight Gresham" 1.0002442201269182]
+ ["Artisan" "Plato Yeshua" 1.0002442201269182]]
+ (druid-query-returning-rows
+ {:aggregation [[:aggregation-options [:distinct [:field-id (data/id :checkins :venue_name)]] {:name "__count_0"}]]
+ :breakout [$venue_category_name $user_name]
+ :order-by [[:asc [:aggregation 0]] [:asc [:field-id (data/id :checkins :venue_category_name)]]]
+ :limit 5}))
\ No newline at end of file
--
GitLab