diff --git a/frontend/src/metabase-lib/fields.ts b/frontend/src/metabase-lib/fields.ts
index 305c013d2114de48668a3359da4b87650fde7bff..c8998e712e50a183c266fc58cd8d902fff85f1f0 100644
--- a/frontend/src/metabase-lib/fields.ts
+++ b/frontend/src/metabase-lib/fields.ts
@@ -17,3 +17,10 @@ export function withFields(
): Query {
return ML.with_fields(query, stageIndex, newFields);
}
+
+export function fieldableColumns(
+ query: Query,
+ stageIndex: number,
+): ColumnMetadata[] {
+ return ML.fieldable_columns(query, stageIndex);
+}
diff --git a/src/metabase/lib/aggregation.cljc b/src/metabase/lib/aggregation.cljc
index c9f1977c4432977ff69097c9177bfd495cc81a3b..4288c3b99085d4ed98dc6bef87308e5e2995c882 100644
--- a/src/metabase/lib/aggregation.cljc
+++ b/src/metabase/lib/aggregation.cljc
@@ -317,17 +317,11 @@
:operator (:short aggregation-operator)
:args [column]}))
-(def ^:private SelectedColumnMetadata
- [:merge
- lib.metadata/ColumnMetadata
- [:map
- [:selected? {:optional true} :boolean]]])
-
(def ^:private SelectedOperatorWithColumns
[:merge
::lib.schema.aggregation/operator
[:map
- [:columns {:optional true} [:sequential SelectedColumnMetadata]]
+ [:columns {:optional true} [:sequential lib.metadata/ColumnMetadata]]
[:selected? {:optional true} :boolean]]])
(mu/defn selected-aggregation-operators :- [:maybe [:sequential SelectedOperatorWithColumns]]
@@ -346,8 +340,7 @@
(mapv (fn [col]
(let [a-ref (lib.ref/ref col)]
(cond-> col
- (or (lib.equality/= a-ref agg-col)
- (lib.equality/= a-ref (lib.util/with-default-effective-type agg-col)))
+ (lib.equality/ref= a-ref agg-col)
(assoc :selected? true))))
cols))))))
agg-operators))))
diff --git a/src/metabase/lib/card.cljc b/src/metabase/lib/card.cljc
index bbb0f57b461228afd2cb46924af81aa2a53149a0..9c86b14ebe5a05f46d45d235dea6038b9b6827f2 100644
--- a/src/metabase/lib/card.cljc
+++ b/src/metabase/lib/card.cljc
@@ -59,8 +59,8 @@
(when-let [card (lib.metadata/card metadata-providerable card-id)]
(card-metadata-columns metadata-providerable card)))
-(defmethod lib.metadata.calculation/default-columns-method :metadata/card
- [query _stage-number card unique-name-fn]
+(defmethod lib.metadata.calculation/visible-columns-method :metadata/card
+ [query _stage-number card {:keys [unique-name-fn], :as _options}]
(mapv (fn [col]
(assoc col :lib/desired-column-alias (unique-name-fn (:name col))))
(card-metadata-columns query card)))
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index f7316153694c48cd127ebe94e7c7aa1e4865b81a..55274b9142dd5ebabc80892ded5e300bbd84a7b4 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -136,7 +136,8 @@
lower]
[lib.field
fields
- with-fields]
+ with-fields
+ fieldable-columns]
[lib.filter
filter
filters
diff --git a/src/metabase/lib/equality.cljc b/src/metabase/lib/equality.cljc
index 2a6d6b4d60e6d7ec2c818933db545145caccc491..ea1576d7275fc05d3a1e5b95e854206eb96c7a71 100644
--- a/src/metabase/lib/equality.cljc
+++ b/src/metabase/lib/equality.cljc
@@ -3,7 +3,8 @@
(:refer-clojure :exclude [=])
(:require
[metabase.lib.dispatch :as lib.dispatch]
- [metabase.lib.hierarchy :as lib.hierarchy]))
+ [metabase.lib.hierarchy :as lib.hierarchy]
+ [metabase.lib.util :as lib.util]))
(defmulti =
"Determine whether two already-normalized pMBQL maps, clauses, or other sorts of expressions are equal. The basic rule
@@ -85,3 +86,10 @@
(map? x) ((get-method = :dispatch-type/map) x y)
(sequential? x) ((get-method = :dispatch-type/sequential) x y)
:else (clojure.core/= x y)))
+
+(defn ref=
+ "Are two refs `x` and `y` equal?"
+ [x y]
+ (or (= x y)
+ (= (lib.util/with-default-effective-type x)
+ (lib.util/with-default-effective-type y))))
diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc
index a5ae7b41903513ac889e39e391c4bef67b9c4495..a9e26602a10ba7cf32993edfe0c17e65a30d0986 100644
--- a/src/metabase/lib/field.cljc
+++ b/src/metabase/lib/field.cljc
@@ -4,6 +4,7 @@
[medley.core :as m]
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.binning :as lib.binning]
+ [metabase.lib.equality :as lib.equality]
[metabase.lib.expression :as lib.expression]
[metabase.lib.join :as lib.join]
[metabase.lib.metadata :as lib.metadata]
@@ -441,7 +442,7 @@
(lib.join/joined-field-desired-alias join-alias (:name field-metadata))
(:name field-metadata)))
-(defn with-fields
+(mu/defn with-fields :- ::lib.schema/query
"Specify the `:fields` for a query. Pass `nil` or an empty sequence to remove `:fields`."
([xs]
(fn [query stage-number]
@@ -450,7 +451,9 @@
([query xs]
(with-fields query -1 xs))
- ([query stage-number xs]
+ ([query :- ::lib.schema/query
+ stage-number :- :int
+ xs]
(let [xs (mapv (fn [x]
(lib.ref/ref (if (fn? x)
(x query stage-number)
@@ -458,10 +461,42 @@
xs)]
(lib.util/update-query-stage query stage-number u/assoc-dissoc :fields (not-empty xs)))))
-(defn fields
+(mu/defn fields :- [:maybe [:ref ::lib.schema/fields]]
"Fetches the `:fields` for a query. Returns `nil` if there are no `:fields`. `:fields` should never be empty; this is
enforced by the Malli schema."
([query]
(fields query -1))
- ([query stage-number]
+
+ ([query :- ::lib.schema/query
+ stage-number :- :int]
(:fields (lib.util/query-stage query stage-number))))
+
+(mu/defn fieldable-columns :- [:sequential lib.metadata/ColumnMetadata]
+ "Return a sequence of column metadatas for columns that you can specify in the `:fields` of a query. This is
+ basically just the columns returned by the source Table/Saved Question/Model or previous query stage.
+
+ Includes a `:selected?` key letting you know this column is already in `:fields` or not; if `:fields` is
+ unspecified, all these columns are returned by default, so `:selected?` is true for all columns (this is a little
+ strange but it matches the behavior of the QB UI)."
+ ([query]
+ (fieldable-columns query -1))
+
+ ([query :- ::lib.schema/query
+ stage-number :- :int]
+ (let [current-fields (fields query stage-number)
+ selected-column? (if (empty? current-fields)
+ (constantly true)
+ (fn [column]
+ (let [col-ref (lib.ref/ref column)]
+ (boolean
+ (some (fn [fields-ref]
+ (lib.equality/ref= col-ref fields-ref))
+ current-fields)))))]
+ (mapv (fn [col]
+ (assoc col :selected? (selected-column? col)))
+ (lib.metadata.calculation/visible-columns query
+ stage-number
+ (lib.util/query-stage query stage-number)
+ {:include-joined? false
+ :include-expressions? false
+ :include-implicitly-joinable? false})))))
diff --git a/src/metabase/lib/join.cljc b/src/metabase/lib/join.cljc
index 1249dc0bedee730cdc8aaf7a05439e288dd2a00e..3afea708e1acf6634d759e62c63a1cdffce4188a 100644
--- a/src/metabase/lib/join.cljc
+++ b/src/metabase/lib/join.cljc
@@ -127,15 +127,18 @@
field-name :- ::lib.schema.common/non-blank-string]
(lib.util/format "%s__%s" join-alias field-name))
-(defmethod lib.metadata.calculation/default-columns-method :mbql/join
- [query stage-number join unique-name-fn]
+(defn- add-source-and-desired-aliases
+ [join unique-name-fn col]
;; should be dev-facing-only so don't need to i18n
(assert (:alias join) "Join must have an alias to determine column aliases!")
- (mapv (fn [col]
- (assoc col
- :lib/source-column-alias (:name col)
- :lib/desired-column-alias (unique-name-fn (joined-field-desired-alias (:alias join) (:name col)))))
- (lib.metadata.calculation/metadata query stage-number join)))
+ (assoc col
+ :lib/source-column-alias (:name col)
+ :lib/desired-column-alias (unique-name-fn (joined-field-desired-alias (:alias join) (:name col)))))
+
+(defmethod lib.metadata.calculation/visible-columns-method :mbql/join
+ [query stage-number join {:keys [unique-name-fn], :as _options}]
+ (mapv (partial add-source-and-desired-aliases join unique-name-fn)
+ (lib.metadata.calculation/metadata query stage-number (assoc join :fields :all))))
(def ^:private JoinsWithAliases
"Schema for a sequence of joins that all have aliases."
@@ -156,14 +159,30 @@
(not (:alias join)) (assoc :alias (unique-name-fn (default-join-alias query stage-number join)))))
joins)))
-(mu/defn all-joins-default-columns :- lib.metadata.calculation/ColumnsWithUniqueAliases
- "Convenience for calling [[lib.metadata.calculation/default-columns]] on all of the joins in a query stage."
+(mu/defn all-joins-visible-columns :- lib.metadata.calculation/ColumnsWithUniqueAliases
+ "Convenience for calling [[lib.metadata.calculation/visible-columns]] on all of the joins in a query stage."
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
+ (into []
+ (mapcat (fn [join]
+ (lib.metadata.calculation/visible-columns query
+ stage-number
+ join
+ {:unique-name-fn unique-name-fn
+ :include-implicitly-joinable? false})))
+ (when-let [joins (:joins (lib.util/query-stage query stage-number))]
+ (ensure-all-joins-have-aliases query stage-number joins))))
+
+(mu/defn all-joins-metadata :- lib.metadata.calculation/ColumnsWithUniqueAliases
+ "Convenience for calling [[lib.metadata.calculation/metadata]] on all the joins in a query stage."
[query :- ::lib.schema/query
stage-number :- :int
unique-name-fn :- fn?]
(into []
(mapcat (fn [join]
- (lib.metadata.calculation/default-columns query stage-number join unique-name-fn)))
+ (map (partial add-source-and-desired-aliases join unique-name-fn)
+ (lib.metadata.calculation/metadata query stage-number join))))
(when-let [joins (:joins (lib.util/query-stage query stage-number))]
(ensure-all-joins-have-aliases query stage-number joins))))
diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs
index 4e941f091756badab5c38fc9d5eff90023a34d7e..4cb9b7087cc7ae18dce3a3c75d1c0bb866bf030f 100644
--- a/src/metabase/lib/js.cljs
+++ b/src/metabase/lib/js.cljs
@@ -402,3 +402,8 @@
(with-fields a-query -1 new-fields))
([a-query stage-number new-fields]
(lib.core/with-fields a-query stage-number new-fields)))
+
+(defn ^:export fieldable-columns
+ "Return a sequence of column metadatas for columns that you can specify in the `:fields` of a query."
+ [a-query stage-number]
+ (to-array (lib.core/fieldable-columns a-query stage-number)))
diff --git a/src/metabase/lib/metadata.cljc b/src/metabase/lib/metadata.cljc
index a7cca573ec9d68db3a14f891e747a05598543213..ece7432fcd4c9b1850f687ffecb8986f787975da 100644
--- a/src/metabase/lib/metadata.cljc
+++ b/src/metabase/lib/metadata.cljc
@@ -93,7 +93,12 @@
[:lib/source-column-alias {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
;; the name we should export this column as, i.e. the RHS of a `SELECT <lhs> AS <rhs>` or equivalent. This is
;; guaranteed to be unique in each stage of the query.
- [:lib/desired-column-alias {:optional true} [:maybe [:string {:min 1, :max 60}]]]])
+ [:lib/desired-column-alias {:optional true} [:maybe [:string {:min 1, :max 60}]]]
+ ;; when column metadata is returned by certain things
+ ;; like [[metabase.lib.aggregation/selected-aggregation-operators]] or [[metabase.lib.field/fieldable-columns]], it
+ ;; might include this key, which tells you whether or not that column is currently selected or not already, e.g.
+ ;; for [[metabase.lib.field/fieldable-columns]] it means its already present in `:fields`
+ [:selected? {:optional true} :boolean]])
(def ^:private CardMetadata
"More or less the same as a [[metabase.models.card]], but with kebab-case keys. Note that the `:dataset-query` is not
diff --git a/src/metabase/lib/metadata/calculation.cljc b/src/metabase/lib/metadata/calculation.cljc
index ae5a090627fe2d6d0b8e800928cfe54259501c6a..828acba4c9e138cc8d1c66c08f8dae59cc55765a 100644
--- a/src/metabase/lib/metadata/calculation.cljc
+++ b/src/metabase/lib/metadata/calculation.cljc
@@ -344,26 +344,8 @@
(merge (default-display-info query stage-number table)
{:is-source-table (= (lib.util/source-table query) (:id table))}))
-(defmulti default-columns-method
- "Impl for [[default-columns]]. This should mostly be similar to the implementation for [[metadata-method]], but needs
- to include `:lib/source-column-alias` and `:lib/desired-column-alias`. `:lib/source-column-alias` should probably be
- the same as `:name`; use the supplied `unique-name-fn` with the signature `(f str) => str` to ensure
- `:lib/desired-column-alias` is unique."
- {:arglists '([query stage-number x unique-name-fn])}
- (fn [_query _stage-number x _unique-name-fn]
- (lib.dispatch/dispatch-value x))
- :hierarchy lib.hierarchy/hierarchy)
-
-#_(defmethod default-columns-method :default
- [query stage-number x unique-name-fn]
- (mapv (fn [col]
- (assoc col
- :lib/source-column-alias (:name col)
- :lib/desired-column-alias (unique-name-fn (:name col))))
- (metadata query stage-number x)))
-
(def ColumnsWithUniqueAliases
- "Schema for column metadata that should be returned by [[default-columns]]. This is mostly used
+ "Schema for column metadata that should be returned by [[visible-columns]]. This is mostly used
to power metadata calculation for stages (see [[metabase.lib.stage]]."
[:and
[:sequential
@@ -383,47 +365,54 @@
(empty? columns)
(apply distinct? (map (comp u/lower-case-en :lib/desired-column-alias) columns))))]])
-(mu/defn default-columns :- ColumnsWithUniqueAliases
- "Return a sequence of column metadatas for columns that are returned 'by default' for a table/join/query stage.
-
- These columns will include `:lib/source-column-alias` and `:lib/desired-column-alias`. `:lib/desired-column-alias`
- is guaranteed to be unique; `unique-name-fn` is a function with the signature
-
- (f column-alias-string) => unique-alias-string
-
- Used to generate unique names."
- ([query]
- (default-columns query (lib.util/query-stage query -1)))
-
- ([query x]
- (default-columns query -1 x))
+(def VisibleColumnsOptions
+ "Schema for options passed to [[visible-columns]] and [[visible-columns-method]]."
+ [:map
+ ;; has the signature (f str) => str
+ [:unique-name-fn {:optional true} [:=>
+ [:cat ::lib.schema.common/non-blank-string]
+ ::lib.schema.common/non-blank-string]]
+ ;; these all default to true
+ [:include-joined? {:optional true} :boolean]
+ [:include-expressions? {:optional true} :boolean]
+ [:include-implicitly-joinable? {:optional true} :boolean]])
+
+(mu/defn ^:private default-visible-columns-options :- VisibleColumnsOptions
+ []
+ {:unique-name-fn (lib.util/unique-name-generator)
+ :include-joined? true
+ :include-expressions? true
+ :include-implicitly-joinable? true})
- ([query stage-number x]
- (default-columns query stage-number x (lib.util/unique-name-generator)))
+(defmulti visible-columns-method
+ "Impl for [[visible-columns]].
- ([query :- ::lib.schema/query
- stage-number :- :int
- x
- unique-name-fn :- fn?]
- (default-columns-method query stage-number x unique-name-fn)))
+ This should mostly be similar to the implementation for [[metadata-method]], but needs to include
+ `:lib/source-column-alias` and `:lib/desired-column-alias`. `:lib/source-column-alias` should probably be the same
+ as `:name`; use the supplied `:unique-name-fn` from `options` with the signature `(f str) => str` to ensure
+ `:lib/desired-column-alias` is unique.
-(defmulti visible-columns-method
- "Impl for [[visible-columns]]."
- {:arglists '([query stage-number x unique-name-fn])}
- (fn [_query _stage-number x _unique-name-fn]
+ Also, columns that aren't 'projected' should be returned as well -- in other words, ignore `:fields`,
+ `:aggregations`, and `:breakouts`."
+ {:arglists '([query stage-number x options])}
+ (fn [_query _stage-number x _options]
(lib.dispatch/dispatch-value x))
:hierarchy lib.hierarchy/hierarchy)
-(defmethod visible-columns-method :default
- [query stage-number x unique-name-fn]
- (default-columns query stage-number x unique-name-fn))
-
(mu/defn visible-columns :- ColumnsWithUniqueAliases
- "Return a sequence of columns that should be *visible* for something, e.g. a query stage or a join. Visible means
- both columns that are 'returned' by the query *AND* ones that are implicitly joinable.
+ "Return a sequence of columns that should be visible *within* a given stage of something, e.g. a query stage or a
+ join query. This includes not just the columns that get returned (ones present in [[metadata]], but other columns
+ that are 'reachable' in this stage of the query. E.g. in a query like
+
+ SELECT id, name
+ FROM table
+ ORDER BY position
+
+ only `id` and `name` are 'returned' columns, but other columns such as `position` are visible in this stage as well
+ and would thus be returned by this function.
- Default implementation is just [[default-columns]]; currently only stages have a specific implementation
- for [[visible-columns-method]], but this might change in the future."
+ Columns from joins, expressions, and implicitly joinable columns are included automatically by default;
+ see [[VisibleColumnsOptions]] for the options for disabling these columns."
([query]
(visible-columns query (lib.util/query-stage query -1)))
@@ -431,10 +420,11 @@
(visible-columns query -1 x))
([query stage-number x]
- (visible-columns query stage-number x (lib.util/unique-name-generator)))
+ (visible-columns query stage-number x nil))
([query :- ::lib.schema/query
stage-number :- :int
x
- unique-name-fn :- fn?]
- (visible-columns-method query stage-number x unique-name-fn)))
+ options :- [:maybe VisibleColumnsOptions]]
+ (let [options (merge (default-visible-columns-options) options)]
+ (visible-columns-method query stage-number x options))))
diff --git a/src/metabase/lib/stage.cljc b/src/metabase/lib/stage.cljc
index c53fab0a0374f48824cbd707bdacf47feef4e151..fe0da548cc5bb91a7a16aee4aa90179ec6061190 100644
--- a/src/metabase/lib/stage.cljc
+++ b/src/metabase/lib/stage.cljc
@@ -147,7 +147,8 @@
unique-name-fn :- fn?]
(when-let [card-id (lib.util/string-table-id->card-id source-table-id)]
(when-let [card (lib.metadata/card query card-id)]
- (lib.metadata.calculation/default-columns query stage-number card unique-name-fn))))
+ (lib.metadata.calculation/visible-columns query stage-number card {:unique-name-fn unique-name-fn
+ :include-implicitly-joinable? false}))))
(mu/defn ^:private expressions-metadata :- [:maybe lib.metadata.calculation/ColumnsWithUniqueAliases]
[query :- ::lib.schema/query
@@ -162,6 +163,41 @@
:lib/desired-column-alias (unique-name-fn (:name expression)))
(u/assoc-default :effective-type (or base-type :type/*)))))))
+(defn- implicitly-joinable-columns
+ "Columns that are implicitly joinable from some other columns in `column-metadatas`. To be joinable, the column has to
+ have appropriate FK metadata, i.e. have an `:fk-target-field-id` pointing to another Field. (I think we only include
+ this information for Databases that support FKs and joins, so I don't think we need to do an additional DB feature
+ check here.)
+
+ This does not include columns from any Tables that are already explicitly joined, and does not include multiple
+ versions of a column when there are multiple pathways to it (i.e. if there is more than one FK to a Table). This
+ behavior matches how things currently work in MLv1, at least for order by; we can adjust as needed in the future if
+ it turns out we do need that stuff.
+
+ Does not include columns that would be implicitly joinable via multiple hops."
+ [query stage-number column-metadatas unique-name-fn]
+ (let [existing-table-ids (into #{} (map :table-id) column-metadatas)]
+ (into []
+ (comp (filter :fk-target-field-id)
+ (m/distinct-by :fk-target-field-id)
+ (map (fn [{source-field-id :id, :keys [fk-target-field-id]}]
+ (-> (lib.metadata/field query fk-target-field-id)
+ (assoc ::source-field-id source-field-id))))
+ (remove #(contains? existing-table-ids (:table-id %)))
+ (m/distinct-by :table-id)
+ (mapcat (fn [{:keys [table-id], ::keys [source-field-id]}]
+ (let [table-metadata (lib.metadata/table query table-id)
+ options {:unique-name-fn unique-name-fn
+ :include-implicitly-joinable? false}]
+ (for [field (lib.metadata.calculation/visible-columns-method query stage-number table-metadata options)
+ :let [field (assoc field
+ :fk-field-id source-field-id
+ :lib/source :source/implicitly-joinable
+ :lib/source-column-alias (:name field))]]
+ (assoc field :lib/desired-column-alias (unique-name-fn
+ (lib.field/desired-alias query field))))))))
+ column-metadatas)))
+
;;; Calculate the columns to return if `:aggregations`/`:breakout`/`:fields` are unspecified.
;;;
;;; Formula for the so-called 'default' columns is
@@ -181,35 +217,57 @@
;;; PLUS
;;;
;;; 3. Columns added by joins at this stage
-(defmethod lib.metadata.calculation/default-columns-method ::stage
- [query stage-number _stage unique-name-fn]
+(mu/defn ^:private previous-stage-or-source-visible-columns :- lib.metadata.calculation/ColumnsWithUniqueAliases
+ "Return columns from the previous query stage or source Table/Card."
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ {:keys [unique-name-fn], :as options} :- lib.metadata.calculation/VisibleColumnsOptions]
+ {:pre [(fn? unique-name-fn)]}
+ (or
+ ;; 1a. columns returned by previous stage
+ (previous-stage-metadata query stage-number unique-name-fn)
+ ;; 1b or 1c
+ (let [{:keys [source-table], :as this-stage} (lib.util/query-stage query stage-number)]
+ (or
+ ;; 1b: default visible Fields for the source Table
+ (when (integer? source-table)
+ (let [table-metadata (lib.metadata/table query source-table)]
+ (lib.metadata.calculation/visible-columns query stage-number table-metadata options)))
+ ;; 1c. Metadata associated with a saved Question
+ (when (string? source-table)
+ (saved-question-metadata query stage-number source-table unique-name-fn))
+ ;; 1d: `:lib/stage-metadata` for the (presumably native) query
+ (for [col (:columns (:lib/stage-metadata this-stage))]
+ (assoc col
+ :lib/source :source/native
+ :lib/source-column-alias (:name col)
+ ;; these should already be unique, but run them thru `unique-name-fn` anyway to make sure anything
+ ;; that gets added later gets deduplicated from these.
+ :lib/desired-column-alias (unique-name-fn (:name col))))))))
+
+(mu/defn ^:private existing-visible-columns :- lib.metadata.calculation/ColumnsWithUniqueAliases
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ {:keys [unique-name-fn include-joined? include-expressions?], :as options} :- lib.metadata.calculation/VisibleColumnsOptions]
(concat
;; 1: columns from the previous stage, source table or query
- (or
- ;; 1a. columns returned by previous stage
- (previous-stage-metadata query stage-number unique-name-fn)
- ;; 1b or 1c
- (let [{:keys [source-table], :as this-stage} (lib.util/query-stage query stage-number)]
- (or
- ;; 1b: default visible Fields for the source Table
- (when (integer? source-table)
- (let [table-metadata (lib.metadata/table query source-table)]
- (lib.metadata.calculation/default-columns query stage-number table-metadata unique-name-fn)))
- ;; 1c. Metadata associated with a saved Question
- (when (string? source-table)
- (saved-question-metadata query stage-number source-table unique-name-fn))
- ;; 1d: `:lib/stage-metadata` for the (presumably native) query
- (for [col (:columns (:lib/stage-metadata this-stage))]
- (assoc col
- :lib/source :source/native
- :lib/source-column-alias (:name col)
- ;; these should already be unique, but run them thru `unique-name-fn` anyway to make sure anything
- ;; that gets added later gets deduplicated from these.
- :lib/desired-column-alias (unique-name-fn (:name col)))))))
+ (previous-stage-or-source-visible-columns query stage-number options)
;; 2: expressions (aka calculated columns) added in this stage
- (expressions-metadata query stage-number unique-name-fn)
+ (when include-expressions?
+ (expressions-metadata query stage-number unique-name-fn))
;; 3: columns added by joins at this stage
- (lib.join/all-joins-default-columns query stage-number unique-name-fn)))
+ (when include-joined?
+ (lib.join/all-joins-visible-columns query stage-number unique-name-fn))))
+
+(defmethod lib.metadata.calculation/visible-columns-method ::stage
+ [query stage-number _stage {:keys [unique-name-fn include-implicitly-joinable?], :as options}]
+ (let [;; query (lib.util/update-query-stage query stage-number dissoc :fields :breakout :aggregation)
+ existing-columns (existing-visible-columns query stage-number options)]
+ (concat
+ existing-columns
+ ;; add implicitly joinable columns if desired
+ (when include-implicitly-joinable?
+ (implicitly-joinable-columns query stage-number existing-columns unique-name-fn)))))
(mu/defn ^:private stage-metadata :- [:maybe lib.metadata.calculation/ColumnsWithUniqueAliases]
"Return results metadata about the expected columns in an MBQL query stage. If the query has
@@ -224,9 +282,9 @@
unique-name-fn :- fn?]
(or
(existing-stage-metadata query stage-number)
- (let [query (ensure-previous-stages-have-metadata query stage-number)
+ (let [query (ensure-previous-stages-have-metadata query stage-number)
summary-cols (summary-columns query stage-number unique-name-fn)
- field-cols (fields-columns query stage-number unique-name-fn)]
+ field-cols (fields-columns query stage-number unique-name-fn)]
;; ... then calculate metadata for this stage
(cond
summary-cols
@@ -237,10 +295,18 @@
(into []
(m/distinct-by #(dissoc % :source_alias :lib/source :lib/source-uuid :lib/desired-column-alias))
(concat field-cols
- (lib.join/all-joins-default-columns query stage-number unique-name-fn))))
+ (lib.join/all-joins-metadata query stage-number unique-name-fn))))
:else
- (lib.metadata.calculation/default-columns query stage-number (lib.util/query-stage query stage-number) unique-name-fn))))))
+ ;; there is no `:fields` or summary columns (aggregtions or breakouts) which means we return all the visible
+ ;; columns from the source or previous stage plus all the expressions. We return only the `:fields` from any
+ ;; joins
+ (concat
+ ;; we don't want to include all visible joined columns, so calculate that separately
+ (previous-stage-or-source-visible-columns query stage-number {:include-implicitly-joinable? false
+ :unique-name-fn unique-name-fn})
+ (expressions-metadata query stage-number unique-name-fn)
+ (lib.join/all-joins-metadata query stage-number unique-name-fn)))))))
(defmethod lib.metadata.calculation/metadata-method ::stage
[query stage-number _stage]
@@ -271,47 +337,6 @@
(lib.util/query-stage query previous-stage-number)
style))))
-(defn- implicitly-joinable-columns
- "Columns that are implicitly joinable from some other columns in `column-metadatas`. To be joinable, the column has to
- have appropriate FK metadata, i.e. have an `:fk-target-field-id` pointing to another Field. (I think we only include
- this information for Databases that support FKs and joins, so I don't think we need to do an additional DB feature
- check here.)
-
- This does not include columns from any Tables that are already explicitly joined, and does not include multiple
- versions of a column when there are multiple pathways to it (i.e. if there is more than one FK to a Table). This
- behavior matches how things currently work in MLv1, at least for order by; we can adjust as needed in the future if
- it turns out we do need that stuff.
-
- Does not include columns that would be implicitly joinable via multiple hops."
- [query stage-number column-metadatas unique-name-fn]
- (let [existing-table-ids (into #{} (map :table-id) column-metadatas)]
- (into []
- (comp (filter :fk-target-field-id)
- (m/distinct-by :fk-target-field-id)
- (map (fn [{source-field-id :id, :keys [fk-target-field-id]}]
- (-> (lib.metadata/field query fk-target-field-id)
- (assoc ::source-field-id source-field-id))))
- (remove #(contains? existing-table-ids (:table-id %)))
- (m/distinct-by :table-id)
- (mapcat (fn [{:keys [table-id], ::keys [source-field-id]}]
- (let [table-metadata (lib.metadata/table query table-id)]
- (for [field (lib.metadata.calculation/default-columns query stage-number table-metadata unique-name-fn)
- :let [field (assoc field
- :fk-field-id source-field-id
- :lib/source :source/implicitly-joinable
- :lib/source-column-alias (:name field))]]
- (assoc field :lib/desired-column-alias (unique-name-fn
- (lib.field/desired-alias query field))))))))
- column-metadatas)))
-
-(defmethod lib.metadata.calculation/visible-columns-method ::stage
- [query stage-number stage unique-name-fn]
- (let [query (lib.util/update-query-stage query stage-number dissoc :fields :breakout :aggregation)
- columns (lib.metadata.calculation/default-columns query stage-number stage unique-name-fn)]
- (concat
- columns
- (implicitly-joinable-columns query stage-number columns unique-name-fn))))
-
(mu/defn append-stage :- ::lib.schema/query
"Adds a new blank stage to the end of the pipeline"
[query]
diff --git a/src/metabase/lib/table.cljc b/src/metabase/lib/table.cljc
index e953d8eed311b71c2252e91bca0cc89e8e3804d0..bb552993826633b97f4bd5dbd496e65d72bf8719 100644
--- a/src/metabase/lib/table.cljc
+++ b/src/metabase/lib/table.cljc
@@ -57,8 +57,8 @@
[(or position 0) (u/lower-case-en (or field-name ""))])
field-metadatas))
-(defmethod lib.metadata.calculation/default-columns-method :metadata/table
- [query _stage-number table-metadata unique-name-fn]
+(defmethod lib.metadata.calculation/visible-columns-method :metadata/table
+ [query _stage-number table-metadata {:keys [unique-name-fn], :as _options}]
(when-let [field-metadatas (lib.metadata/fields query (:id table-metadata))]
(->> field-metadatas
remove-hidden-default-fields
diff --git a/src/metabase/lib/util.cljc b/src/metabase/lib/util.cljc
index 654c4980a1d9e8cdec50864799d515f3e7f5ee6b..88d1c7d3b6539d7696259a561290bef36f0930de 100644
--- a/src/metabase/lib/util.cljc
+++ b/src/metabase/lib/util.cljc
@@ -422,7 +422,9 @@
[query]
(-> query :stages first :source-table))
-(defn unique-name-generator
+(mu/defn unique-name-generator :- [:=>
+ [:cat ::lib.schema.common/non-blank-string]
+ ::lib.schema.common/non-blank-string]
"Create a new function with the signature
(f str) => str
diff --git a/test/metabase/lib/breakout_test.cljc b/test/metabase/lib/breakout_test.cljc
index e46b3c5b24bfe58cd798b8118f724dd7d2fc9b26..740e4000ff7db551e3c30bf313de9f5b563ace3c 100644
--- a/test/metabase/lib/breakout_test.cljc
+++ b/test/metabase/lib/breakout_test.cljc
@@ -441,3 +441,23 @@
(testing "description"
(is (= "Grouped by Expr"
(lib/describe-query query'))))))))
+
+(deftest ^:parallel breakoutable-columns-include-all-visible-columns-test
+ (testing "Include all visible columns, not just projected ones (#31233)"
+ (is (= ["ID"
+ "NAME"
+ "CATEGORY_ID"
+ "LATITUDE"
+ "LONGITUDE"
+ "PRICE"
+ "Categories__ID" ; this column is not projected, but should still be returned.
+ "Categories__NAME"]
+ (map :lib/desired-column-alias
+ (-> (lib/query meta/metadata-provider (meta/table-metadata :venues))
+ (lib/join (-> (lib/join-clause
+ (meta/table-metadata :categories)
+ [(lib/=
+ (lib/field "VENUES" "CATEGORY_ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Categories"))])
+ (lib/with-join-fields [(lib/with-join-alias (lib/field "CATEGORIES" "NAME") "Categories")])))
+ lib/breakoutable-columns))))))
diff --git a/test/metabase/lib/field_test.cljc b/test/metabase/lib/field_test.cljc
index f6f0df4acf62d5a3f7a53307d860cd63521ef97e..069baac40f77fbfa0314b44c6f5649f97376039e 100644
--- a/test/metabase/lib/field_test.cljc
+++ b/test/metabase/lib/field_test.cljc
@@ -484,3 +484,26 @@
(is (has-fields? query)
"sanity check")
(is (not (has-fields? query'))))))))))
+
+(deftest ^:parallel fieldable-columns-test
+ (testing "query with no :fields"
+ (is (=? [{:lib/desired-column-alias "ID", :selected? true}
+ {:lib/desired-column-alias "NAME", :selected? true}
+ {:lib/desired-column-alias "CATEGORY_ID", :selected? true}
+ {:lib/desired-column-alias "LATITUDE", :selected? true}
+ {:lib/desired-column-alias "LONGITUDE", :selected? true}
+ {:lib/desired-column-alias "PRICE", :selected? true}]
+ (lib/fieldable-columns (lib/query-for-table-name meta/metadata-provider "VENUES"))))))
+
+(deftest ^:parallel fieldable-columns-query-with-fields-test
+ (testing "query with :fields"
+ (is (=? [{:lib/desired-column-alias "ID", :selected? true}
+ {:lib/desired-column-alias "NAME", :selected? true}
+ {:lib/desired-column-alias "CATEGORY_ID", :selected? false}
+ {:lib/desired-column-alias "LATITUDE", :selected? false}
+ {:lib/desired-column-alias "LONGITUDE", :selected? false}
+ {:lib/desired-column-alias "PRICE", :selected? false}]
+ (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+ (lib/with-fields [(meta/field-metadata :venues :id)
+ (meta/field-metadata :venues :name)])
+ lib/fieldable-columns )))))
diff --git a/test/metabase/lib/metadata/calculation_test.cljc b/test/metabase/lib/metadata/calculation_test.cljc
index 20ae12ef3fa65faa17b2a268a952380d46ea31f7..1d097fdeb67e14b024b35712326952c5fe22cd38 100644
--- a/test/metabase/lib/metadata/calculation_test.cljc
+++ b/test/metabase/lib/metadata/calculation_test.cljc
@@ -58,3 +58,23 @@
"Product → Rating"
"Product → Created At"]
results))))
+
+(deftest ^:parallel visible-columns-test
+ (testing "Include all visible columns, not just projected ones (#31233)"
+ (is (= ["ID"
+ "NAME"
+ "CATEGORY_ID"
+ "LATITUDE"
+ "LONGITUDE"
+ "PRICE"
+ "Categories__ID" ; this column is not projected, but should still be returned.
+ "Categories__NAME"]
+ (map :lib/desired-column-alias
+ (-> (lib/query meta/metadata-provider (meta/table-metadata :venues))
+ (lib/join (-> (lib/join-clause
+ (meta/table-metadata :categories)
+ [(lib/=
+ (lib/field "VENUES" "CATEGORY_ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Categories"))])
+ (lib/with-join-fields [(lib/with-join-alias (lib/field "CATEGORIES" "NAME") "Categories")])))
+ lib.metadata.calculation/visible-columns))))))
diff --git a/test/metabase/lib/order_by_test.cljc b/test/metabase/lib/order_by_test.cljc
index a00df7f41e69979edd3dae2011ddf9ca1b1bb831..005e3d28417336ba4c965e2ab738d405c5668efb 100644
--- a/test/metabase/lib/order_by_test.cljc
+++ b/test/metabase/lib/order_by_test.cljc
@@ -647,6 +647,26 @@
{:display-name "Name", :lib/source :source/implicitly-joinable}]
(lib/orderable-columns query')))))))
+(deftest ^:parallel orderable-columns-include-all-visible-columns-test
+ (testing "Include all visible columns, not just projected ones (#31233)"
+ (is (= ["ID"
+ "NAME"
+ "CATEGORY_ID"
+ "LATITUDE"
+ "LONGITUDE"
+ "PRICE"
+ "Categories__ID" ; this column is not projected, but should still be returned.
+ "Categories__NAME"]
+ (map :lib/desired-column-alias
+ (-> (lib/query meta/metadata-provider (meta/table-metadata :venues))
+ (lib/join (-> (lib/join-clause
+ (meta/table-metadata :categories)
+ [(lib/=
+ (lib/field "VENUES" "CATEGORY_ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Categories"))])
+ (lib/with-join-fields [(lib/with-join-alias (lib/field "CATEGORIES" "NAME") "Categories")])))
+ lib/orderable-columns))))))
+
(deftest ^:parallel order-by-aggregation-test
(testing "Should be able to order by an aggregation (#30089)"
(let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")