Skip to content
Snippets Groups Projects
Unverified Commit 4b80c779 authored by Braden Shepherdson's avatar Braden Shepherdson Committed by GitHub
Browse files

[MLv2] Ensure `:name` is unique on returned columns in the FE (#39118)

This is FE-only because this is a compatibility issue with the (legacy)
metadata coming back from the QP.

In time, the FE logic should be ported to MLv2 or made to depend on
things other than the `:name` field. MLv2 code itself (including BE
usage) should be based on the (already unique)
`:lib/desired-column-alias`.

Fixes #37517.
parent e8a19ead
No related branches found
No related tags found
No related merge requests found
......@@ -647,9 +647,15 @@
(defn- returned-columns*
"Inner implementation for [[returned-columns]], which wraps this with caching."
[a-query stage-number]
(let [stage (lib.util/query-stage a-query stage-number)]
(let [stage (lib.util/query-stage a-query stage-number)
unique-name-fn (lib.util/unique-name-generator)]
(->> (lib.metadata.calculation/returned-columns a-query stage-number stage)
(map #(assoc % :selected? true))
(map #(-> %
(assoc :selected? true)
;; Unique names are required by the FE for compatibility.
;; This applies only for JS; Clojure usage should prefer `:lib/desired-column-alias` to `:name`, and
;; that's already unique by construction.
(update :name unique-name-fn)))
to-array)))
(defn ^:export returned-columns
......
......@@ -517,3 +517,17 @@
(testing "unless it's missing in the input"
(let [di (lib.js/display-info query -1 (dissoc discount :fingerprint))]
(is (not (gobject/containsKey di "fingerprint")))))))))
(deftest ^:parallel returned-columns-unique-names-test
(testing "returned-columns should ensure the :name fields are unique (#37517)"
(let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :orders))
(lib/join (lib/join-clause (meta/table-metadata :orders)
[(lib/= (meta/field-metadata :orders :id)
(lib/with-join-alias (meta/field-metadata :orders :id)
"Orders"))])))]
(is (= #{1}
(->> (lib.js/returned-columns query -1)
(map :name)
frequencies
vals
set))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment