diff --git a/frontend/src/metabase-lib/drills.unit.spec.ts b/frontend/src/metabase-lib/drills.unit.spec.ts index ae8768002134cd37b63fe76df9b636530345abe9..85986a5645590491317a9a65e0be70d11166db76 100644 --- a/frontend/src/metabase-lib/drills.unit.spec.ts +++ b/frontend/src/metabase-lib/drills.unit.spec.ts @@ -1789,18 +1789,18 @@ describe("drillThru", () => { }, }, - // FIXME: distribution drill result for FK columns creates extra binning, which is wrong (metabase#34343) - // { - // drillType: "drill-thru/distribution", - // clickType: "header", - // columnName: "USER_ID", - // queryType: "unaggregated", - // expectedQuery: { - // aggregation: [["count"]], - // breakout: [["field", ORDERS.USER_ID, null]], - // "source-table": ORDERS_ID, - // }, - // }, + // distribution drill result for FK columns creates extra binning, which is wrong (metabase#34343) + { + drillType: "drill-thru/distribution", + clickType: "header", + columnName: "USER_ID", + queryType: "unaggregated", + expectedQuery: { + aggregation: [["count"]], + breakout: [["field", ORDERS.USER_ID, { "base-type": "type/Integer" }]], + "source-table": ORDERS_ID, + }, + }, { drillType: "drill-thru/distribution", clickType: "header", diff --git a/src/metabase/lib/drill_thru/distribution.cljc b/src/metabase/lib/drill_thru/distribution.cljc index 29de98294153f80e6d05fc98f46d2281fd0f0794..068abd5354468c624187e403a026646fdc54c701 100644 --- a/src/metabase/lib/drill_thru/distribution.cljc +++ b/src/metabase/lib/drill_thru/distribution.cljc @@ -34,15 +34,25 @@ :type :drill-thru/distribution :column column})) +(defn- add-temporal-bucketing-or-binning + [column] + (cond + (lib.types.isa/temporal? column) + (lib.temporal-bucket/with-temporal-bucket column :month) + + (and (lib.types.isa/numeric? column) + (not (lib.types.isa/foreign-key? column))) + (lib.binning/with-binning column (lib.binning/default-auto-bin)) + + :else + column)) + (mu/defmethod lib.drill-thru.common/drill-thru-method :drill-thru/distribution :- ::lib.schema/query [query :- ::lib.schema/query stage-number :- :int {:keys [column] :as _drill-thru} :- ::lib.schema.drill-thru/drill-thru.distribution] (when (lib.drill-thru.common/mbql-stage? query stage-number) - (let [breakout (cond - (lib.types.isa/temporal? column) (lib.temporal-bucket/with-temporal-bucket column :month) - (lib.types.isa/numeric? column) (lib.binning/with-binning column (lib.binning/default-auto-bin)) - :else column)] + (let [breakout (add-temporal-bucketing-or-binning column)] (-> query ;; Remove most of the target stage. (lib.util/update-query-stage stage-number dissoc :aggregation :breakout :limit :order-by) diff --git a/test/metabase/lib/drill_thru/distribution_test.cljc b/test/metabase/lib/drill_thru/distribution_test.cljc index 4c1f3b2ccf773dd6aac40d90f87705205e3b07fb..15e8041633fd57d79cd3701323027613440b1743 100644 --- a/test/metabase/lib/drill_thru/distribution_test.cljc +++ b/test/metabase/lib/drill_thru/distribution_test.cljc @@ -46,3 +46,18 @@ :query-type :unaggregated :column-name "QUANTITY" :expected {:type :drill-thru/distribution}})) + +(deftest ^:parallel apply-to-fk-column-test + (testing "do not apply binning to FK columns (#34343)" + (lib.drill-thru.tu/test-drill-application + {:click-type :header + :column-name "USER_ID" + :query-type :unaggregated + :drill-type :drill-thru/distribution + :expected {:type :drill-thru/distribution + :column {:name "USER_ID"}} + :expected-query {:stages [{:source-table (meta/id :orders) + :aggregation [[:count {}]] + :breakout [[:field + {:binning (symbol "nil #_\"key is not present.\"")} + (meta/id :orders :user-id)]]}]}})))