diff --git a/src/metabase/lib/aggregation.cljc b/src/metabase/lib/aggregation.cljc
index aa1426984c2705a771e66dd8d30a399bc3264b20..a98506dab0cbc95094d7eb8d2bf651e5d9137af4 100644
--- a/src/metabase/lib/aggregation.cljc
+++ b/src/metabase/lib/aggregation.cljc
@@ -11,6 +11,15 @@
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
+(mu/defn column-metadata->aggregation-ref :- :mbql.clause/aggregation
+ "Given `:metadata/field` column metadata for an aggregation, construct an `:aggregation` reference."
+ [metadata :- lib.metadata/ColumnMetadata]
+ (let [options {:lib/uuid (str (random-uuid))
+ :effective-type ((some-fn :effective_type :base_type) metadata)}
+ index (::aggregation-index metadata)]
+ (assert (integer? index) "Metadata for an aggregation reference should include ::aggregation-index")
+ [:aggregation options index]))
+
(mu/defn resolve-aggregation :- ::lib.schema.aggregation/aggregation
"Resolve an aggregation with a specific `index`."
[query :- ::lib.schema/query
@@ -32,14 +41,16 @@
(for [aggregation aggregations]
(lib.metadata.calculation/display-name query stage-number aggregation)))))
-(defmethod lib.metadata.calculation/metadata :aggregation
- [query stage-number [_ag opts index, :as aggregation-ref]]
+(defmethod lib.metadata.calculation/metadata-method :aggregation
+ [query stage-number [_ag opts index, :as _aggregation-ref]]
(let [aggregation (resolve-aggregation query stage-number index)]
(merge
(lib.metadata.calculation/metadata query stage-number aggregation)
- {:field_ref aggregation-ref}
+ {:lib/source :source/aggregations}
(when (:base-type opts)
- {:base_type (:base-type opts)}))))
+ {:base_type (:base-type opts)})
+ (when (:effective-type opts)
+ {:effective_type opts}))))
;;; TODO -- merge this stuff into `defop` somehow.
@@ -213,10 +224,6 @@
stage-number :- :int]
(when-let [aggregation-exprs (not-empty (:aggregation (lib.util/query-stage query stage-number)))]
(map-indexed (fn [i aggregation]
- (let [metadata (lib.metadata.calculation/metadata query stage-number aggregation)
- ag-ref [:aggregation
- {:lib/uuid (str (random-uuid))
- :base-type (:base_type metadata)}
- i]]
- (assoc metadata :field_ref ag-ref, :source :aggregation)))
+ (let [metadata (lib.metadata.calculation/metadata query stage-number aggregation)]
+ (assoc metadata :lib/source :source/aggregations, ::aggregation-index i)))
aggregation-exprs)))
diff --git a/src/metabase/lib/breakout.cljc b/src/metabase/lib/breakout.cljc
index 2c1ac99b4ef09669b707e1e3d86389330dfc1075..5c4d84a5cbde2b774b875ef980a24a3a6b744cb5 100644
--- a/src/metabase/lib/breakout.cljc
+++ b/src/metabase/lib/breakout.cljc
@@ -37,5 +37,5 @@
stage-number :- :int]
(when-let [breakout-exprs (not-empty (:breakout (lib.util/query-stage query stage-number)))]
(mapv (fn [field-ref]
- (assoc (lib.metadata.calculation/metadata query stage-number field-ref) :source :breakout))
+ (assoc (lib.metadata.calculation/metadata query stage-number field-ref) :lib/source :source/breakouts))
breakout-exprs)))
diff --git a/src/metabase/lib/common.cljc b/src/metabase/lib/common.cljc
index e11b85e1760d44f1b2a8cf4f6216ce1d23116d36..cda9d9be0ab0807e574f77de58dd8a8bdad0e0d9 100644
--- a/src/metabase/lib/common.cljc
+++ b/src/metabase/lib/common.cljc
@@ -1,14 +1,15 @@
(ns metabase.lib.common
(:require
[metabase.lib.dispatch :as lib.dispatch]
- [metabase.lib.field :as lib.field]
- #_{:clj-kondo/ignore [:unused-namespace]}
[metabase.lib.options :as lib.options]
+ [metabase.lib.ref :as lib.ref]
[metabase.lib.schema.common :as schema.common]
- #_{:clj-kondo/ignore [:unused-namespace]}
[metabase.util.malli :as mu])
#?(:cljs (:require-macros [metabase.lib.common])))
+(comment lib.options/keep-me
+ mu/keep-me)
+
(mu/defn external-op :- [:maybe ::schema.common/external-op]
"Convert the internal operator `clause` to the external format."
[[operator options :as clause]]
@@ -29,8 +30,8 @@
x)
(defmethod ->op-arg :metadata/field
- [query stage-number field-metadata]
- (lib.field/field query stage-number field-metadata))
+ [_query _stage-number field-metadata]
+ (lib.ref/ref field-metadata))
(defmethod ->op-arg :lib/external-op
[query stage-number {:keys [operator options args] :or {options {}}}]
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index cca93928b361bafad409d34f460a958691733bd6..f47949feab45c9c42170d7a0a9aedb173b48b2d7 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -2,7 +2,7 @@
"Currently this is mostly a convenience namespace for REPL and test usage. We'll probably have a slightly different
version of this for namespace for QB and QP usage in the future -- TBD."
(:refer-clojure :exclude [filter remove replace and or not = < <= > ->> >= not-empty case count distinct max min
- + - * / time abs concat replace])
+ + - * / time abs concat replace ref])
(:require
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.breakout :as lib.breakout]
@@ -18,6 +18,7 @@
[metabase.lib.normalize :as lib.normalize]
[metabase.lib.order-by :as lib.order-by]
[metabase.lib.query :as lib.query]
+ [metabase.lib.ref :as lib.ref]
[metabase.lib.segment :as lib.segment]
[metabase.lib.stage :as lib.stage]
[metabase.lib.table :as lib.table]
@@ -38,6 +39,7 @@
lib.normalize/keep-me
lib.order-by/keep-me
lib.query/keep-me
+ lib.ref/keep-me
lib.segment/keep-me
lib.stage/keep-me
lib.table/keep-me
@@ -67,6 +69,7 @@
table]
[lib.expression
expression
+ expressions
+
-
*
@@ -161,6 +164,8 @@
remove-clause
replace-clause
saved-question-query]
+ [lib.ref
+ ref]
[lib.stage
append-stage
drop-stage]
diff --git a/src/metabase/lib/dev.cljc b/src/metabase/lib/dev.cljc
index 42b3a72a297fc3e7a1ce4802fb2a8d418e05d005..a28321ed17914bcb7bc191a8d183f0b42f104761 100644
--- a/src/metabase/lib/dev.cljc
+++ b/src/metabase/lib/dev.cljc
@@ -2,9 +2,9 @@
"Conveniences for usage in REPL and tests. Things in this namespace are not meant for normal usage in the FE client or
in QB code."
(:require
- [metabase.lib.field :as lib.field]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.query :as lib.query]
+ [metabase.lib.ref :as lib.ref]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.common :as lib.schema.common]
[metabase.lib.schema.id :as lib.schema.id]
@@ -17,13 +17,11 @@
::lib.schema.common/non-blank-string]]
(if (integer? id-or-name)
(let [id id-or-name]
- (fn [query stage-number]
- (->> (lib.metadata/field query id)
- (lib.field/field query stage-number))))
+ (fn [query _stage-number]
+ (lib.ref/ref (lib.metadata/field query id))))
(let [column-name id-or-name]
(fn [query stage-number]
- (->> (lib.metadata/stage-column query stage-number column-name)
- (lib.field/field query stage-number))))))
+ (lib.ref/ref (lib.metadata/stage-column query stage-number column-name))))))
([table-name :- ::lib.schema.common/non-blank-string
field-name :- ::lib.schema.common/non-blank-string]
@@ -32,9 +30,8 @@
([schema :- [:maybe ::lib.schema.common/non-blank-string]
table-name :- ::lib.schema.common/non-blank-string
field-name :- ::lib.schema.common/non-blank-string]
- (fn [query stage-number]
- (->> (lib.metadata/field query schema table-name field-name)
- (lib.field/field query stage-number)))))
+ (fn [query _stage-number]
+ (lib.ref/ref (lib.metadata/field query schema table-name field-name)))))
(mu/defn query-for-table-name :- ::lib.schema/query
"Create a new query for a specific Table with a table name."
diff --git a/src/metabase/lib/expression.cljc b/src/metabase/lib/expression.cljc
index e173572fefeeea4beaf2c7b06d6da8bda0729adf..aa5aa295749dac4fd8edac745996ed705743e106 100644
--- a/src/metabase/lib/expression.cljc
+++ b/src/metabase/lib/expression.cljc
@@ -16,6 +16,13 @@
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
+(mu/defn column-metadata->expression-ref :- :mbql.clause/expression
+ "Given `:metadata/field` column metadata for an expression, construct an `:expression` reference."
+ [metadata :- lib.metadata/ColumnMetadata]
+ (let [options {:lib/uuid (str (random-uuid))
+ :effective-type ((some-fn :effective_type :base_type) metadata)}]
+ [:expression options (:name metadata)]))
+
(mu/defn resolve-expression :- ::lib.schema.expression/expression
"Find the expression with `expression-name` in a given stage of a `query`, or throw an Exception if it doesn't
exist."
@@ -34,11 +41,11 @@
[{:keys [operator options args] :or {options {}}}]
(lib.schema.expression/type-of* (into [(keyword operator) options] args)))
-(defmethod lib.metadata.calculation/metadata :expression
+(defmethod lib.metadata.calculation/metadata-method :expression
[query stage-number [_expression opts expression-name, :as expression-ref]]
(let [expression (resolve-expression query stage-number expression-name)]
{:lib/type :metadata/field
- :field_ref expression-ref
+ :lib/source :source/expressions
:name expression-name
:display_name (lib.metadata.calculation/display-name query stage-number expression-ref)
:base_type (or (:base-type opts)
@@ -208,11 +215,15 @@
(mu/defn expressions :- [:sequential lib.metadata/ColumnMetadata]
"Get metadata about the expressions in a given stage of a `query`."
- [query :- ::lib.schema/query
- stage-number :- :int]
- (for [[expression-name expression-definition] (:expressions (lib.util/query-stage query stage-number))]
- (let [metadata (lib.metadata.calculation/metadata query stage-number expression-definition)]
- (merge
- metadata
- {:field_ref [:expression {:lib/uuid (str (random-uuid)), :base-type (:base_type metadata)} expression-name]
- :source :expressions}))))
+ ([query]
+ (expressions query -1))
+
+ ([query :- ::lib.schema/query
+ stage-number :- :int]
+ (for [[expression-name expression-definition] (:expressions (lib.util/query-stage query stage-number))]
+ (let [metadata (lib.metadata.calculation/metadata query stage-number expression-definition)]
+ (merge
+ metadata
+ {:lib/source :source/expressions
+ :name expression-name
+ :display_name expression-name})))))
diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc
index a6acae705c312c70f898160427d9f2b57799d0c1..dbd505dcaebc9e4169be817e123e52caccef6a67 100644
--- a/src/metabase/lib/field.cljc
+++ b/src/metabase/lib/field.cljc
@@ -1,12 +1,12 @@
(ns metabase.lib.field
(:require
- [metabase.lib.convert :as lib.convert]
- [metabase.lib.dispatch :as lib.dispatch]
+ [metabase.lib.aggregation :as lib.aggregation]
+ [metabase.lib.expression :as lib.expression]
[metabase.lib.join :as lib.join]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.normalize :as lib.normalize]
- [metabase.lib.options :as lib.options]
+ [metabase.lib.ref :as lib.ref]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.common :as lib.schema.common]
[metabase.lib.schema.id :as lib.schema.id]
@@ -81,24 +81,29 @@
(update metadata :name (fn [field-name]
(str parent-name \. field-name)))))
-(defmethod lib.metadata.calculation/metadata :metadata/field
+(defmethod lib.metadata.calculation/metadata-method :metadata/field
[_query _stage-number field-metadata]
field-metadata)
;;; TODO -- base type should be affected by `temporal-unit`, right?
-(defmethod lib.metadata.calculation/metadata :field
- [query stage-number [_tag {:keys [base-type temporal-unit], :as opts} :as field-ref]]
+(defmethod lib.metadata.calculation/metadata-method :field
+ [query stage-number [_tag {:keys [source-field effective-type base-type temporal-unit join-alias], :as opts} :as field-ref]]
(let [field-metadata (resolve-field-metadata query stage-number field-ref)
metadata (merge
- {:lib/type :metadata/field
- :field_ref field-ref}
+ {:lib/type :metadata/field}
field-metadata
{:display_name (or (:display-name opts)
(lib.metadata.calculation/display-name query stage-number field-ref))}
+ (when effective-type
+ {:effective_type effective-type})
(when base-type
{:base_type base-type})
(when temporal-unit
- {:unit temporal-unit}))]
+ {::temporal-unit temporal-unit})
+ (when join-alias
+ {::join-alias join-alias})
+ (when source-field
+ {:fk_field_id source-field}))]
(cond->> metadata
(:parent_id metadata) (add-parent-column-metadata query))))
@@ -131,47 +136,67 @@
;; mostly for the benefit of JS, which does not enforce the Malli schemas.
(i18n/tru "[Unknown Field]")))
-(defmulti ^:private ->field
- {:arglists '([query stage-number field])}
- (fn [_query _stage-number field]
- (lib.dispatch/dispatch-value field)))
+(defmethod lib.temporal-bucket/current-temporal-bucket-method :field
+ [[_tag opts _id-or-name]]
+ (:temporal-unit opts))
-(defmethod ->field :field
- [_query _stage-number field-clause]
- field-clause)
+(defmethod lib.temporal-bucket/current-temporal-bucket-method :metadata/field
+ [metadata]
+ (::temporal-unit metadata))
-(defmethod ->field :metadata/field
- [_query _stage-number {base-type :base_type, field-id :id, field-name :name, field-ref :field_ref, :as _field-metadata}]
- (cond-> (or (when field-ref
- (lib.convert/->pMBQL field-ref))
- [:field {} (or field-id
- field-name)])
- base-type (lib.options/update-options assoc :base-type base-type)
- true lib.options/ensure-uuid))
-
-(defmethod ->field :dispatch-type/integer
- [query _stage-number field-id]
- (lib.metadata/field query field-id))
+(defmethod lib.temporal-bucket/temporal-bucket-method :field
+ [[_tag options id-or-name] unit]
+ (if unit
+ [:field (assoc options :temporal-unit unit) id-or-name]
+ [:field (dissoc options :temporal-unit) id-or-name]))
-;;; Pass in a function that takes `query` and `stage-number` to support ad-hoc usage in tests etc
-(defmethod ->field :dispatch-type/fn
- [query stage-number f]
- (f query stage-number))
+(defmethod lib.temporal-bucket/temporal-bucket-method :metadata/field
+ [metadata unit]
+ (assoc metadata ::temporal-unit unit))
-(defmethod lib.temporal-bucket/temporal-bucket* :field
- [[_field options id-or-name] unit]
- [:field (assoc options :temporal-unit unit) id-or-name])
+(defmethod lib.join/current-join-alias-method :field
+ [[_tag opts]]
+ (get opts :join-alias))
-(mu/defn field :- :mbql.clause/field
- "Create a `:field` clause."
- ([query x]
- (->field query -1 x))
- ([query stage-number x]
- (->field query stage-number x)))
+(defmethod lib.join/current-join-alias-method :metadata/field
+ [metadata]
+ (::join-alias metadata))
(defmethod lib.join/with-join-alias-method :field
- [field-ref join-alias]
- (lib.options/update-options field-ref assoc :join-alias join-alias))
+ [[_tag opts id-or-name] join-alias]
+ (if join-alias
+ [:field (assoc opts :join-alias join-alias) id-or-name]
+ [:field (dissoc opts :join-alias) id-or-name]))
+
+(defmethod lib.join/with-join-alias-method :metadata/field
+ [metadata join-alias]
+ (assoc metadata ::join-alias join-alias))
+
+(defmethod lib.ref/ref-method :field
+ [field-clause]
+ field-clause)
+
+(defmethod lib.ref/ref-method :metadata/field
+ [metadata]
+ (case (:lib/source metadata)
+ :source/aggregation (lib.aggregation/column-metadata->aggregation-ref metadata)
+ :source/expressions (lib.expression/column-metadata->expression-ref metadata)
+ (let [options (merge
+ {:lib/uuid (str (random-uuid))
+ :base-type (:base_type metadata)
+ :effective-type ((some-fn :effective_type :base_type) metadata)}
+ (when-let [join-alias (::join-alias metadata)]
+ {:join-alias join-alias})
+ (when-let [temporal-unit (::temporal-unit metadata)]
+ {:temporal-unit temporal-unit})
+ (when-let [source-field-id (:fk_field_id metadata)]
+ {:source-field source-field-id})
+ ;; TODO -- binning options.
+ )
+ always-use-name? (#{:source/card :source/native :source/previous-stage} (:lib/source metadata))]
+ [:field options (if always-use-name?
+ (:name metadata)
+ (or (:id metadata) (:name metadata)))])))
(defn fields
"Specify the `:fields` for a query."
@@ -183,6 +208,9 @@
(fields query -1 xs))
([query stage-number xs]
- (let [xs (mapv #(->field query stage-number %)
+ (let [xs (mapv (fn [x]
+ (lib.ref/ref (if (fn? x)
+ (x query stage-number)
+ x)))
xs)]
(lib.util/update-query-stage query stage-number assoc :fields xs))))
diff --git a/src/metabase/lib/join.cljc b/src/metabase/lib/join.cljc
index ed067364cc8db53180f46ee68b6eca5df08418af..330871592f8e9dad2913999f42aa264645dbf8a6 100644
--- a/src/metabase/lib/join.cljc
+++ b/src/metabase/lib/join.cljc
@@ -13,6 +13,42 @@
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
+(defmulti with-join-alias-method
+ "Implementation for [[with-join-alias]]."
+ {:arglists '([x join-alias])}
+ (fn [x _join-alias]
+ (lib.dispatch/dispatch-value x)))
+
+(defmethod with-join-alias-method :dispatch-type/fn
+ [f join-alias]
+ (fn [query stage-number]
+ (let [x (f query stage-number)]
+ (with-join-alias-method x join-alias))))
+
+(defmethod with-join-alias-method :mbql/join
+ [join join-alias]
+ (assoc join :alias join-alias))
+
+(mu/defn with-join-alias
+ "Add a specific `join-alias` to something `x`, either a `:field` or join map. Does not recursively update other
+ references (yet; we can add this in the future)."
+ [x join-alias :- ::lib.schema.common/non-blank-string]
+ (with-join-alias-method x join-alias))
+
+(defmulti current-join-alias-method
+ "Impl for [[current-join-alias]]."
+ {:arglists '([x])}
+ lib.dispatch/dispatch-value)
+
+(defmethod current-join-alias-method :default
+ [_x]
+ nil)
+
+(mu/defn current-join-alias :- [:maybe ::lib.schema.common/non-blank-string]
+ "Get the current join alias associated with something, if it has one."
+ [x]
+ (current-join-alias-method x))
+
(mu/defn resolve-join :- ::lib.schema.join/join
"Resolve a join with a specific `join-alias`."
[query :- ::lib.schema/query
@@ -35,21 +71,22 @@
(i18n/tru "Saved Question #{0}" card-id-str)))
(i18n/tru "Native Query")))
-(mu/defn ^:private column-from-join-fields :- lib.metadata/ColumnMetadata
+(mu/defn ^:private column-from-join-fields :- lib.metadata.calculation/ColumnMetadataWithSource
"For a column that comes from a join `:fields` list, add or update metadata as needed, e.g. include join name in the
display name."
[query :- ::lib.schema/query
stage-number :- :int
column-metadata :- lib.metadata/ColumnMetadata
join-alias :- ::lib.schema.common/non-blank-string]
- (let [[ref-type options arg] (:field_ref column-metadata)
- ref-with-join-alias [ref-type (assoc options :join-alias join-alias) arg]
- column-metadata (assoc column-metadata :source_alias join-alias)]
- (assoc column-metadata
- :field_ref ref-with-join-alias
- :display_name (lib.metadata.calculation/display-name query stage-number column-metadata))))
-
-(defmethod lib.metadata.calculation/metadata :mbql/join
+ (let [column-metadata (assoc column-metadata :source_alias join-alias)
+ col (-> (assoc column-metadata
+ :display_name (lib.metadata.calculation/display-name query stage-number column-metadata)
+ :lib/source :source/fields)
+ (with-join-alias join-alias))]
+ (assert (= (current-join-alias col) join-alias))
+ col))
+
+(defmethod lib.metadata.calculation/metadata-method :mbql/join
[query stage-number {:keys [fields stages], join-alias :alias, :or {fields :none}, :as _join}]
(when-not (= fields :none)
(let [field-metadatas (if (= fields :all)
@@ -149,28 +186,6 @@
(cond-> (join-clause query stage-number x)
condition (assoc :condition (join-condition query stage-number condition)))))
-(defmulti with-join-alias-method
- "Implementation for [[with-join-alias]]."
- {:arglists '([x join-alias])}
- (fn [x _join-alias]
- (lib.dispatch/dispatch-value x)))
-
-(mu/defn with-join-alias
- "Add a specific `join-alias` to something `x`, either a `:field` or join map. Does not recursively update other
- references (yet; we can add this in the future)."
- [x join-alias :- ::lib.schema.common/non-blank-string]
- (with-join-alias-method x join-alias))
-
-(defmethod with-join-alias-method :dispatch-type/fn
- [f join-alias]
- (fn [query stage-number]
- (let [x (f query stage-number)]
- (with-join-alias-method x join-alias))))
-
-(defmethod with-join-alias-method :mbql/join
- [join join-alias]
- (assoc join :alias join-alias))
-
(mu/defn with-join-fields
"Update a join (or a function that will return a join) to include `:fields`, either `:all`, `:none`, or a sequence of
references."
diff --git a/src/metabase/lib/metadata.cljc b/src/metabase/lib/metadata.cljc
index 72c3429ef1ec0e34f83bbed6cd5d9c7c56b6c229..8c429db1b5bbb42e012cb4864787989acc9c653e 100644
--- a/src/metabase/lib/metadata.cljc
+++ b/src/metabase/lib/metadata.cljc
@@ -5,7 +5,8 @@
[metabase.lib.schema.common :as lib.schema.common]
[metabase.lib.schema.id :as lib.schema.id]
[metabase.lib.util :as lib.util]
- [metabase.util.malli :as mu]))
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.registry :as mr]))
;;; Column vs Field?
;;;
@@ -28,6 +29,27 @@
;;; will this affect what MLv2 needs to know or does? Not clear at this point, but we'll probably want to abstract
;;; away dealing with Dimensions in the future so the FE QB GUI doesn't need to special case them.
+(mr/def ::column-source
+ [:enum
+ ;; these are for things from some sort of source other than the current stage;
+ ;; they must be referenced with string names rather than Field IDs
+ :source/card
+ :source/native
+ :source/previous-stage
+ ;; these are for things that were introduced by the current stage of the query; `:field` references should be
+ ;; referenced with Field IDs if available.
+ ;;
+ ;; default columns returned by the `:source-table` for the current stage.
+ :source/table-defaults
+ ;; specifically introduced by the corresponding top-level clauses.
+ :source/fields
+ :source/aggregations
+ :source/breakouts
+ ;; introduced by a join, not necessarily ultimately returned.
+ :source/joins
+ ;; Introduced by `:expressions`; not necessarily ultimately returned.
+ :source/expressions])
+
(def ColumnMetadata
"Malli schema for a valid map of column metadata, which can mean one of two things:
@@ -42,10 +64,25 @@
that they are largely compatible. So they're the same for now. We can revisit this in the future if we actually want
to differentiate between the two versions."
[:map
- [:lib/type [:= :metadata/field]] ; TODO -- should this be changed to `:metadata/column`?
- [:id {:optional true} ::lib.schema.id/field]
- [:name ::lib.schema.common/non-blank-string]
- [:display_name {:optional true} [:maybe ::lib.schema.common/non-blank-string]]])
+ [:lib/type [:= :metadata/field]] ; TODO -- should this be changed to `:metadata/column`?
+ [:name ::lib.schema.common/non-blank-string]
+ ;; TODO -- ignore `base_type` and make `effective_type` required; see #29707
+ [:base_type ::lib.schema.common/base-type]
+ [:id {:optional true} ::lib.schema.id/field]
+ [:display_name {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
+ [:effective_type {:optional true} [:maybe ::lib.schema.common/base-type]]
+ ;; if this is a field from another table (implicit join), this is the field in the current table that should be
+ ;; used to perform the implicit join. e.g. if current table is `VENUES` and this field is `CATEGORIES.ID`, then the
+ ;; `fk_field_id` would be `VENUES.CATEGORY_ID`. In a `:field` reference this is saved in the options map as
+ ;; `:source-field`.
+ [:fk_field_id {:optional true} [:maybe ::lib.schema.id/field]]
+ ;; Join alias of the table we're joining against, if any. Not really 100% clear why we would need this on top
+ ;; of [[metabase.lib.join/current-join-alias]], which stores the same info under a namespaced key. I think we can
+ ;; remove it.
+ [:source_alias {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
+ ;; what top-level clause in the query this metadata originated from, if it is calculated (i.e., if this metadata
+ ;; was generated by [[metabase.lib.metadata.calculation/metadata]])
+ [:lib/source {:optional true} [:ref ::column-source]]])
(def ^:private CardMetadata
[:map
diff --git a/src/metabase/lib/metadata/calculation.cljc b/src/metabase/lib/metadata/calculation.cljc
index 70e1d8daaf97022000ae658859c85a1ea5cca007..476360ec23a5a8c96c58865f9c6d58c1bfb8506a 100644
--- a/src/metabase/lib/metadata/calculation.cljc
+++ b/src/metabase/lib/metadata/calculation.cljc
@@ -2,6 +2,7 @@
(:require
[clojure.string :as str]
[metabase.lib.dispatch :as lib.dispatch]
+ [metabase.lib.metadata :as lib.metadata]
[metabase.lib.options :as lib.options]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.common :as lib.schema.common]
@@ -108,22 +109,41 @@
top-level-key :- TopLevelKey]
(describe-top-level-key-method query stage-number (keyword top-level-key))))
-(defmulti metadata
- "Calculate appropriate metadata for something. What this looks like depends on what we're calculating metadata for. If
- it's a reference or expression of some sort, this should return a single `:metadata/field` map (i.e., something
- satisfying the [[metabase.lib.metadata/ColumnMetadata]] schema. If it's something like a stage of a query or a join
- definition, it should return a sequence of metadata maps for all the columns 'returned' at that stage of the query."
+(defmulti metadata-method
+ "Impl for [[metadata]]."
{:arglists '([query stage-number x])}
(fn [_query _stage-number x]
(lib.dispatch/dispatch-value x)))
-(defmethod metadata :default
+(defmethod metadata-method :default
[query stage-number x]
{:lib/type :metadata/field
:base_type (lib.schema.expresssion/type-of x)
:name (column-name query stage-number x)
:display_name (display-name query stage-number x)})
+(def ColumnMetadataWithSource
+ "Schema for the column metadata that should be returned by [[metadata]]."
+ [:merge
+ lib.metadata/ColumnMetadata
+ [:map
+ [:lib/source ::lib.metadata/column-source]]])
+
+(mu/defn metadata :- [:or
+ lib.metadata/ColumnMetadata
+ [:sequential ColumnMetadataWithSource]]
+ "Calculate appropriate metadata for something. What this looks like depends on what we're calculating metadata for.
+ If it's a reference or expression of some sort, this should return a single `:metadata/field` map (i.e., something
+ satisfying the [[metabase.lib.metadata/ColumnMetadata]] schema. If it's something like a stage of a query or a join
+ definition, it should return a sequence of metadata maps for all the columns 'returned' at that stage of the query,
+ and include the `:lib/source` of where they came from."
+ ([query]
+ (metadata query -1 query))
+ ([query x]
+ (metadata query -1 x))
+ ([query stage-number x]
+ (metadata-method query stage-number x)))
+
(mu/defn describe-query :- ::lib.schema.common/non-blank-string
"Convenience for calling [[display-name]] on a query to describe the results of its final stage."
[query]
diff --git a/src/metabase/lib/normalize.cljc b/src/metabase/lib/normalize.cljc
index e292b8d429ab747b44c9f8ae209b31fbd6d1ee41..b0e17e42088900dbf8cd39d092456be5472282ff 100644
--- a/src/metabase/lib/normalize.cljc
+++ b/src/metabase/lib/normalize.cljc
@@ -35,9 +35,11 @@
"Default normalization functions keys when doing map normalization."
{:base-type keyword
:type keyword
+ ;; we can calculate `:field_ref` now using [[metabase.lib.ref/ref]]; `:field_ref` is wrong half of the time anyway,
+ ;; so ignore it.
+ :field_ref (constantly ::do-not-use-me)
:lib/type keyword
- :lib/options normalize
- :field_ref normalize})
+ :lib/options normalize})
(defn normalize-map
"[[normalize]] a map using `key-fn` (default [[clojure.core/keyword]]) for keys and
diff --git a/src/metabase/lib/options.cljc b/src/metabase/lib/options.cljc
index 484a07f12ca0f9b5f8204ac77b2dd72d33bd50df..30ecf1c1c58f0a051487022f5185bcbed1bc6b33 100644
--- a/src/metabase/lib/options.cljc
+++ b/src/metabase/lib/options.cljc
@@ -3,6 +3,8 @@
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
+;;; TODO -- not 100% sure we actually need all of this stuff anymore.
+
(defn- mbql-clause? [x]
(and (vector? x)
(keyword? (first x))))
diff --git a/src/metabase/lib/order_by.cljc b/src/metabase/lib/order_by.cljc
index 29fdc7c93c835abe6533ebef810ed3fdfdb3bac1..d65decbde0eb42048fd5ffba2640e486a8199077 100644
--- a/src/metabase/lib/order_by.cljc
+++ b/src/metabase/lib/order_by.cljc
@@ -3,10 +3,10 @@
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.breakout :as lib.breakout]
[metabase.lib.dispatch :as lib.dispatch]
- [metabase.lib.field :as lib.field]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.options :as lib.options]
+ [metabase.lib.ref :as lib.ref]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.expression :as lib.schema.expression]
[metabase.lib.schema.order-by :as lib.schema.order-by]
@@ -45,11 +45,14 @@
[_query _stage-number clause]
(lib.options/ensure-uuid clause))
-;;; by default, try to convert `x` to a Field clause and then order by `:asc`
+(defmethod ->order-by-clause :dispatch-type/fn
+ [query stage-number f]
+ (->order-by-clause query stage-number (f query stage-number)))
+
+;;; by default, try to convert `x` to a ref and then order by `:asc`
(defmethod ->order-by-clause :default
- [query stage-number x]
- (let [field-clause (lib.field/field query stage-number x)]
- (lib.options/ensure-uuid [:asc field-clause])))
+ [_query _stage-number x]
+ (lib.options/ensure-uuid [:asc (lib.ref/ref x)]))
(defn order-by-clause
"Create an order-by clause independently of a query, e.g. for `replace` or whatever."
diff --git a/src/metabase/lib/query.cljc b/src/metabase/lib/query.cljc
index f4eda4cd44d6b1b4e1858514d59b7b804f66324f..9604659e98137383d3bb3d009b0535c634e84162 100644
--- a/src/metabase/lib/query.cljc
+++ b/src/metabase/lib/query.cljc
@@ -60,7 +60,7 @@
{:type keyword
:stages (partial mapv lib.normalize/normalize)}))
-(defmethod lib.metadata.calculation/metadata :mbql/query
+(defmethod lib.metadata.calculation/metadata-method :mbql/query
[query stage-number x]
(lib.metadata.calculation/metadata query stage-number (lib.util/query-stage x stage-number)))
diff --git a/src/metabase/lib/ref.cljc b/src/metabase/lib/ref.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..e3960a72c7003b9895ab8ddd91e3801b3b77545b
--- /dev/null
+++ b/src/metabase/lib/ref.cljc
@@ -0,0 +1,18 @@
+(ns metabase.lib.ref
+ (:refer-clojure :exclude [ref])
+ (:require
+ [metabase.lib.dispatch :as lib.dispatch]
+ [metabase.lib.schema.ref :as lib.schema.ref]
+ [metabase.util.malli :as mu]))
+
+(defmulti ref-method
+ "Impl for [[ref]]. This should create a new ref every time it is called, i.e. it should have a fresh UUID every time
+ you call it."
+ {:arglists '([x])}
+ lib.dispatch/dispatch-value)
+
+(mu/defn ref :- ::lib.schema.ref/ref
+ "Create a fresh ref that can be added to a query, e.g. a `:field`, `:aggregation`, or `:expression` reference. Will
+ create a new UUID every time this is called."
+ ([x]
+ (ref-method x)))
diff --git a/src/metabase/lib/schema/ref.cljc b/src/metabase/lib/schema/ref.cljc
index 7bcde520d51ce18cd1c91d9d81ee4e07c1366d9f..fe59398578793243dde0249928bee24a1fc5a995 100644
--- a/src/metabase/lib/schema/ref.cljc
+++ b/src/metabase/lib/schema/ref.cljc
@@ -50,7 +50,7 @@
(defmethod expression/type-of* :field
[[_tag opts _id-or-name]]
- (or (:base-type opts)
+ (or ((some-fn :effective-type :base-type) opts)
::expression/type.unknown))
(mbql-clause/define-tuple-mbql-clause :expression
@@ -58,7 +58,7 @@
(defmethod expression/type-of* :expression
[[_tag opts _expression-name]]
- (or (:base-type opts)
+ (or ((some-fn :effective-type :base-type) opts)
::expression/type.unknown))
(mr/def ::aggregation-options
@@ -76,7 +76,7 @@
(defmethod expression/type-of* :aggregation
[[_tag opts _index]]
- (or (:base-type opts)
+ (or ((some-fn :effective-type :base-type) opts)
::expression/type.unknown))
(mr/def ::ref
diff --git a/src/metabase/lib/stage.cljc b/src/metabase/lib/stage.cljc
index 8a7e3ed7801ce597ccfe5742f3bb9ce029cf3adc..5c0b2cf0b22ac2aed647ee8b93a28edb78d8e151 100644
--- a/src/metabase/lib/stage.cljc
+++ b/src/metabase/lib/stage.cljc
@@ -30,13 +30,83 @@
{:aggregation (partial mapv lib.normalize/normalize)
:filter lib.normalize/normalize}))
-(mu/defn ^:private fields-columns :- [:maybe [:sequential lib.metadata/ColumnMetadata]]
+(mu/defn ^:private ensure-previous-stages-have-metadata :- ::lib.schema/query
+ "Recursively calculate the metadata for the previous stages and add it to them, we'll need it for metadata
+ calculations for `stage-number` and we don't want to have to calculate it more than once..."
+ [query :- ::lib.schema/query
+ stage-number :- :int]
+ (let [previous-stage-number (lib.util/previous-stage-number query stage-number)]
+ (cond-> query
+ previous-stage-number
+ (lib.util/update-query-stage previous-stage-number
+ assoc
+ ::cached-metadata
+ {:lib/type :metadata/results
+ :columns (stage-metadata query previous-stage-number)}))))
+
+(def ^:private StageMetadataColumns
+ [:sequential {:min 1} lib.metadata.calculation/ColumnMetadataWithSource])
+
+(def ^:private DistinctStageMetadataColumns
+ [:and
+ StageMetadataColumns
+ [:fn
+ ;; should be dev-facing only, so don't need to i18n
+ {:error/message "Column :names must be distinct!"
+ :error/fn (fn [{:keys [value]} _]
+ (str "Column :names must be distinct, got: " (pr-str (mapv :name value))))}
+ (fn [columns]
+ (apply distinct? (map :name columns)))]])
+
+(mu/defn ^:private existing-stage-metadata :- [:maybe StageMetadataColumns]
+ "Return existing stage metadata attached to a stage if is already present: return it as-is, but only if this is a
+ native stage or a source-Card stage. if it's any other sort of stage then ignore the metadata, it's probably wrong;
+ we can recalculate the correct metadata anyway."
+ [query :- ::lib.schema/query
+ stage-number :- :int]
+ (let [{stage-type :lib/type, :keys [source-table] :as stage} (lib.util/query-stage query stage-number)]
+ (or (::cached-metadata stage)
+ (when-let [metadata (:lib/stage-metadata stage)]
+ (when (or (= stage-type :mbql.stage/native)
+ (and (string? source-table)
+ (str/starts-with? source-table "card__")))
+ (let [source-type (case stage-type
+ :mbql.stage/native :source/native
+ :mbql.stage/mbql :source/card)]
+ (for [col (:columns metadata)]
+ (assoc col :lib/source source-type))))))))
+
+(mu/defn ^:private breakouts-columns :- [:maybe StageMetadataColumns]
+ [query :- ::lib.schema/query
+ stage-number :- :int]
+ (not-empty
+ (for [breakout (lib.breakout/breakouts query stage-number)]
+ (assoc breakout :lib/source :source/breakouts))))
+
+(mu/defn ^:private aggregations-columns :- [:maybe StageMetadataColumns]
+ [query :- ::lib.schema/query
+ stage-number :- :int]
+ (not-empty
+ (for [ag (lib.aggregation/aggregations query stage-number)]
+ (assoc ag :lib/source :source/aggregations))))
+
+(mu/defn ^:private fields-columns :- [:maybe StageMetadataColumns]
[query :- ::lib.schema/query
stage-number :- :int]
(when-let [{fields :fields} (lib.util/query-stage query stage-number)]
- (mapv (fn [ref-clause]
- (assoc (lib.metadata.calculation/metadata query stage-number ref-clause) :source :fields))
- fields)))
+ (not-empty
+ (for [field-ref fields]
+ (assoc (lib.metadata.calculation/metadata query stage-number field-ref) :lib/source :source/fields)))))
+
+(mu/defn ^:private breakout-ags-fields-columns :- [:maybe StageMetadataColumns]
+ [query :- ::lib.schema/query
+ stage-number :- :int]
+ (not-empty
+ (into []
+ cat
+ [(breakouts-columns query stage-number)
+ (aggregations-columns query stage-number)
+ (fields-columns query stage-number)])))
(defn- remove-hidden-default-fields
"Remove Fields that shouldn't be visible from the default Fields for a source Table.
@@ -54,15 +124,7 @@
[(or position 0) (u/lower-case-en (or field-name ""))])
field-metadatas))
-(defn- ensure-field-refs [field-metadatas]
- (for [field-metadata field-metadatas]
- (cond-> field-metadata
- (not (:field_ref field-metadata))
- (assoc :field_ref [:field
- {:lib/uuid (str (random-uuid)), :base-type (:base_type field-metadata)}
- ((some-fn :id :name) field-metadata)]))))
-
-(mu/defn ^:private source-table-default-fields :- [:maybe [:sequential lib.metadata/ColumnMetadata]]
+(mu/defn ^:private source-table-default-fields :- [:maybe StageMetadataColumns]
"Determine the Fields we'd normally return for a source Table.
See [[metabase.query-processor.middleware.add-implicit-clauses/add-implicit-fields]]."
[query :- ::lib.schema/query
@@ -71,7 +133,8 @@
(->> field-metadatas
remove-hidden-default-fields
sort-default-fields
- ensure-field-refs)))
+ (map (fn [col]
+ (assoc col :lib/source :source/table-defaults))))))
(mu/defn ^:private default-join-alias :- ::lib.schema.common/non-blank-string
"Generate an alias for a join that doesn't already have one."
@@ -99,13 +162,14 @@
(not (:alias join)) (assoc :alias (unique-name-generator (default-join-alias query stage-number join)))))
joins)))
-(mu/defn ^:private default-columns-added-by-join :- [:maybe [:sequential lib.metadata/ColumnMetadata]]
+(mu/defn ^:private default-columns-added-by-join :- [:maybe StageMetadataColumns]
[query :- ::lib.schema/query
stage-number :- :int
join :- ::lib.schema.join/join]
- (lib.metadata.calculation/metadata query stage-number join))
+ (for [col (lib.metadata.calculation/metadata query stage-number join)]
+ (assoc col :lib/source :source/joins)))
-(mu/defn ^:private default-columns-added-by-joins :- [:maybe [:sequential lib.metadata/ColumnMetadata]]
+(mu/defn ^:private default-columns-added-by-joins :- [:maybe StageMetadataColumns]
[query :- ::lib.schema/query
stage-number :- :int]
(when-let [joins (not-empty (:joins (lib.util/query-stage query stage-number)))]
@@ -114,7 +178,19 @@
(mapcat (partial default-columns-added-by-join query stage-number))
(ensure-all-joins-have-aliases query stage-number joins)))))
-(mu/defn ^:private default-columns :- [:sequential lib.metadata/ColumnMetadata]
+(mu/defn ^:private saved-question-metadata :- [:maybe StageMetadataColumns]
+ "Metadata associated with a Saved Question, if `:source-table` is a `card__<id>` string."
+ [query :- ::lib.schema/query
+ source-table-id]
+ (when (string? source-table-id)
+ (when-let [[_match card-id-str] (re-find #"^card__(\d+)$" source-table-id)]
+ (when-let [card-id (parse-long card-id-str)]
+ (when-let [result-metadata (:result_metadata (lib.metadata/card query card-id))]
+ (when-let [cols (not-empty (:columns result-metadata))]
+ (for [col cols]
+ (assoc col :lib/source :source/card))))))))
+
+(mu/defn ^:private default-columns :- StageMetadataColumns
"Calculate the columns to return if `:aggregations`/`:breakout`/`:fields` are unspecified.
Formula for the so-called 'default' columns is
@@ -136,66 +212,46 @@
;; 1: columns from the previous stage, source table or query
(if-let [previous-stage-number (lib.util/previous-stage-number query stage-number)]
;; 1a. columns returned by previous stage
- (stage-metadata query previous-stage-number)
+ (for [col (stage-metadata query previous-stage-number)]
+ (assoc col :lib/source :source/previous-stage))
;; 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)
(source-table-default-fields query source-table))
- ;; 1c. Metadata associated with a Saved Question, if `:source-table` is a `card__<id>` string, OR
- (when (string? source-table)
- (when-let [[_match card-id-str] (re-find #"^card__(\d+)$" source-table)]
- (when-let [card-id (parse-long card-id-str)]
- (when-let [result-metadata (:result_metadata (lib.metadata/card query card-id))]
- (not-empty (for [col (:columns result-metadata)]
- (assoc col
- :field_ref [:field
- {:lib/uuid (str (random-uuid)), :base-type (:base_type col)}
- (:name col)])))))))
- ;; 1d: `:lib/stage-metadata` for the native query
- (:columns (:lib/stage-metadata this-stage)))))
+ ;; 1c. Metadata associated with a saved Question
+ (saved-question-metadata query source-table)
+ ;; 1d: `:lib/stage-metadata` for the (presumably native) query
+ (for [col (:columns (:lib/stage-metadata this-stage))]
+ (assoc col :lib/source :source/native)))))
;; 2: columns added by joins at this stage
(default-columns-added-by-joins query stage-number)))
-(mu/defn ^:private stage-metadata :- [:and
- [:sequential {:min 1} lib.metadata/ColumnMetadata]
- [:fn
- ;; should be dev-facing only, so don't need to i18n
- {:error/message "Column :names must be distinct!"}
- (fn [columns]
- (apply distinct? (map :name columns)))]]
+(defn- ensure-distinct-names [metadatas]
+ (when (seq metadatas)
+ (let [f (mbql.u/unique-name-generator)]
+ (for [metadata metadatas]
+ (update metadata :name f)))))
+
+(mu/defn ^:private stage-metadata :- DistinctStageMetadataColumns
"Return results metadata about the expected columns in an MBQL query stage. If the query has
aggregations/breakouts/fields, then return THOSE. Otherwise return the defaults based on the source Table or
previous stage + joins."
[query :- ::lib.schema/query
stage-number :- :int]
- (or
- ;; stage metadata is already present: return it as-is
- (when-let [metadata (:lib/stage-metadata (lib.util/query-stage query stage-number))]
- (:columns metadata))
- ;; otherwise recursively calculate the metadata for the previous stages and add it to them, we'll need it for
- ;; calculations for this stage and we don't have to calculate it more than once...
- (let [query (let [previous-stage-number (lib.util/previous-stage-number query stage-number)]
- (cond-> query
- previous-stage-number
- (lib.util/update-query-stage previous-stage-number
- assoc
- :lib/stage-metadata
- {:lib/type :metadata/results
- :columns (stage-metadata query previous-stage-number)})))]
- ;; ... then calculate metadata for this stage
- (or
- (not-empty (into []
- cat
- [(lib.breakout/breakouts query stage-number)
- (lib.aggregation/aggregations query stage-number)
- (fields-columns query stage-number)]))
- (default-columns query stage-number)))))
+ (ensure-distinct-names
+ (or
+ (existing-stage-metadata query stage-number)
+ (let [query (ensure-previous-stages-have-metadata query stage-number)]
+ ;; ... then calculate metadata for this stage
+ (or
+ (breakout-ags-fields-columns query stage-number)
+ (default-columns query stage-number))))))
(doseq [stage-type [:mbql.stage/mbql
:mbql.stage/native]]
- (defmethod lib.metadata.calculation/metadata stage-type
+ (defmethod lib.metadata.calculation/metadata-method stage-type
[query stage-number _stage]
(stage-metadata query stage-number)))
@@ -236,19 +292,15 @@
(m/distinct-by :fk_target_field_id)
(map (fn [{source-field-id :id, target-field-id :fk_target_field_id}]
(-> (lib.metadata/field query target-field-id)
- (assoc :source-field-id source-field-id))))
+ (assoc ::source-field-id source-field-id))))
(remove #(contains? existing-table-ids (:table_id %)))
(m/distinct-by :table_id)
- (mapcat (fn [{table-id :table_id, :keys [source-field-id]}]
+ (mapcat (fn [{table-id :table_id, ::keys [source-field-id]}]
(for [field (source-table-default-fields query table-id)]
- (assoc field :field_ref [:field
- {:lib/uuid (str (random-uuid))
- :base-type (:base_type field)
- :source-field source-field-id}
- (:id field)])))))
+ (assoc field :fk_field_id source-field-id)))))
column-metadatas)))
-(mu/defn visible-columns :- [:sequential lib.metadata/ColumnMetadata]
+(mu/defn visible-columns :- StageMetadataColumns
"Columns that are visible inside a given stage of a query. Ignores `:fields`, `:breakout`, and `:aggregation`.
Includes columns that are implicitly joinable from other Tables."
[query stage-number]
diff --git a/src/metabase/lib/temporal_bucket.cljc b/src/metabase/lib/temporal_bucket.cljc
index 0c5231813030b20968604cff4772b301d1c1075d..37f304a705e7a8e4344791fd40f71089342d07a5 100644
--- a/src/metabase/lib/temporal_bucket.cljc
+++ b/src/metabase/lib/temporal_bucket.cljc
@@ -36,18 +36,18 @@
(i18n/tru "next {0} {1}" (pr-str n) (unit->i18n n unit))
(i18n/tru "last {0} {1}" (pr-str (abs n)) (unit->i18n (abs n) unit)))))
-(defmulti temporal-bucket*
+(defmulti temporal-bucket-method
"Implementation for [[temporal-bucket]]. Implement this to tell [[temporal-bucket]] how to add a bucket to a
particular MBQL clause."
{:arglists '([x unit])}
(fn [x _unit]
(lib.dispatch/dispatch-value x)))
-(defmethod temporal-bucket* :dispatch-type/fn
+(defmethod temporal-bucket-method :dispatch-type/fn
[f unit]
(fn [query stage-number]
(let [x (f query stage-number)]
- (temporal-bucket* x unit))))
+ (temporal-bucket-method x unit))))
(mu/defn temporal-bucket
"Add a temporal bucketing unit, e.g. `:day` or `:day-of-year`, to an MBQL clause or something that can be converted to
@@ -59,4 +59,33 @@
[:field 1 {:temporal-unit :day}]"
[x unit :- ::lib.schema.temporal-bucketing/unit]
- (temporal-bucket* x unit))
+ (temporal-bucket-method x unit))
+
+(defmulti current-temporal-bucket-method
+ "Implementation of [[current-temporal-bucket]]. Return the current temporal bucketing unit associated with `x`."
+ {:arglists '([x])}
+ lib.dispatch/dispatch-value)
+
+(defmethod current-temporal-bucket-method :default
+ [_x]
+ nil)
+
+(mu/defn current-temporal-bucket :- [:maybe ::lib.schema.temporal-bucketing/unit]
+ "Get the current temporal bucketing unit associated with something, if any."
+ [x]
+ (current-temporal-bucket-method x))
+
+(defmulti available-temporal-buckets-method
+ "Implementation for [[available-temporal-buckets]]. Return a set of units from
+ `:metabase.lib.schema.temporal-bucketing/unit` that are allowed to be used with `x`."
+ {:arglists '([x])}
+ lib.dispatch/dispatch-value)
+
+(defmethod available-temporal-buckets-method :default
+ [_x]
+ nil)
+
+(mu/defn available-temporal-buckets :- [:maybe [:set {:min 1} [:ref ::lib.schema.temporal-bucketing/unit]]]
+ "Get a set of available temporal bucketing units for `x`. Returns nil if no units are available."
+ [x]
+ (not-empty (temporal-bucket-method x)))
diff --git a/src/metabase/lib/util.cljc b/src/metabase/lib/util.cljc
index dbde3000c4b2e3cc7222c09e0d40aea984ede369..40804e83c15d7d06da0f809cce2e996f3504961a 100644
--- a/src/metabase/lib/util.cljc
+++ b/src/metabase/lib/util.cljc
@@ -10,7 +10,6 @@
[clojure.string :as str]
[metabase.lib.options :as lib.options]
[metabase.lib.schema :as lib.schema]
- [metabase.lib.schema.common :as lib.schema.common]
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]))
@@ -148,7 +147,7 @@
:native (native-query->pipeline query)
:query (mbql-query->pipeline query)))
-(mu/defn ^:private non-negative-stage-index :- ::lib.schema.common/int-greater-than-or-equal-to-zero
+(mu/defn ^:private non-negative-stage-index :- [:int {:min 0}]
"If `stage-number` index is a negative number e.g. `-1` convert it to a positive index so we can use `nth` on
`stages`. `-1` = the last stage, `-2` = the penultimate stage, etc."
[stages :- [:sequential [:ref ::lib.schema/stage]]
@@ -162,12 +161,11 @@
{:num-stages (count stages)})))
stage-number'))
-(defn previous-stage-number
+(mu/defn previous-stage-number :- [:maybe [:int {:min 0}]]
"The index of the previous stage, if there is one. `nil` if there is no previous stage."
- [{:keys [stages], :as _query} stage-number]
- (let [stage-number (if (neg? stage-number)
- (+ (count stages) stage-number)
- stage-number)]
+ [{:keys [stages], :as _query} :- :map
+ stage-number :- :int]
+ (let [stage-number (non-negative-stage-index stages stage-number)]
(when (pos? stage-number)
(dec stage-number))))
diff --git a/test/metabase/lib/aggregation_test.cljc b/test/metabase/lib/aggregation_test.cljc
index 61efb32b310dff4d89c5ac4f2ed6e34b4d19d952..b9c313aa026cde7d97348930661b7f5c31b746e1 100644
--- a/test/metabase/lib/aggregation_test.cljc
+++ b/test/metabase/lib/aggregation_test.cljc
@@ -3,7 +3,6 @@
[clojure.test :refer [are deftest is testing]]
[metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
- [metabase.lib.field :as lib.field]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.query :as lib.query]
@@ -193,5 +192,5 @@
(is (=? result-query
(-> q
(lib/aggregate {:operator :sum
- :args [(lib.field/field q (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))]})
+ :args [(lib/ref (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))]})
(dissoc :lib/metadata)))))))
diff --git a/test/metabase/lib/dev_test.cljc b/test/metabase/lib/dev_test.cljc
index 28819f72f887868310897ca2d60b110570ab7cda..a851e7ab82dc4b3b9a7b3cba6ba27d8f81738bbb 100644
--- a/test/metabase/lib/dev_test.cljc
+++ b/test/metabase/lib/dev_test.cljc
@@ -2,9 +2,6 @@
(:require
[clojure.test :refer [are deftest is]]
[metabase.lib.core :as lib]
- [metabase.lib.field :as lib.field]
- [metabase.lib.metadata :as lib.metadata]
- [metabase.lib.query :as lib.query]
[metabase.lib.test-metadata :as meta]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
@@ -20,17 +17,6 @@
(is (=? [:field {:lib/uuid string?} (meta/id :venues :id)]
(f {:lib/type :mbql/query, :lib/metadata meta/metadata-provider} -1)))))
-(deftest ^:parallel field-from-results-metadata-test
- (let [field-metadata (lib.metadata/stage-column (lib.query/saved-question-query
- meta/metadata-provider
- meta/saved-question)
- "ID")]
- (is (=? {:lib/type :metadata/field
- :name "ID"}
- field-metadata))
- (is (=? [:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]
- (#'lib.field/->field {} -1 field-metadata)))))
-
(deftest ^:parallel query-for-table-name-test
(is (=? {:lib/type :mbql/query
:database (meta/id)
diff --git a/test/metabase/lib/expression_test.cljc b/test/metabase/lib/expression_test.cljc
index 17fb916f3c6314f194da5319324ae200563843bb..954a18afc499651f557f6d0c9266bad10f58e7fb 100644
--- a/test/metabase/lib/expression_test.cljc
+++ b/test/metabase/lib/expression_test.cljc
@@ -90,7 +90,7 @@
(is (=? {:base_type :type/Integer
:name "double-price"
:display_name "double-price"
- :field_ref [:expression {:lib/uuid string?} "double-price"]}
+ :lib/source :source/expressions}
(lib.metadata.calculation/metadata
(lib.tu/venues-query-with-last-stage
{:expressions {"double-price" [:*
@@ -110,8 +110,7 @@
(is (=? [{:name "prev_month"
:display_name "prev_month"
:base_type :type/DateTime
- :source :fields
- :field_ref [:expression {:lib/uuid string?} "prev_month"]}]
+ :lib/source :source/fields}]
(lib.metadata.calculation/metadata query -1 query)))))
(deftest ^:parallel date-interval-names-test
@@ -165,7 +164,6 @@
(testing "Uses the first clause"
(testing "Gets the type information from the field"
(is (=? {:name "expr"
- :field_ref [:expression {} "expr"]
:display_name "expr"
:base_type :type/Text}
(infer-first [:coalesce
@@ -178,8 +176,7 @@
(testing "Gets the type information from the literal"
(is (=? {:base_type :type/Text
:name "expr"
- :display_name "expr"
- :field_ref [:expression {} "expr"]}
+ :display_name "expr"}
(infer-first [:coalesce {:lib/uuid (str (random-uuid))} "bar" (lib.tu/field-clause :venues :name)])))))))
(deftest ^:parallel infer-case-test
@@ -187,7 +184,6 @@
(testing "Uses first available type information"
(testing "From a field"
(is (=? {:name "expr"
- :field_ref [:expression {} "expr"]
:display_name "expr"
:base_type :type/Text}
(infer-first [:coalesce
@@ -203,7 +199,7 @@
(is (=? {:base_type :type/DateTime
:name "last-login-plus-2"
:display_name "last-login-plus-2"
- :field_ref [:expression {} "last-login-plus-2"]}
+ :lib/source :source/expressions}
(lib.metadata.calculation/metadata
(lib.tu/venues-query-with-last-stage
{:expressions {"last-login-plus-2" [:datetime-add
@@ -224,3 +220,11 @@
{:expressions {"one-hundred" 100}})
-1
[:expression {:lib/uuid (str (random-uuid))} "double-price"])))))
+
+(deftest ^:parallel expressions-names-test
+ (testing "expressions should include the original expression name"
+ (is (=? [{:name "expr"
+ :display_name "expr"}]
+ (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+ (lib/expression "expr" (lib/absolute-datetime "2020" :month))
+ lib/expressions)))))
diff --git a/test/metabase/lib/field_test.cljc b/test/metabase/lib/field_test.cljc
index b68486656523ab052ac5573462381315dd56859d..d653d60b88988c6e92ea9c42222510910bcb72fe 100644
--- a/test/metabase/lib/field_test.cljc
+++ b/test/metabase/lib/field_test.cljc
@@ -2,17 +2,25 @@
(:require
[clojure.test :refer [deftest is testing]]
[metabase.lib.core :as lib]
- [metabase.lib.field :as lib.field]
+ [metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
+ [metabase.lib.temporal-bucket :as lib.temporal-bucket]
[metabase.lib.test-metadata :as meta]
[metabase.lib.test-util :as lib.tu]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
-(deftest ^:parallel field-from-database-metadata-test
- (is (=? [:field {:base-type :type/BigInteger, :lib/uuid string?} (meta/id :venues :id)]
- (lib.field/field {:lib/metadata meta/metadata-provider} (meta/field-metadata :venues :id)))))
+(deftest ^:parallel field-from-results-metadata-test
+ (let [field-metadata (lib.metadata/stage-column (lib/saved-question-query
+ meta/metadata-provider
+ meta/saved-question)
+ "ID")]
+ (is (=? {:lib/type :metadata/field
+ :name "ID"}
+ field-metadata))
+ (is (=? [:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]
+ (lib/ref field-metadata)))))
(defn- grandparent-parent-child-id [field]
(+ (meta/id :venues :id)
@@ -23,17 +31,20 @@
(def ^:private grandparent-parent-child-metadata-provider
"A MetadataProvider for a Table that nested Fields: grandparent, parent, and child"
- (let [grandparent {:lib/type :metadata/field
- :name "grandparent"
- :id (grandparent-parent-child-id :grandparent)}
+ (let [grandparent {:lib/type :metadata/field
+ :name "grandparent"
+ :id (grandparent-parent-child-id :grandparent)
+ :base_type :type/Text}
parent {:lib/type :metadata/field
:name "parent"
:parent_id (grandparent-parent-child-id :grandparent)
- :id (grandparent-parent-child-id :parent)}
+ :id (grandparent-parent-child-id :parent)
+ :base_type :type/Text}
child {:lib/type :metadata/field
:name "child"
:parent_id (grandparent-parent-child-id :parent)
- :id (grandparent-parent-child-id :child)}]
+ :id (grandparent-parent-child-id :child)
+ :base_type :type/Text}]
(lib.tu/mock-metadata-provider
{:database meta/metadata
:tables [(meta/table-metadata :venues)]
@@ -58,7 +69,6 @@
(testing "For fields with parents we should return them with a combined name including parent's name"
(is (=? {:table_id (meta/id :venues)
:name "grandparent.parent"
- :field_ref [:field {} (grandparent-parent-child-id :parent)]
:parent_id (grandparent-parent-child-id :grandparent)
:id (grandparent-parent-child-id :parent)
:visibility_type :normal}
@@ -66,7 +76,6 @@
(testing "nested-nested fields should include grandparent name (etc)"
(is (=? {:table_id (meta/id :venues)
:name "grandparent.parent.child"
- :field_ref [:field {} (grandparent-parent-child-id :child)]
:parent_id (grandparent-parent-child-id :parent)
:id (grandparent-parent-child-id :child)
:visibility_type :normal}
@@ -77,22 +86,22 @@
(is (=? {:name "sum"
:display_name "sum of User ID"
:base_type :type/Integer
- :field_ref [:field {:base-type :type/Integer} "sum"]
:semantic_type :type/FK}
(lib.metadata.calculation/metadata
(lib.tu/venues-query-with-last-stage
- {:lib/stage-metadata
- {:lib/type :metadata/results
- :columns [{:lib/type :metadata/field
- :name "abc"
- :display_name "another Field"
- :base_type :type/Integer
- :semantic_type :type/FK}
- {:lib/type :metadata/field
- :name "sum"
- :display_name "sum of User ID"
- :base_type :type/Integer
- :semantic_type :type/FK}]}})
+ {:lib/type :mbql.stage/native
+ :lib/stage-metadata {:lib/type :metadata/results
+ :columns [{:lib/type :metadata/field
+ :name "abc"
+ :display_name "another Field"
+ :base_type :type/Integer
+ :semantic_type :type/FK}
+ {:lib/type :metadata/field
+ :name "sum"
+ :display_name "sum of User ID"
+ :base_type :type/Integer
+ :semantic_type :type/FK}]}
+ :native "SELECT whatever"})
-1
[:field {:lib/uuid (str (random-uuid)), :base-type :type/Integer} "sum"])))))
@@ -136,7 +145,7 @@
(let [field (f query -1)]
(is (=? [:field {:temporal-unit :year} (meta/id :checkins :date)]
field))
- (is (=? {:unit :year}
- (lib.metadata.calculation/metadata query -1 field)))
+ (is (=? :year
+ (lib.temporal-bucket/current-temporal-bucket (lib.metadata.calculation/metadata query -1 field))))
(is (= "Date (year)"
(lib.metadata.calculation/display-name query -1 field))))))
diff --git a/test/metabase/lib/filter_test.cljc b/test/metabase/lib/filter_test.cljc
index 74a548070c48295164c7cab18917efcf8978505b..1690d3eb53ebaa84b173e2f5cc5f134a1c371553 100644
--- a/test/metabase/lib/filter_test.cljc
+++ b/test/metabase/lib/filter_test.cljc
@@ -2,7 +2,6 @@
(:require
[clojure.test :refer [deftest is testing]]
[metabase.lib.core :as lib]
- [metabase.lib.field :as lib.field]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.test-metadata :as meta]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
@@ -144,7 +143,7 @@
(is (=? simple-filtered-query
(-> q1
(lib/filter {:operator :between
- :args [(lib.field/field q1 venues-category-id-metadata) 42 100]})
+ :args [(lib/ref venues-category-id-metadata) 42 100]})
(dissoc :lib/metadata)))))))
(deftest ^:parallel add-filter-test
@@ -190,14 +189,14 @@
(assoc-in simple-query [:stages 0 :filter] first-filter)
second-add
(lib/add-filter first-add {:operator "starts-with"
- :args [(lib.field/field simple-query venues-name-metadata) "prefix"]})
+ :args [(lib/ref venues-name-metadata) "prefix"]})
and-query
(assoc-in filtered-query
[:stages 0 :filter]
[:and {:lib/uuid string?} first-filter second-filter])
third-add
(lib/add-filter second-add {:operator :contains
- :args [(lib.field/field simple-query venues-name-metadata) "part"]})
+ :args [(lib/ref venues-name-metadata) "part"]})
extended-and-query
(assoc-in filtered-query
[:stages 0 :filter]
diff --git a/test/metabase/lib/join_test.cljc b/test/metabase/lib/join_test.cljc
index c88c71f16c6fcf5a46d43d060620d77e86b40b6b..26dcb96586c7f574781345b928ff807d27e973da 100644
--- a/test/metabase/lib/join_test.cljc
+++ b/test/metabase/lib/join_test.cljc
@@ -1,8 +1,8 @@
(ns metabase.lib.join-test
(:require
[clojure.test :refer [deftest is testing]]
+ [metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
- [metabase.lib.field :as lib.field]
[metabase.lib.join :as lib.join]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
@@ -46,8 +46,8 @@
(-> q
(lib/join (lib/query-for-table-name meta/metadata-provider "CATEGORIES")
{:operator :=
- :args [(lib.field/field q (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))
- (lib.field/field q (lib.metadata/field q nil "CATEGORIES" "ID"))]})
+ :args [(lib/ref (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))
+ (lib/ref (lib.metadata/field q nil "CATEGORIES" "ID"))]})
(dissoc :lib/metadata))))))
(deftest ^:parallel join-saved-question-test
@@ -104,18 +104,21 @@
(lib/join q2 (lib/= venues-category-id-metadata categories-id-metadata))
(dissoc :lib/metadata)))))))
-;;; FIXME
-#_(deftest ^:parallel col-info-implicit-join-test
+(deftest ^:parallel col-info-implicit-join-test
(testing (str "when a `:field` with `:source-field` (implicit join) is used, we should add in `:fk_field_id` "
"info about the source Field")
- (is (=? [(merge (dissoc (meta/field-metadata :categories :name) :database_type)
- {:fk_field_id (meta/id :venues :category-id)
- :source :fields
- :field_ref [:field {:fk-field-id (meta/id :venues :category-id)} (meta/id :categories :name)]})]
- (stage-metadata
- {:type :query
- :query {:source-table (meta/id :venues)
- :fields [[:field (meta/id :categories :name) {:fk-field-id (meta/id :venues :category-id)}]]}})))))
+ (let [query (lib/query
+ meta/metadata-provider
+ (lib.convert/->pMBQL
+ {:database (meta/id)
+ :type :query
+ :query {:source-table (meta/id :venues)
+ :fields [[:field (meta/id :categories :name) {:source-field (meta/id :venues :category-id)}]]}}))]
+ (is (=? [{:name "NAME"
+ :id (meta/id :categories :name)
+ :fk_field_id (meta/id :venues :category-id)
+ :lib/source :source/fields}]
+ (lib.metadata.calculation/metadata query -1 query))))))
(deftest ^:parallel col-info-explicit-join-test
(testing "Display name for a joined field should include a nice name for the join; include other info like :source_alias"
@@ -142,10 +145,15 @@
:source-table (meta/id :categories)}]}]}]
:database (meta/id)
:lib/metadata meta/metadata-provider}]
- (is (=? [(merge (meta/field-metadata :categories :name)
- {:display_name "Categories → Name"
- :source :fields
- :field_ref [:field
- {:lib/uuid string?, :join-alias "CATEGORIES__via__CATEGORY_ID"}
- (meta/id :categories :name)]})]
- (lib.metadata.calculation/metadata query -1 query))))))
+ (let [metadata (lib.metadata.calculation/metadata query)]
+ (is (=? [(merge (meta/field-metadata :categories :name)
+ {:display_name "Categories → Name"
+ :lib/source :source/fields
+ :metabase.lib.field/join-alias "CATEGORIES__via__CATEGORY_ID"})]
+ metadata))
+ (is (=? "CATEGORIES__via__CATEGORY_ID"
+ (lib.join/current-join-alias (first metadata))))
+ (is (=? [:field
+ {:lib/uuid string?, :join-alias "CATEGORIES__via__CATEGORY_ID"}
+ (meta/id :categories :name)]
+ (lib/ref (first metadata))))))))
diff --git a/test/metabase/lib/metadata_test.cljc b/test/metabase/lib/metadata_test.cljc
index beb930797b0ac2d1b798898b41afcd9f284258d9..5655f62e601576192aa6af72c374f6e29c21f5ee 100644
--- a/test/metabase/lib/metadata_test.cljc
+++ b/test/metabase/lib/metadata_test.cljc
@@ -34,7 +34,6 @@
(let [query (lib/saved-question-query meta/metadata-provider meta/saved-question)]
(are [x] (=? {:lib/type :metadata/field
:display_name "CATEGORY_ID"
- :field_ref [:field "CATEGORY_ID" {:base-type :type/Integer}]
:name "CATEGORY_ID"
:base_type :type/Integer
:effective_type :type/Integer
diff --git a/test/metabase/lib/order_by_test.cljc b/test/metabase/lib/order_by_test.cljc
index 0df5e17e90372fecdfbc33e9fa4899ca130ba392..637278d25e871f2b4579e802f4b66206b68ff473 100644
--- a/test/metabase/lib/order_by_test.cljc
+++ b/test/metabase/lib/order_by_test.cljc
@@ -98,7 +98,7 @@
lib/order-bys))))
;;; the following tests use raw legacy MBQL because they're direct ports of JavaScript tests from MLv1 and I wanted to
-;;; make sure that given an existing query, the expected description was generated correctly.
+;;; make sure that given an existing query the expected description was generated correctly.
(defn- describe-legacy-query-order-by [query]
(-> (lib.query/query meta/metadata-provider (lib.convert/->pMBQL query))
@@ -122,26 +122,8 @@
(is (= "Sorted by Count ascending"
(describe-legacy-query-order-by query)))))
-(deftest ^:parallel describe-order-by-expression-reference-test
- ;; it("should work with expressions", () => {
- ;; const query = {
- ;; "source-table": PRODUCTS.id,
- ;; expressions: {
- ;; Foo: ["concat", "Foo ", ["field", 4, null]],
- ;; },
- ;; "order-by": [["asc", ["expression", "Foo", null]]],
- ;; };
- ;; expect(base_question._getOrderByDescription(PRODUCTS, query)).toEqual([
- ;; "Sorted by ",
- ;; ["Foo ascending"],
- ;; ]);
- ;; });
- ;; });
-
- )
-
(deftest ^:parallel orderable-columns-breakouts-test
- (testing "If query has aggregations and/or breakouts, you can only order by those."
+ (testing "If query has aggregations and/or breakouts you can only order by those."
(let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
(lib/aggregate (lib/sum (lib/field "VENUES" "PRICE")))
(lib/aggregate (lib/avg (lib/+ (lib/field "VENUES" "PRICE") 1)))
@@ -153,11 +135,8 @@
:table_id (meta/id :venues)
:name "CATEGORY_ID"
:has_field_values :none
- :source :breakout
+ :lib/source :source/breakouts
:fk_target_field_id (meta/id :categories :id)
- :field_ref [:field
- {:base-type :type/Integer, :lib/uuid string?}
- (meta/id :venues :category-id)]
:effective_type :type/Integer
:id (meta/id :venues :category-id)
:display_name "Category ID"
@@ -166,28 +145,25 @@
:base_type :type/Integer
:name "sum_price"
:display_name "Sum of Price"
- :field_ref [:aggregation {:lib/uuid string?, :base-type :type/Integer} 0]
- :source :aggregation}
+ :lib/source :source/aggregations}
{:lib/type :metadata/field
:base_type :type/Float
:name "avg_price_plus_1"
:display_name "Average of Price + 1"
- :field_ref [:aggregation {:lib/uuid string?, :base-type :type/Float} 1]
- :source :aggregation}]
+ :lib/source :source/aggregations}]
(lib/orderable-columns query)))))))
(deftest ^:parallel orderable-columns-breakouts-with-expression-test
- (testing "If query has aggregations and/or breakouts, you can only order by those (with an expression)"
+ (testing "If query has aggregations and/or breakouts you can only order by those (with an expression)"
(let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
(lib/expression "Category ID + 1" (lib/+ (lib/field "VENUES" "CATEGORY_ID") 1))
(lib/breakout [:expression {:lib/uuid (str (random-uuid))} "Category ID + 1"]))]
(testing (lib.util/format "Query =\n%s" (u/pprint-to-str query))
(is (=? [{:lib/type :metadata/field
- :field_ref [:expression {:lib/uuid string?} "Category ID + 1"]
:name "Category ID + 1"
:display_name "Category ID + 1"
:base_type :type/Integer
- :source :breakout}]
+ :lib/source :source/breakouts}]
(lib/orderable-columns query)))))))
(deftest ^:parallel orderable-columns-test
@@ -198,60 +174,48 @@
:display_name "ID"
:id (meta/id :venues :id)
:table_id (meta/id :venues)
- :base_type :type/BigInteger
- :field_ref [:field {:lib/uuid string?, :base-type :type/BigInteger} (meta/id :venues :id)]}
+ :base_type :type/BigInteger}
{:lib/type :metadata/field
:name "NAME"
:display_name "Name"
:id (meta/id :venues :name)
:table_id (meta/id :venues)
- :base_type :type/Text
- :field_ref [:field {:lib/uuid string?, :base-type :type/Text} (meta/id :venues :name)]}
+ :base_type :type/Text}
{:lib/type :metadata/field
:name "CATEGORY_ID"
:display_name "Category ID"
:id (meta/id :venues :category-id)
- :table_id (meta/id :venues)
- :field_ref [:field {:lib/uuid string?, :base-type :type/Integer} (meta/id :venues :category-id)]}
+ :table_id (meta/id :venues)}
{:lib/type :metadata/field
:name "LATITUDE"
:display_name "Latitude"
:id (meta/id :venues :latitude)
:table_id (meta/id :venues)
- :base_type :type/Float
- :field_ref [:field {:lib/uuid string?, :base-type :type/Float} (meta/id :venues :latitude)]}
+ :base_type :type/Float}
{:lib/type :metadata/field
:name "LONGITUDE"
:display_name "Longitude"
:id (meta/id :venues :longitude)
:table_id (meta/id :venues)
- :base_type :type/Float
- :field_ref [:field {:lib/uuid string?, :base-type :type/Float} (meta/id :venues :longitude)]}
+ :base_type :type/Float}
{:lib/type :metadata/field
:name "PRICE"
:display_name "Price"
:id (meta/id :venues :price)
:table_id (meta/id :venues)
- :base_type :type/Integer
- :field_ref [:field {:lib/uuid string?, :base-type :type/Integer} (meta/id :venues :price)]}
+ :base_type :type/Integer}
{:lib/type :metadata/field
:name "ID"
:display_name "ID"
:id (meta/id :categories :id)
:table_id (meta/id :categories)
- :base_type :type/BigInteger
- :field_ref [:field
- {:lib/uuid string?, :base-type :type/BigInteger, :source-field (meta/id :venues :category-id)}
- (meta/id :categories :id)]}
+ :base_type :type/BigInteger}
{:lib/type :metadata/field
:name "NAME"
:display_name "Name"
:id (meta/id :categories :name)
:table_id (meta/id :categories)
- :base_type :type/Text
- :field_ref [:field
- {:lib/uuid string?, :base-type :type/Text, :source-field (meta/id :venues :category-id)}
- (meta/id :categories :name)]}]
+ :base_type :type/Text}]
(lib/orderable-columns query))))))
(deftest ^:parallel orderable-expressions-test
@@ -261,20 +225,17 @@
(testing (lib.util/format "Query =\n%s" (u/pprint-to-str query))
(is (=? [{:lib/type :metadata/field
:base_type :type/Integer
- :name "category_id_plus_1"
+ :name "Category ID + 1"
:display_name "Category ID + 1"
- :field_ref [:expression
- {:lib/uuid string?, :base-type :type/Integer}
- "Category ID + 1"]
- :source :expressions}
- {:id (meta/id :venues :id), :name "ID"}
- {:id (meta/id :venues :name), :name "NAME"}
- {:id (meta/id :venues :category-id), :name "CATEGORY_ID"}
- {:id (meta/id :venues :latitude), :name "LATITUDE"}
- {:id (meta/id :venues :longitude), :name "LONGITUDE"}
- {:id (meta/id :venues :price), :name "PRICE"}
- {:id (meta/id :categories :id), :name "ID"}
- {:id (meta/id :categories :name), :name "NAME"}]
+ :lib/source :source/expressions}
+ {:id (meta/id :venues :id) :name "ID"}
+ {:id (meta/id :venues :name) :name "NAME"}
+ {:id (meta/id :venues :category-id) :name "CATEGORY_ID"}
+ {:id (meta/id :venues :latitude) :name "LATITUDE"}
+ {:id (meta/id :venues :longitude) :name "LONGITUDE"}
+ {:id (meta/id :venues :price) :name "PRICE"}
+ {:id (meta/id :categories :id) :name "ID"}
+ {:id (meta/id :categories :name) :name "NAME"}]
(lib/orderable-columns query)))))))
(deftest ^:parallel orderable-expressions-exclude-boolean-expressions-test
@@ -282,14 +243,14 @@
(let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
(lib/expression "Name is empty?" (lib/is-empty (lib/field "VENUES" "NAME"))))]
(testing (lib.util/format "Query =\n%s" (u/pprint-to-str query))
- (is (=? [{:id (meta/id :venues :id), :name "ID"}
- {:id (meta/id :venues :name), :name "NAME"}
- {:id (meta/id :venues :category-id), :name "CATEGORY_ID"}
- {:id (meta/id :venues :latitude), :name "LATITUDE"}
- {:id (meta/id :venues :longitude), :name "LONGITUDE"}
- {:id (meta/id :venues :price), :name "PRICE"}
- {:id (meta/id :categories :id), :name "ID"}
- {:id (meta/id :categories :name), :name "NAME"}]
+ (is (=? [{:id (meta/id :venues :id) :name "ID"}
+ {:id (meta/id :venues :name) :name "NAME"}
+ {:id (meta/id :venues :category-id) :name "CATEGORY_ID"}
+ {:id (meta/id :venues :latitude) :name "LATITUDE"}
+ {:id (meta/id :venues :longitude) :name "LONGITUDE"}
+ {:id (meta/id :venues :price) :name "PRICE"}
+ {:id (meta/id :categories :id) :name "ID"}
+ {:id (meta/id :categories :name) :name "NAME"}]
(lib/orderable-columns query)))))))
(deftest ^:parallel orderable-explicit-joins-test
@@ -303,32 +264,26 @@
(lib/with-join-alias "Cat")
(lib/with-join-fields :all))))]
(testing (lib.util/format "Query =\n%s" (u/pprint-to-str query))
- (is (=? [{:id (meta/id :venues :id), :name "ID"}
- {:id (meta/id :venues :name), :name "NAME"}
- {:id (meta/id :venues :category-id), :name "CATEGORY_ID"}
- {:id (meta/id :venues :latitude), :name "LATITUDE"}
- {:id (meta/id :venues :longitude), :name "LONGITUDE"}
- {:id (meta/id :venues :price), :name "PRICE"}
+ (is (=? [{:id (meta/id :venues :id) :name "ID"}
+ {:id (meta/id :venues :name) :name "NAME"}
+ {:id (meta/id :venues :category-id) :name "CATEGORY_ID"}
+ {:id (meta/id :venues :latitude) :name "LATITUDE"}
+ {:id (meta/id :venues :longitude) :name "LONGITUDE"}
+ {:id (meta/id :venues :price) :name "PRICE"}
{:lib/type :metadata/field
:name "ID"
:display_name "Categories → ID" ; should we be using the explicit alias we gave this join?
:source_alias "Cat"
:id (meta/id :categories :id)
:table_id (meta/id :categories)
- :base_type :type/BigInteger
- :field_ref [:field
- {:lib/uuid string?, :base-type :type/BigInteger, :join-alias "Cat"}
- (meta/id :categories :id)]}
+ :base_type :type/BigInteger}
{:lib/type :metadata/field
:name "NAME"
:display_name "Categories → Name"
:source_alias "Cat"
:id (meta/id :categories :name)
:table_id (meta/id :categories)
- :base_type :type/Text
- :field_ref [:field
- {:lib/uuid string?, :base-type :type/Text, :join-alias "Cat"}
- (meta/id :categories :name)]}]
+ :base_type :type/Text}]
(lib/orderable-columns query)))))))
(deftest ^:parallel orderable-columns-source-metadata-test
@@ -336,23 +291,17 @@
(let [query (lib.tu/query-with-card-source-table)]
(testing (lib.util/format "Query =\n%s" (u/pprint-to-str query))
(is (=? [{:name "ID"
- :base_type :type/BigInteger
- :field_ref [:field {:lib/uuid string?, :base-type :type/BigInteger} "ID"]}
+ :base_type :type/BigInteger}
{:name "NAME"
- :base_type :type/Text
- :field_ref [:field {:lib/uuid string?, :base-type :type/Text} "NAME"]}
+ :base_type :type/Text}
{:name "CATEGORY_ID"
- :base_type :type/Integer
- :field_ref [:field {:lib/uuid string?, :base-type :type/Integer} "CATEGORY_ID"]}
+ :base_type :type/Integer}
{:name "LATITUDE"
- :base_type :type/Float
- :field_ref [:field {:lib/uuid string?, :base-type :type/Float} "LATITUDE"]}
+ :base_type :type/Float}
{:name "LONGITUDE"
- :base_type :type/Float
- :field_ref [:field {:lib/uuid string?, :base-type :type/Float} "LONGITUDE"]}
+ :base_type :type/Float}
{:name "PRICE"
- :base_type :type/Integer
- :field_ref [:field {:lib/uuid string?, :base-type :type/Integer} "PRICE"]}]
+ :base_type :type/Integer}]
(lib/orderable-columns query)))))))
(deftest ^:parallel orderable-columns-e2e-test
@@ -375,9 +324,73 @@
:lib/options {:lib/uuid string?}
:order-by [[:asc
{:lib/uuid string?}
- [:field {:lib/uuid string?, :base-type :type/Text} (meta/id :venues :name)]]]}]}
+ [:field {:lib/uuid string? :base-type :type/Text} (meta/id :venues :name)]]]}]}
query'))
(is (=? [[:asc
{:lib/uuid string?}
- [:field {:lib/uuid string?, :base-type :type/Text} (meta/id :venues :name)]]]
+ [:field {:lib/uuid string? :base-type :type/Text} (meta/id :venues :name)]]]
(lib/order-bys query'))))))))
+
+(deftest ^:parallel order-bys-with-duplicate-column-names-test
+ (testing "Order by stuff should work with two different columns named ID (#29702)"
+ (is (=? [{:id (meta/id :venues :id)
+ :name "ID"
+ :lib/source :source/previous-stage
+ :lib/type :metadata/field
+ :base_type :type/BigInteger
+ :effective_type :type/BigInteger
+ :display_name "ID"
+ :table_id (meta/id :venues)}
+ {:id (meta/id :categories :id)
+ :name "ID_2"
+ :lib/source :source/previous-stage
+ :lib/type :metadata/field
+ :base_type :type/BigInteger
+ :effective_type :type/BigInteger
+ :display_name "ID"
+ :table_id (meta/id :categories)}]
+ (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+ (lib/join (-> (lib/join-clause
+ (meta/table-metadata :categories)
+ (lib/=
+ (lib/field "VENUES" "CATEGORY_ID")
+ (lib/field "CATEGORIES" "ID")))
+ (lib/with-join-fields :all)))
+ (lib/fields [(lib/field "VENUES" "ID") (lib/field "CATEGORIES" "ID")])
+ (lib/append-stage)
+ (lib/orderable-columns))))))
+
+(deftest ^:parallel orderable-columns-include-expressions-test
+ (testing "orderable-columns should include expressions"
+ (is (=? [{:lib/type :metadata/field
+ :name "expr"
+ :display_name "expr"
+ :lib/source :source/expressions}
+ {:name "ID"}
+ {:name "NAME"}
+ {:name "CATEGORY_ID"}
+ {:name "LATITUDE"}
+ {:name "LONGITUDE"}
+ {:name "PRICE"}
+ {:name "ID"}
+ {:name "NAME"}]
+ (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+ (lib/expression "expr" (lib/absolute-datetime "2020" :month))
+ (lib/fields [(lib/field "VENUES" "ID")])
+ (lib/orderable-columns))))))
+
+(deftest ^:parallel order-by-expression-test
+ (let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+ (lib/expression "expr" (lib/absolute-datetime "2020" :month))
+ (lib/fields [(lib/field "VENUES" "ID")]))
+ [expr] (lib/orderable-columns query)]
+ (is (=? {:lib/type :metadata/field
+ :lib/source :source/expressions
+ :name "expr"}
+ expr))
+ (let [updated-query (lib/order-by query expr)]
+ (is (=? {:stages [{:order-by [[:asc {} [:expression {} "expr"]]]}]}
+ updated-query))
+ (testing "description"
+ (is (= "Venues, Sorted by expr ascending"
+ (lib/describe-query updated-query)))))))
diff --git a/test/metabase/lib/stage_test.cljc b/test/metabase/lib/stage_test.cljc
index 9a7885ccfc381bd8af8947591e72482ea3e7bdcc..5ff8d19cadd4a874d72a7647fc8550d2e51a81f7 100644
--- a/test/metabase/lib/stage_test.cljc
+++ b/test/metabase/lib/stage_test.cljc
@@ -26,8 +26,7 @@
:lib/metadata meta/metadata-provider}]
(is (mc/validate ::lib.schema/query query))
(is (=? [(merge (meta/field-metadata :venues :price)
- {:source :fields
- :field_ref [:field {:lib/uuid string?} (meta/id :venues :price)]})]
+ {:lib/source :source/fields})]
(lib.metadata.calculation/metadata query -1 query))))))
(deftest ^:parallel deduplicate-expression-names-in-aggregations-test
@@ -44,12 +43,10 @@
[:avg {} (lib.tu/field-clause :venues :price)]]]})]
(is (=? [{:base_type :type/Float
:name "0_9_times_avg_price"
- :display_name "0.9 × Average of Price"
- :field_ref [:aggregation {:lib/uuid string?} 0]}
+ :display_name "0.9 × Average of Price"}
{:base_type :type/Float
:name "0_8_times_avg_price"
- :display_name "0.8 × Average of Price"
- :field_ref [:aggregation {:lib/uuid string?} 1]}]
+ :display_name "0.8 × Average of Price"}]
(lib.metadata.calculation/metadata query -1 query)))))))
(deftest ^:parallel stage-display-name-card-source-query
diff --git a/test/metabase/lib/test_metadata.cljc b/test/metabase/lib/test_metadata.cljc
index 8b54a78a42e296cf4fa9eb684a0ebee8dc8b9387..451dd6d07c854cd00edd5af9f5a7145be573f43c 100644
--- a/test/metabase/lib/test_metadata.cljc
+++ b/test/metabase/lib/test_metadata.cljc
@@ -801,7 +801,6 @@
{:lib/type :metadata/results
:columns [{:lib/type :metadata/field
:display_name "ID"
- :field_ref [:field "ID" {:base-type :type/BigInteger}]
:name "ID"
:base_type :type/BigInteger
:effective_type :type/BigInteger
@@ -809,7 +808,6 @@
:fingerprint nil}
{:lib/type :metadata/field
:display_name "NAME" ; TODO -- these display names are icky
- :field_ref [:field "NAME" {:base-type :type/Text}]
:name "NAME"
:base_type :type/Text
:effective_type :type/Text
@@ -822,7 +820,6 @@
:average-length 15.63}}}}
{:lib/type :metadata/field
:display_name "CATEGORY_ID"
- :field_ref [:field "CATEGORY_ID" {:base-type :type/Integer}]
:name "CATEGORY_ID"
:base_type :type/Integer
:effective_type :type/Integer
@@ -832,7 +829,6 @@
{:min 2.0, :q1 6.89564392373896, :q3 49.240253073352044, :max 74.0, :sd 23.058108414099443, :avg 29.98}}}}
{:lib/type :metadata/field
:display_name "LATITUDE"
- :field_ref [:field "LATITUDE" {:base-type :type/Float}]
:name "LATITUDE"
:base_type :type/Float
:effective_type :type/Float
@@ -847,7 +843,6 @@
:avg 35.505891999999996}}}}
{:lib/type :metadata/field
:display_name "LONGITUDE"
- :field_ref [:field "LONGITUDE" {:base-type :type/Float}]
:name "LONGITUDE"
:base_type :type/Float
:effective_type :type/Float
@@ -862,7 +857,6 @@
:avg -115.99848699999998}}}}
{:lib/type :metadata/field
:display_name "PRICE"
- :field_ref [:field "PRICE" {:base-type :type/Integer}]
:name "PRICE"
:base_type :type/Integer
:effective_type :type/Integer