Skip to content
Snippets Groups Projects
Unverified Commit a52bff8f authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Include name of joined table in field display_name (#10252)

* Include name of joined table in field display_name
[ci drivers]

* Fix linter errors
parent 04ebe615
No related branches found
No related tags found
No related merge requests found
......@@ -71,6 +71,14 @@
;;; --------------------------------------------------- Field Info ---------------------------------------------------
(s/defn ^:private display-name-for-joined-field
"Return an appropriate display name for a joined field that includes the table it came from if applicable."
[field-display-name {:keys [source-table], join-alias :alias}]
(let [join-display-name (if (integer? source-table)
(:name (qp.store/table source-table))
join-alias)]
(format "%s → %s" join-display-name field-display-name)))
(s/defn ^:private col-info-for-field-clause :- su/Map
[inner-query :- su/Map, clause :- mbql.s/Field]
;; for various things that can wrap Field clauses recurse on the wrapped Field but include a little bit of info
......@@ -85,8 +93,10 @@
(assoc (col-info-for-field-clause inner-query field) :unit unit)
[:joined-field alias field]
(let [{:keys [fk-field-id]} (join-with-alias inner-query alias)]
(assoc (col-info-for-field-clause inner-query field) :fk_field_id fk-field-id))
(let [{:keys [fk-field-id], :as join} (join-with-alias inner-query alias)]
(-> (col-info-for-field-clause inner-query field)
(assoc :fk_field_id fk-field-id)
(update :display_name display-name-for-joined-field join)))
;; TODO - should be able to remove this now
[:fk-> [:field-id source-field-id] field]
......
......@@ -75,9 +75,11 @@
[:field-id (data/id :categories :name)]]]}}
{:columns [:name]}))))
;; we should get `:fk_field_id` and information where possible when using `:joined-field` clauses
;; we should get `:fk_field_id` and information where possible when using `:joined-field` clauses; display_name should
;; include the joined table
(expect
[(assoc (info-for-field :categories :name)
:display_name "VENUES → Name"
:fk_field_id (data/id :venues :category_id), :source :fields)]
(qp.test-util/with-everything-store
(data/$ids venues
......@@ -92,6 +94,25 @@
:fk-field-id %category_id}]}}
{:columns [:name]})))))
;; when using `:joined-field` clauses for a join a source query (instead of a source table), `display_name` should
;; include the join alias
(expect
[(assoc (info-for-field :categories :name)
:display_name "cats → Name"
:fk_field_id (data/id :venues :category_id), :source :fields)]
(qp.test-util/with-everything-store
(data/$ids venues
(doall
(annotate/column-info
{:type :query
:query {:fields [&cats.categories.name]
:joins [{:alias "cats"
:source-query {:source-table $$venues}
:condition [:= $category_id &cats.categories.id]
:strategy :left-join
:fk-field-id %category_id}]}}
{:columns [:name]})))))
;; when a `:datetime-field` form is used, we should add in info about the `:unit`
(expect
[(assoc (info-for-field :venues :price)
......
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