Skip to content

Commit

Permalink
[QP] Fix confusion of expressions with the same name as columns
Browse files Browse the repository at this point in the history
When determining column aliases in `add-alias-info`, reuse an existing
desired column alias if one is present.

Fixes #39059. Might fix #25931.
  • Loading branch information
bshepherdson committed Feb 28, 2024
1 parent 1c5508b commit d3d572e
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/metabase/lib/expression.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@
expression-name :- ::lib.schema.common/non-blank-string
expressionable]
(let [stage-number (or stage-number -1)]
(when (conflicting-name? query stage-number expression-name)
#_(when (conflicting-name? query stage-number expression-name)
(throw (ex-info "Expression name conflicts with a column in the same query stage"
{:expression-name expression-name})))
(lib.util/update-query-stage
Expand Down
10 changes: 8 additions & 2 deletions src/metabase/query_processor/util/add_alias_info.clj
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,14 @@
(when-let [field-name (let [[_ id-or-name] field-clause]
(when (string? id-or-name)
id-or-name))]
(or (m/find-first (fn [[_ expression-name :as _expression-clause]]
(or ;; Expressions by exact name.
(m/find-first (fn [[_ expression-name :as _expression-clause]]
(= expression-name field-name))
(filter (partial mbql.u/is-clause? :expression) all-exports))
;; Expressions whose ::desired-alias matches the name we're searching for.
(m/find-first (fn [[_expression _expression-name {::keys [desired-alias]} :as _expression-clause]]
(= desired-alias field-name))
(filter (partial mbql.u/is-clause? :expression) all-exports))
(m/find-first (fn [[_ _ opts :as _aggregation-options-clause]]
(= (::source-alias opts) field-name))
(filter (partial mbql.u/is-clause? :aggregation-options) all-exports))))
Expand Down Expand Up @@ -360,10 +365,11 @@
"Determine the appropriate `::desired-alias` for a `field-clause`."
{:arglists '([inner-query field-clause expensive-field-info])}
[_inner-query
[_ _id-or-name {:keys [join-alias]} :as _field-clause]
[_ _id-or-name {:keys [join-alias] ::keys [desired-alias]} :as _field-clause]
{:keys [field-name alias-from-join alias-from-source-query]}]
(cond
join-alias (prefix-field-alias join-alias (or alias-from-join field-name))
desired-alias desired-alias
alias-from-source-query alias-from-source-query
:else field-name))

Expand Down
17 changes: 13 additions & 4 deletions src/metabase/query_processor/util/nest_query.clj
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,19 @@
{base-type :base_type} (some-> expression-definition annotate/infer-expression-type)
{::add/keys [desired-alias]} (mbql.u/match-one source-query
[:expression (_ :guard (partial = expression-name)) source-opts]
source-opts)]
[:field
(or desired-alias expression-name)
(assoc opts :base-type (or base-type :type/*))]))
source-opts)
source-alias (or desired-alias expression-name)]
(let [res [:field
source-alias
(-> opts
(assoc :base-type (or base-type :type/*)
::add/source-table ::add/source
::add/source-alias source-alias)
#_(dissoc ::add/desired-alias))]]
#_(tap> (with-meta [_clause res]
{:portal.viewer/default :portal.viewer/diff
::raise-source-query-expression-ref true}))
res)))

(defn- rewrite-fields-and-expressions [query]
(mbql.u/replace query
Expand Down
4 changes: 2 additions & 2 deletions test/metabase/query_processor/util/nest_query_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -665,15 +665,15 @@
:breakout [[:field "CATEGORY_2" {:base-type :type/Text
::add/source-table ::add/source
::add/source-alias "CATEGORY_2"
::add/desired-alias "CATEGORY_2"
::add/desired-alias "CATEGORY"
::add/position 0}]]
:aggregation [[:aggregation-options [:count] {:name "count"
::add/desired-alias "count"
::add/position 1}]]
:order-by [[:asc [:field "CATEGORY_2" {:base-type :type/Text
::add/source-table ::add/source
::add/source-alias "CATEGORY_2"
::add/desired-alias "CATEGORY_2"
::add/desired-alias "CATEGORY"
::add/position 0}]]]
:limit 1})
(-> (lib.tu.macros/mbql-query products
Expand Down

0 comments on commit d3d572e

Please sign in to comment.