From d79628636b2bb860c08e58f213b825c67eb1b447 Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Mon, 24 Apr 2023 19:09:26 +0300
Subject: [PATCH] Generate unique aliases for operations in aggregations
(#30302)
Fixes #30262.
---
.../metabase/driver/mongo/query_processor.clj | 13 +++++++++++--
.../expression_aggregations_test.clj | 17 +++++++++++++++++
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
index ed7899d288d..0a30da3d0f2 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
@@ -1049,7 +1049,16 @@
(extract-aggregations (first args) parent-name aggregations-seen)
(aggregation-op op)
- (let [aggr-name (str parent-name "~" (annotate/aggregation-name aggr-expr))]
+ (let [aliases-taken (set (vals aggregations-seen))
+ aggr-name (annotate/aggregation-name aggr-expr)
+ desired-alias (str parent-name "~" aggr-name)
+ ;; find a free alias by appending increasing integers
+ ;; to the desired alias
+ aggr-name (some (fn [suffix]
+ (let [alias (str desired-alias suffix)]
+ (when-not (aliases-taken alias)
+ alias)))
+ (cons "" (iterate inc 1)))]
[(str \$ aggr-name) (assoc aggregations-seen aggr-expr aggr-name)])
:else
@@ -1061,7 +1070,7 @@
[aggr-expr aggregations-seen])))
(defn- simplify-extracted-aggregations
- "Simplifies the extracted aggregation ()for `aggr-name` if the expression
+ "Simplifies the extracted aggregation for `aggr-name` if the expression
contains only a single top-level aggregation. In this case there is no
need for namespacing and `aggr-name` can be used as the name of the group
introduced for the aggregation.
diff --git a/test/metabase/query_processor_test/expression_aggregations_test.clj b/test/metabase/query_processor_test/expression_aggregations_test.clj
index 3a8d5dee814..2a130c6d1ce 100644
--- a/test/metabase/query_processor_test/expression_aggregations_test.clj
+++ b/test/metabase/query_processor_test/expression_aggregations_test.clj
@@ -143,6 +143,23 @@
{:aggregation [[:+ [:max $price] [:min [:- $price $id]]]]
:breakout [$price]})))))))
+(deftest integer-aggregation-division-test
+ (testing "division of two sum aggregations (#30262)"
+ (mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
+ (mt/dataset sample-dataset
+ (testing "expression parts not selected"
+ (is (= [[27]]
+ (mt/formatted-rows [int]
+ (mt/run-mbql-query orders
+ {:aggregation [[:/ [:sum $product_id] [:sum $quantity]]]})))))
+ (testing "expression parts also selected"
+ (is (= [[1885900 69540 27]]
+ (mt/formatted-rows [int int int]
+ (mt/run-mbql-query orders
+ {:aggregation [[:sum $product_id]
+ [:sum $quantity]
+ [:/ [:sum $product_id] [:sum $quantity]]]})))))))))
+
(deftest aggregation-without-field-test
(mt/test-drivers (mt/normal-drivers-with-feature :expression-aggregations)
(testing "aggregation w/o field"
--
GitLab