diff --git a/src/metabase/lib/expression.cljc b/src/metabase/lib/expression.cljc index 604a9dc94593d8caed35ac71de13be69e86c0f08..4fa2700f3a9a7d6055e69a21d88c89434df60621 100644 --- a/src/metabase/lib/expression.cljc +++ b/src/metabase/lib/expression.cljc @@ -55,12 +55,13 @@ (defmethod lib.metadata.calculation/metadata-method :expression [query stage-number [_expression opts expression-name, :as expression-ref-clause]] - {:lib/type :metadata/column - :lib/source-uuid (:lib/uuid opts) - :name expression-name - :display-name (lib.metadata.calculation/display-name query stage-number expression-ref-clause) - :base-type (lib.metadata.calculation/type-of query stage-number expression-ref-clause) - :lib/source :source/expressions}) + {:lib/type :metadata/column + :lib/source-uuid (:lib/uuid opts) + :name expression-name + :lib/expression-name expression-name + :display-name (lib.metadata.calculation/display-name query stage-number expression-ref-clause) + :base-type (lib.metadata.calculation/type-of query stage-number expression-ref-clause) + :lib/source :source/expressions}) (defmethod lib.metadata.calculation/display-name-method :dispatch-type/integer [_query _stage-number n _style] diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc index b47e2326cd15c34cf78622796001e9214b111665..219da042bff9f0decbe66976d2b70be2c9b79e4b 100644 --- a/src/metabase/lib/field.cljc +++ b/src/metabase/lib/field.cljc @@ -419,12 +419,9 @@ [field-clause] field-clause) -(defmethod lib.ref/ref-method :metadata/column - [{source :lib/source, :as metadata}] - (case source - :source/aggregations (lib.aggregation/column-metadata->aggregation-ref metadata) - :source/expressions (lib.expression/column-metadata->expression-ref metadata) - (let [inherited-column? (#{:source/card :source/native :source/previous-stage} (:lib/source metadata)) +(defn- column-metadata->field-ref + [metadata] + (let [inherited-column? (#{:source/card :source/native :source/previous-stage} (:lib/source metadata)) options (merge {:lib/uuid (str (random-uuid)) :base-type (:base-type metadata) :effective-type (column-metadata-effective-type metadata)} @@ -438,7 +435,22 @@ {:source-field source-field-id}))] [:field options (if inherited-column? (or (:lib/desired-column-alias metadata) (:name metadata)) - (or (:id metadata) (:name metadata)))]))) + (or (:id metadata) (:name metadata)))])) + +(defmethod lib.ref/ref-method :metadata/column + [{source :lib/source, :as metadata}] + (case source + :source/aggregations (lib.aggregation/column-metadata->aggregation-ref metadata) + :source/expressions (lib.expression/column-metadata->expression-ref metadata) + ;; :source/breakouts hides the true origin of the column. Since it's impossible to + ;; break out by aggregation references at the current stage, we only have to check + ;; if we break out by an expression reference. :expression-name is only set for + ;; expression references, so if it's set, we have to generate an expression ref, + ;; otherwise we generate a normal field ref. + :source/breakouts (if (contains? metadata :lib/expression-name) + (lib.expression/column-metadata->expression-ref metadata) + (column-metadata->field-ref metadata)) + (column-metadata->field-ref metadata))) (defn- implicit-join-name [query {:keys [fk-field-id table-id], :as _field-metadata}] (when (and fk-field-id table-id) diff --git a/test/metabase/lib/order_by_test.cljc b/test/metabase/lib/order_by_test.cljc index e431bab3fb567463aee3269bca913b3cc212346d..895a26dd99523453c648ad19e0938f6628cefca2 100644 --- a/test/metabase/lib/order_by_test.cljc +++ b/test/metabase/lib/order_by_test.cljc @@ -1,6 +1,6 @@ (ns metabase.lib.order-by-test (:require - [clojure.test :refer [deftest is testing]] + [clojure.test :refer [are deftest is testing]] [medley.core :as m] [metabase.lib.convert :as lib.convert] [metabase.lib.core :as lib] @@ -163,6 +163,22 @@ :lib/source :source/breakouts}] (lib/orderable-columns query))))))) +(deftest ^:parallel order-by-breakout-expression-test + (testing "order-by with a broken out expression has correct reference (#32845)" + (let [query (lib/expression lib.tu/venues-query + "Category ID + 1" + (lib/+ (meta/field-metadata :venues :category-id) 1)) + breakout-col (m/find-first #(= (:lib/source %) :source/expressions) + (lib/breakoutable-columns query 0)) + query (lib/breakout query breakout-col)] + (are [query] (=? [:desc {} [:expression + {:base-type :type/Integer, :effective-type :type/Integer} + "Category ID + 1"]] + (get-in (lib/order-by query 0 (first (lib/orderable-columns query 0)) :desc) + [:stages 0 :order-by 0])) + query + (lib/append-stage query))))) + (deftest ^:parallel orderable-columns-test (let [query lib.tu/venues-query] (testing (lib.util/format "Query =\n%s" (u/pprint-to-str query))