diff --git a/shared/src/metabase/shared/parameters/parameters.cljc b/shared/src/metabase/shared/parameters/parameters.cljc index 904f72f4518abb09e96b0cef9a82e8c5f46c0bea..6c3584ff42c94457e1e9e455bc800a827329b0af 100644 --- a/shared/src/metabase/shared/parameters/parameters.cljc +++ b/shared/src/metabase/shared/parameters/parameters.cljc @@ -66,6 +66,7 @@ (formatted-value :date/single end locale)) ""))) +;;; TODO -- sorta duplicated with [[metabase.lib.metadata.calculate.display-name/interval-display-name]] but not exactly (defn- translated-interval [interval n] (case interval diff --git a/shared/src/metabase/shared/util/i18n.clj b/shared/src/metabase/shared/util/i18n.clj index 3a8e38593757f10f32fa818544007436c2a64104..1f07d0f93c9067fb485eff348dda22260c3cd0da 100644 --- a/shared/src/metabase/shared/util/i18n.clj +++ b/shared/src/metabase/shared/util/i18n.clj @@ -34,10 +34,22 @@ :cljs `(js-i18n ~format-string ~@args))) +(defmacro trun + "i18n a string with both singular and plural forms, using the current user's locale. The appropriate plural form will + be returned based on the value of `n`. `n` can be interpolated into the format strings using the `{0}` + syntax. (Other placeholders are not supported)." + [format-string format-string-pl n] + (macros/case + :clj + `(i18n/trun ~format-string ~format-string-pl ~n) + + :cljs + `(js-i18n-n ~format-string ~format-string-pl ~n))) + (defmacro trsn "i18n a string with both singular and plural forms, using the site's locale. The appropriate plural form will be returned based on the value of `n`. `n` can be interpolated into the format strings using the `{0}` syntax. (Other - placeholders are not supported). " + placeholders are not supported)." [format-string format-string-pl n] (macros/case :clj diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc index 2816e045e1f35c4fdb94d06bbb16e35b534ca42b..6dc96ebc3969357452e9ba474bdda302bbeb5d9c 100644 --- a/src/metabase/lib/core.cljc +++ b/src/metabase/lib/core.cljc @@ -4,6 +4,7 @@ (:refer-clojure :exclude [remove replace =]) (:require [metabase.lib.dev :as lib.dev] + [metabase.lib.field :as lib.field] [metabase.lib.filter :as lib.filter] [metabase.lib.join :as lib.join] [metabase.lib.order-by :as lib.order-by] @@ -12,6 +13,7 @@ [metabase.shared.util.namespaces :as shared.ns])) (comment lib.dev/keep-me + lib.field/keep-me lib.filter/keep-me lib.join/keep-me lib.order-by/keep-me @@ -22,6 +24,8 @@ [lib.dev field query-for-table-name] + [lib.field + with-join-alias] [lib.filter =] [lib.join diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc index f10cc3f5a53fef4018916739f17b4dfa316b3509..cf906f975aac9971d86cc846d7db2fe1be5d0417 100644 --- a/src/metabase/lib/field.cljc +++ b/src/metabase/lib/field.cljc @@ -47,3 +47,11 @@ (->field query -1 x)) ([query stage-number x] (->field query stage-number x))) + +(defn with-join-alias + "Update a `field` so that it has `join-alias`." + [field-or-fn join-alias] + (if (fn? field-or-fn) + (fn [query stage-number] + (with-join-alias (field-or-fn query stage-number) join-alias)) + (lib.options/update-options field-or-fn assoc :join-alias join-alias))) diff --git a/src/metabase/lib/join.cljc b/src/metabase/lib/join.cljc index 8c4964c9c65c619bd7ac2d0365378c0f4f9ecedc..581f22db24122a0a94a352f727ea3b3d50c12e46 100644 --- a/src/metabase/lib/join.cljc +++ b/src/metabase/lib/join.cljc @@ -17,6 +17,10 @@ ;; TODO -- should the default implementation call [[metabase.lib.query/query]]? That way if we implement a method to ;; create an MBQL query from a `Table`, then we'd also get [[join]] support for free? +(defmethod ->join-clause :mbql/join + [_query _stage-number a-join-clause] + a-join-clause) + (defmethod ->join-clause :mbql/query [_query _stage-number another-query] (-> {:lib/type :mbql/join @@ -29,6 +33,14 @@ :stages [mbql-stage]} lib.options/ensure-uuid)) +(defmethod ->join-clause :metadata/table + [query stage-number table-metadata] + (->join-clause query + stage-number + {:lib/type :mbql.stage/mbql + :lib/options {:lib/uuid (str (random-uuid))} + :source-table (:id table-metadata)})) + (defmethod ->join-clause :dispatch-type/fn [query stage-number f] (->join-clause query stage-number (f query stage-number))) @@ -83,6 +95,9 @@ `condition` is currently required, but in the future I think we should make this smarter and try to infer a sensible default condition for things, e.g. when joining a Table B from Table A, if there is an FK relationship between A and B, join via that relationship. Not yet implemented!" + ([query a-join-clause] + (join query -1 a-join-clause (:condition a-join-clause))) + ([query x condition] (join query -1 x condition)) diff --git a/src/metabase/lib/metadata/calculate.cljc b/src/metabase/lib/metadata/calculate.cljc new file mode 100644 index 0000000000000000000000000000000000000000..01eaf031b998c03a72ef5647055996e5564622e3 --- /dev/null +++ b/src/metabase/lib/metadata/calculate.cljc @@ -0,0 +1,257 @@ +(ns metabase.lib.metadata.calculate + (:refer-clojure :exclude [ref]) + (:require + [metabase.lib.dispatch :as lib.dispatch] + [metabase.lib.metadata :as lib.metadata] + [metabase.lib.metadata.calculate.names :as calculate.names] + [metabase.lib.metadata.calculate.resolve :as calculate.resolve] + [metabase.lib.schema :as lib.schema] + [metabase.lib.schema.aggregation :as lib.schema.aggregation] + [metabase.lib.schema.common :as lib.schema.common] + [metabase.lib.schema.expression :as lib.schema.expresssion] + [metabase.lib.schema.id :as lib.schema.id] + [metabase.lib.schema.join :as lib.schema.join] + [metabase.lib.util :as lib.util] + [metabase.mbql.util :as mbql.u] + [metabase.util :as u] + [metabase.util.malli :as mu])) + +(declare stage-metadata) + +(mu/defn ^:private add-parent-column-metadata + "If this is a nested column, add metadata about the parent column." + [query :- ::lib.schema/query + metadata :- lib.metadata/ColumnMetadata] + (let [parent-metadata (lib.metadata/field query (:parent_id metadata)) + {parent-name :name} (cond->> parent-metadata + (:parent_id parent-metadata) (add-parent-column-metadata query))] + (update metadata :name (fn [field-name] + (str parent-name \. field-name))))) + +(defmulti ^:private metadata-for-ref + {:arglists '([query stage-number ref])} + (fn [_query _stage-number ref] + (lib.dispatch/dispatch-value ref))) + +(defmethod metadata-for-ref :field + [query stage-number [_tag opts :as field-ref]] + (let [metadata (merge + {:lib/type :metadata/field + :field_ref field-ref} + (calculate.resolve/field-metadata query stage-number field-ref) + {:display_name (calculate.names/display-name query stage-number field-ref)} + (when (:base-type opts) + {:base_type (:base-type opts)}))] + (cond->> metadata + (:parent_id metadata) (add-parent-column-metadata query)))) + +(defmethod metadata-for-ref :expression + [query stage-number [_expression opts expression-name, :as expression-ref]] + (let [expression (calculate.resolve/expression query stage-number expression-name)] + {:lib/type :metadata/field + :field_ref expression-ref + :name expression-name + :display_name (calculate.names/display-name query stage-number expression-ref) + :base_type (or (:base-type opts) + (lib.schema.expresssion/type-of expression))})) + +(mu/defn ^:private metadata-for-aggregation :- lib.metadata/ColumnMetadata + [query :- ::lib.schema/query + stage-number :- :int + aggregation :- ::lib.schema.aggregation/aggregation + index :- ::lib.schema.common/int-greater-than-or-equal-to-zero] + (let [display-name (calculate.names/display-name query stage-number aggregation)] + {:lib/type :metadata/field + :source :aggregation + :base_type (lib.schema.expresssion/type-of aggregation) + :field_ref [:aggregation {:lib/uuid (str (random-uuid))} index] + :name (calculate.names/column-name query stage-number aggregation) + :display_name display-name})) + +(defmethod metadata-for-ref :aggregation + [query stage-number [_ag opts index, :as aggregation-ref]] + (let [aggregation (calculate.resolve/aggregation query stage-number index)] + (merge + (metadata-for-aggregation query stage-number aggregation index) + {:field_ref aggregation-ref} + (when (:base-type opts) + {:base_type (:base-type opts)})))) + +(mu/defn ^:private breakout-columns :- [:maybe [:sequential lib.metadata/ColumnMetadata]] + [query :- ::lib.schema/query + stage-number :- :int] + (when-let [{breakouts :breakout} (lib.util/query-stage query stage-number)] + (mapv (fn [ref] + (assoc (metadata-for-ref query stage-number ref) :source :breakout)) + breakouts))) + +(mu/defn ^:private aggregation-columns :- [:maybe [:sequential lib.metadata/ColumnMetadata]] + [query :- ::lib.schema/query + stage-number :- :int] + (when-let [{aggregations :aggregation} (lib.util/query-stage query stage-number)] + (map-indexed (fn [i aggregation] + (metadata-for-aggregation query stage-number aggregation i)) + aggregations))) + +(mu/defn ^:private fields-columns :- [:maybe [:sequential lib.metadata/ColumnMetadata]] + [query :- ::lib.schema/query + stage-number :- :int] + (when-let [{fields :fields} (lib.util/query-stage query stage-number)] + (mapv (fn [ref] + (assoc (metadata-for-ref query stage-number ref) :source :fields)) + fields))) + +(defn- remove-hidden-default-fields + "Remove Fields that shouldn't be visible from the default Fields for a source Table. + See [[metabase.query-processor.middleware.add-implicit-clauses/table->sorted-fields*]]." + [field-metadatas] + (remove (fn [{visibility-type :visibility_type, active? :active, :as _field-metadata}] + (or (false? active?) + (#{:sensitive :retired} (some-> visibility-type keyword)))) + field-metadatas)) + +(defn- sort-default-fields + "Sort default Fields for a source Table. See [[metabase.models.table/field-order-rule]]." + [field-metadatas] + (sort-by (fn [{field-name :name, :keys [position], :as _field-metadata}] + [(or position 0) (u/lower-case-en (or field-name ""))]) + field-metadatas)) + +(mu/defn ^:private source-table-default-fields :- [:maybe [:sequential lib.metadata/ColumnMetadata]] + "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 + table-id :- ::lib.schema.id/table] + (when-let [field-metadatas (lib.metadata/fields query table-id)] + (->> field-metadatas + remove-hidden-default-fields + sort-default-fields))) + +(mu/defn ^:private default-join-alias :- ::lib.schema.common/non-blank-string + "Generate an alias for a join that doesn't already have one." + [query :- ::lib.schema/query + stage-number :- :int + join :- ::lib.schema.join/join] + (calculate.names/display-name query stage-number join)) + +(def ^:private JoinsWithAliases + "Schema for a sequence of joins that all have aliases." + [:and + ::lib.schema.join/joins + [:sequential + [:map + [:alias ::lib.schema.common/non-blank-string]]]]) + +(mu/defn ^:private ensure-all-joins-have-aliases :- JoinsWithAliases + "Make sure all the joins in a query have an `:alias` if they don't already have one." + [query :- ::lib.schema/query + stage-number :- :int + joins :- ::lib.schema.join/joins] + (let [unique-name-generator (mbql.u/unique-name-generator)] + (mapv (fn [join] + (cond-> join + (not (:alias join)) (assoc :alias (unique-name-generator (default-join-alias query stage-number join))))) + joins))) + +(mu/defn ^:private column-from-join-fields :- lib.metadata/ColumnMetadata + "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 (calculate.names/display-name query stage-number column-metadata)))) + +(mu/defn ^:private default-columns-added-by-join :- [:sequential lib.metadata/ColumnMetadata] + [query :- ::lib.schema/query + stage-number :- :int + {:keys [fields stages], join-alias :alias, :or {fields :none}, :as _join} :- ::lib.schema.join/join] + (when-not (= fields :none) + (let [field-metadatas (if (= fields :all) + (stage-metadata (assoc query :stages stages)) + (for [field-ref fields] + ;; resolve the field ref in the context of the join. Not sure if this is right. + (calculate.resolve/field-metadata query stage-number field-ref)))] + (mapv (fn [field-metadata] + (column-from-join-fields query stage-number field-metadata join-alias)) + field-metadatas)))) + +(mu/defn ^:private default-columns-added-by-joins :- [:maybe [:sequential lib.metadata/ColumnMetadata]] + [query :- ::lib.schema/query + stage-number :- :int] + (when-let [joins (not-empty (:joins (lib.util/query-stage query stage-number)))] + (not-empty + (into [] + (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 {:min 1} lib.metadata/ColumnMetadata] + "Calculate the columns to return if `:aggregations`/`:breakout`/`:fields` are unspecified. + + Formula for the so-called 'default' columns is + + 1a. Columns returned by the previous stage of the query (if there is one), OR + + 1b. Default 'visible' Fields for our `:source-table`, OR + + 1c. `:lib/stage-metadata` if this is a `:mbql.stage/native` stage + + PLUS + + 2. Columns added by joins at this stage" + [query :- ::lib.schema/query + stage-number :- :int] + (concat + ;; 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) + ;; 1b or 1c + (let [{:keys [source-table], :as this-stage} (lib.util/query-stage query stage-number)] + (if (integer? source-table) + ;; 1b: default visible Fields for the source Table + (source-table-default-fields query source-table) + ;; 1c: `:lib/stage-metadata` for the native query + (:columns (:lib/stage-metadata this-stage))))) + ;; 2: columns added by joins at this stage + (default-columns-added-by-joins query stage-number))) + +(mu/defn 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)))]] + "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-metadata query -1)) + + ([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 + [(breakout-columns query stage-number) + (aggregation-columns query stage-number) + (fields-columns query stage-number)])) + (default-columns query stage-number)))))) diff --git a/src/metabase/lib/metadata/calculate/names.cljc b/src/metabase/lib/metadata/calculate/names.cljc new file mode 100644 index 0000000000000000000000000000000000000000..50068c643da7313f698499dcecfdcb0f022b4844 --- /dev/null +++ b/src/metabase/lib/metadata/calculate/names.cljc @@ -0,0 +1,355 @@ +(ns metabase.lib.metadata.calculate.names + "Logic for calculating human-friendly display names for things." + (:require + [clojure.string :as str] + [metabase.lib.dispatch :as lib.dispatch] + [metabase.lib.metadata :as lib.metadata] + [metabase.lib.metadata.calculate.resolve :as calculate.resolve] + [metabase.lib.schema :as lib.schema] + [metabase.lib.schema.common :as lib.schema.common] + [metabase.lib.schema.temporal-bucketing :as lib.schema.temporal-bucketing] + [metabase.shared.util.i18n :as i18n] + [metabase.util :as u] + [metabase.util.humanization :as u.humanization] + [metabase.util.malli :as mu] + #?@(:cljs ([goog.string :refer [format]] + [goog.string.format :as gstring.format])))) + +;; The formatting functionality is only loaded if you depend on goog.string.format. +#?(:cljs (comment gstring.format/keep-me)) + +(defmulti ^:private display-name* + "Impl for [[display-name]]." + {:arglists '([query stage-number x])} + (fn [_query _stage-number x] + (lib.dispatch/dispatch-value x))) + +(defn- options-when-mbql-clause + "If this is an MBQL clause, return its options map, if it has one." + [x] + (when (and (vector? x) + (keyword? (first x)) + (map? (second x))) + (second x))) + +(mu/defn display-name :- ::lib.schema.common/non-blank-string + "Calculate a nice human-friendly display name for something." + [query :- ::lib.schema/query + stage-number :- :int + x] + (or + ;; if this is an MBQL clause with `:display-name` in the options map, then use that rather than calculating a name. + (:display-name (options-when-mbql-clause x)) + (try + (display-name* query stage-number x) + (catch #?(:clj Throwable :cljs js/Error) e + (throw (ex-info (i18n/tru "Error calculating display name for {0}: {1}" (pr-str x) (ex-message e)) + {:x x + :query query + :stage-number stage-number} + e)))))) + +(defmulti ^:private column-name* + "Impl for [[column-name]]." + {:arglists '([query stage-number x])} + (fn [_query _stage-number x] + (lib.dispatch/dispatch-value x))) + +(mu/defn column-name :- ::lib.schema.common/non-blank-string + "Calculate a database-friendly name to use for an expression." + [query :- ::lib.schema/query + stage-number :- :int + x] + (or + ;; if this is an MBQL clause with `:name` in the options map, then use that rather than calculating a name. + (:name (options-when-mbql-clause x)) + (try + (column-name* query stage-number x) + (catch #?(:clj Throwable :cljs js/Error) e + (throw (ex-info (i18n/tru "Error calculating column name for {0}: {1}" (pr-str x) (ex-message e)) + {:x x + :query query + :stage-number stage-number} + e)))))) + +(defn- slugify [s] + (-> s + (str/replace #"\+" (i18n/tru "plus")) + (str/replace #"\-" (i18n/tru "minus")) + (str/replace #"[\(\)]" "") + u/slugify)) + +;;; default impl just takes the display name and slugifies it. +(defmethod column-name* :default + [query stage-number x] + (slugify (display-name query stage-number x))) + +(defmethod display-name* :mbql/join + [query _stage-number {[first-stage] :stages, :as _join}] + (if-let [source-table (:source-table first-stage)] + (if (integer? source-table) + (:display_name (lib.metadata/table query source-table)) + ;; handle card__<id> source tables. + (let [[_ card-id-str] (re-matches #"^card__(\d+)$" source-table)] + (i18n/tru "Saved Question #{0}" card-id-str))) + (i18n/tru "Native Query"))) + +(defmethod display-name* :metadata/field + [query stage-number {field-display-name :display_name, field-name :name, join-alias :source_alias, :as _field-metadata}] + (let [field-display-name (or field-display-name + (u.humanization/name->human-readable-name :simple field-name)) + join-display-name (when join-alias + (let [join (calculate.resolve/join query stage-number join-alias)] + (display-name query stage-number join)))] + (if join-display-name + (str join-display-name " → " field-display-name) + field-display-name))) + +(defmethod display-name* :field + [query stage-number [_field {:keys [join-alias], :as _opts} _id-or-name, :as field-clause]] + (let [field-metadata (cond-> (calculate.resolve/field-metadata query stage-number field-clause) + join-alias (assoc :source_alias join-alias))] + (display-name query stage-number field-metadata))) + +(defmethod display-name* :expression + [_query _stage-number [_expression _opts expression-name]] + expression-name) + +(defmethod column-name* :expression + [_query _stage-number [_expression _opts expression-name]] + expression-name) + +(def ^:private ^:dynamic *nested* + "Whether the display name we are generated is recursively nested inside another display name. For infix math operators + we'll wrap the results in parentheses to make the display name more obvious." + false) + +(defn- wrap-str-in-parens-if-nested [s] + (if *nested* + (str \( s \)) + s)) + +(defn- infix-display-name* + "Generate a infix-style display name for an arithmetic expression like `:+`, e.g. `x + y`." + [query stage-number operator args] + (wrap-str-in-parens-if-nested + (binding [*nested* true] + (str/join (str \space (name operator) \space) + (map (partial display-name* query stage-number) + args))))) + +(defmethod display-name* :+ + [query stage-number [_plus _opts & args]] + (infix-display-name* query stage-number "+" args)) + +(defmethod display-name* :- + [query stage-number [_minute _opts & args]] + (infix-display-name* query stage-number "-" args)) + +(defmethod display-name* :/ + [query stage-number [_divide _opts & args]] + (infix-display-name* query stage-number "÷" args)) + +(defmethod display-name* :* + [query stage-number [_multiply _opts & args]] + (infix-display-name* query stage-number "×" args)) + +(defn- infix-column-name* + [query stage-number operator-str args] + (str/join (str \_ operator-str \_) + (map (partial column-name* query stage-number) + args))) + +(defmethod column-name* :+ + [query stage-number [_plus _opts & args]] + (infix-column-name* query stage-number "plus" args)) + +(defmethod column-name* :- + [query stage-number [_minute _opts & args]] + (infix-column-name* query stage-number "minus" args)) + +(defmethod column-name* :/ + [query stage-number [_divide _opts & args]] + (infix-column-name* query stage-number "divided_by" args)) + +(defmethod column-name* :* + [query stage-number [_multiply _opts & args]] + (infix-column-name* query stage-number "times" args)) + +(defmethod display-name* :count + [query stage-number [_count _opts x]] + ;; x is optional. + (if x + (i18n/tru "Count of {0}" (display-name query stage-number x)) + (i18n/tru "Count"))) + +(defmethod column-name* :count + [query stage-number [_count _opts x]] + (if x + (str "count_" (column-name query stage-number x)) + "count")) + +(defmethod display-name* :case + [_query _stage-number _case] + (i18n/tru "Case")) + +(defmethod column-name* :case + [_query _stage-number _case] + "case") + +(defmethod display-name* :distinct + [query stage-number [_distinct _opts x]] + (i18n/tru "Distinct values of {0}" (display-name query stage-number x))) + +(defmethod column-name* :distinct + [query stage-number [_distinct _opts x]] + (str "distinct_" (column-name query stage-number x))) + +(defmethod display-name* :avg + [query stage-number [_avg _opts x]] + (i18n/tru "Average of {0}" (display-name query stage-number x))) + +(defmethod column-name* :avg + [query stage-number [_avg _opts x]] + (str "avg_" (column-name query stage-number x))) + +(defmethod display-name* :cum-count + [query stage-number [_cum-count _opts x]] + (i18n/tru "Cumulative count of {0}" (display-name query stage-number x))) + +(defmethod column-name* :cum-count + [query stage-number [_avg _opts x]] + (str "cum_count_" (column-name query stage-number x))) + +(defmethod display-name* :sum + [query stage-number [_sum _opts x]] + (i18n/tru "Sum of {0}" (display-name query stage-number x))) + +(defmethod column-name* :sum + [query stage-number [_sum _opts x]] + (str "sum_" (column-name query stage-number x))) + +(defmethod display-name* :cum-sum + [query stage-number [_cum-sum _opts x]] + (i18n/tru "Cumulative sum of {0}" (display-name query stage-number x))) + +(defmethod column-name* :cum-sum + [query stage-number [_avg _opts x]] + (str "cum_sum_" (column-name query stage-number x))) + +(defmethod display-name* :stddev + [query stage-number [_stddev _opts x]] + (i18n/tru "Standard deviation of {0}" (display-name query stage-number x))) + +(defmethod column-name* :stddev + [query stage-number [_avg _opts x]] + (str "std_dev_" (column-name query stage-number x))) + +(defmethod display-name* :min + [query stage-number [_min _opts x]] + (i18n/tru "Min of {0}" (display-name query stage-number x))) + +(defmethod column-name* :min + [query stage-number [_min _opts x]] + (str "min_" (column-name query stage-number x))) + +(defmethod display-name* :max + [query stage-number [_max _opts x]] + (i18n/tru "Max of {0}" (display-name query stage-number x))) + +(defmethod column-name* :max + [query stage-number [_max _opts x]] + (str "max_" (column-name query stage-number x))) + +(defmethod display-name* :var + [query stage-number [_var _opts x]] + (i18n/tru "Variance of {0}" (display-name query stage-number x))) + +(defmethod column-name* :var + [query stage-number [_var _opts x]] + (str "var_" (column-name query stage-number x))) + +(defmethod display-name* :median + [query stage-number [_median _opts x]] + (i18n/tru "Median of {0}" (display-name query stage-number x))) + +(defmethod column-name* :median + [query stage-number [_median _opts x]] + (str "median_" (column-name query stage-number x))) + +(defmethod display-name* :percentile + [query stage-number [_percentile _opts x p]] + (i18n/tru "{0}th percentile of {1}" p (display-name query stage-number x))) + +(defmethod column-name* :percentile + [query stage-number [_percentile _opts x p]] + (format "p%d_%s" p (column-name query stage-number x))) + +;;; we don't currently have sophisticated logic for generating nice display names for filter clauses + +(defmethod display-name* :sum-where + [query stage-number [_sum-where _opts x _pred]] + (i18n/tru "Sum of {0} matching condition" (display-name query stage-number x))) + +(defmethod column-name* :sum-where + [query stage-number [_sum-where _opts x]] + (str "sum_where_" (column-name query stage-number x))) + +(defmethod display-name* :share + [_query _stage-number _share] + (i18n/tru "Share of rows matching condition")) + +(defmethod column-name* :share + [_query _stage-number _share] + "share") + +(defmethod display-name* :count-where + [_query _stage-number _count-where] + (i18n/tru "Count of rows matching condition")) + +(defmethod column-name* :count-where + [_query _stage-number _count-where] + "count-where") + +(mu/defn ^:private interval-display-name :- ::lib.schema.common/non-blank-string + "e.g. something like \"- 2 days\"" + [amount :- :int + unit :- ::lib.schema.temporal-bucketing/unit.date-time.interval] + ;; TODO -- sorta duplicated with [[metabase.shared.parameters.parameters/translated-interval]], but not exactly + (let [unit-str (case unit + :millisecond (i18n/trun "millisecond" "milliseconds" (abs amount)) + :second (i18n/trun "second" "seconds" (abs amount)) + :minute (i18n/trun "minute" "minutes" (abs amount)) + :hour (i18n/trun "hour" "hours" (abs amount)) + :day (i18n/trun "day" "days" (abs amount)) + :week (i18n/trun "week" "weeks" (abs amount)) + :month (i18n/trun "month" "months" (abs amount)) + :quarter (i18n/trun "quarter" "quarters" (abs amount)) + :year (i18n/trun "year" "years" (abs amount)))] + (wrap-str-in-parens-if-nested + (if (pos? amount) + (format "+ %d %s" amount unit-str) + (format "- %d %s" (abs amount) unit-str))))) + +(defmethod display-name* :datetime-add + [query stage-number [_datetime-add _opts x amount unit]] + (str (display-name query stage-number x) + \space + (interval-display-name amount unit))) + +;;; for now we'll just pretend `:coalesce` isn't a present and just use the display name for the expr it wraps. +(defmethod display-name* :coalesce + [query stage-number [_coalesce _opts expr _null-expr]] + (display-name query stage-number expr)) + +(defmethod column-name* :coalesce + [query stage-number [_coalesce _opts expr _null-expr]] + (column-name query stage-number expr)) + +(defmethod display-name* :dispatch-type/number + [_query _stage-number n] + (str n)) + +(defmethod display-name* :dispatch-type/string + [_query _stage-number s] + (str \" s \")) diff --git a/src/metabase/lib/metadata/calculate/resolve.cljc b/src/metabase/lib/metadata/calculate/resolve.cljc new file mode 100644 index 0000000000000000000000000000000000000000..13b53b4e45c7c088f6b81481cdc21c18fe05b13f --- /dev/null +++ b/src/metabase/lib/metadata/calculate/resolve.cljc @@ -0,0 +1,91 @@ +(ns metabase.lib.metadata.calculate.resolve + "Logic for resolving references." + (:refer-clojure :exclude [ref]) + (:require + [medley.core :as m] + [metabase.lib.metadata :as lib.metadata] + [metabase.lib.schema :as lib.schema] + [metabase.lib.schema.aggregation :as lib.schema.aggregation] + [metabase.lib.schema.common :as lib.schema.common] + [metabase.lib.schema.expression :as lib.schema.expression] + [metabase.lib.schema.id :as lib.schema.id] + [metabase.lib.schema.join :as lib.schema.join] + [metabase.lib.schema.ref] + [metabase.lib.util :as lib.util] + [metabase.shared.util.i18n :as i18n] + [metabase.util.malli :as mu])) + +(comment metabase.lib.schema.ref/keep-me) + +(mu/defn ^:private resolve-field-id :- lib.metadata/ColumnMetadata + "Integer Field ID: get metadata from the metadata provider. This is probably not 100% the correct thing to do if + this isn't the first stage of the query, but we can fix that behavior in a follow-on" + [query :- ::lib.schema/query + _stage-number :- :int + field-id :- ::lib.schema.id/field] + (lib.metadata/field query field-id)) + +(mu/defn ^:private resolve-field-name :- lib.metadata/ColumnMetadata + "String column name: get metadata from the previous stage, if it exists, otherwise if this is the first stage and we + have a native query or a Saved Question source query or whatever get it from our results metadata." + [query :- ::lib.schema/query + stage-number :- :int + column-name :- ::lib.schema.common/non-blank-string] + (or (some (fn [column] + (when (= (:name column) column-name) + column)) + (if-let [previous-stage-number (lib.util/previous-stage-number query stage-number)] + (let [previous-stage (lib.util/query-stage query previous-stage-number)] + (:lib/stage-metadata previous-stage)) + (get-in (lib.util/query-stage query stage-number) [:lib/stage-metadata :columns]))) + (throw (ex-info (i18n/tru "Invalid :field clause: column {0} does not exist" (pr-str column-name)) + {:name column-name + :query query + :stage-number stage-number})))) + +(mu/defn field-metadata :- lib.metadata/ColumnMetadata + "Resolve metadata for a `:field` ref." + [query :- ::lib.schema/query + stage-number :- :int + [_field _opts id-or-name, :as _field-clause] :- :mbql.clause/field] + (if (integer? id-or-name) + (resolve-field-id query stage-number id-or-name) + (resolve-field-name query stage-number id-or-name))) + +(mu/defn join :- ::lib.schema.join/join + "Resolve a join with a specific `join-alias`." + [query :- ::lib.schema/query + stage-number :- :int + join-alias :- ::lib.schema.common/non-blank-string] + (or (m/find-first #(= (:alias %) join-alias) + (:joins (lib.util/query-stage query stage-number))) + (throw (ex-info (i18n/tru "No join named {0}" (pr-str join-alias)) + {:join-alias join-alias + :query query + :stage-number stage-number})))) + +(mu/defn aggregation :- ::lib.schema.aggregation/aggregation + "Resolve an aggregation with a specific `index`." + [query :- ::lib.schema/query + stage-number :- :int + index :- ::lib.schema.common/int-greater-than-or-equal-to-zero] + (let [{aggregations :aggregation} (lib.util/query-stage query stage-number)] + (when (<= (count aggregations) index) + (throw (ex-info (i18n/tru "No aggregation at index {0}" index) + {:index index + :query query + :stage-number stage-number}))) + (nth aggregations index))) + +(mu/defn 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." + [query :- ::lib.schema/query + stage-number :- :int + expression-name :- ::lib.schema.common/non-blank-string] + (let [stage (lib.util/query-stage query stage-number)] + (or (get-in stage [:expressions expression-name]) + (throw (ex-info (i18n/tru "No expression named {0}" (pr-str expression-name)) + {:expression-name expression-name + :query query + :stage-number stage-number}))))) diff --git a/src/metabase/lib/schema.cljc b/src/metabase/lib/schema.cljc index a9e6e88b74767e9b21709c6ae2b41e410fb49a9a..5ccd81fe784753d08f3346bad432d930ce08c36e 100644 --- a/src/metabase/lib/schema.cljc +++ b/src/metabase/lib/schema.cljc @@ -11,6 +11,8 @@ [metabase.lib.schema.common :as common] [metabase.lib.schema.expression :as expression] [metabase.lib.schema.expression.arithmetic] + [metabase.lib.schema.expression.conditional] + [metabase.lib.schema.expression.temporal] [metabase.lib.schema.filter] [metabase.lib.schema.id :as id] [metabase.lib.schema.join :as join] @@ -20,6 +22,8 @@ [metabase.util.malli.registry :as mr])) (comment metabase.lib.schema.expression.arithmetic/keep-me + metabase.lib.schema.expression.conditional/keep-me + metabase.lib.schema.expression.temporal/keep-me metabase.lib.schema.filter/keep-me metabase.lib.schema.literal/keep-me) diff --git a/src/metabase/lib/schema/aggregation.cljc b/src/metabase/lib/schema/aggregation.cljc index ebafdacaf48d913ed8343e55461ba7c5e5108d21..cee9e47deb5ea0d6c92fa0f52af1cb97db916439 100644 --- a/src/metabase/lib/schema/aggregation.cljc +++ b/src/metabase/lib/schema/aggregation.cljc @@ -1,19 +1,33 @@ (ns metabase.lib.schema.aggregation (:require - [metabase.lib.schema.common :as common] [metabase.lib.schema.expression :as expression] + [metabase.lib.schema.mbql-clause :as mbql-clause] [metabase.util.malli.registry :as mr])) -(mr/def ::sum - [:tuple - [:= :sum] - ::common/options - [:ref ::expression/number]]) +;; count has an optional expression arg +(mbql-clause/define-catn-mbql-clause :count + [:expression [:? [:ref ::expression/number]]]) + +(defmethod expression/type-of* :count + [[_tag _opts expr]] + (if-not expr + :type/Integer + (expression/type-of expr))) + +(mbql-clause/define-tuple-mbql-clause :sum + [:ref ::expression/number]) + +(defmethod expression/type-of* :sum + [[_tag _opts expr]] + (expression/type-of expr)) + +(mbql-clause/define-tuple-mbql-clause :avg :- :type/Float + [:ref ::expression/number]) (mr/def ::aggregation + ;; placeholder! [:or - ::sum - ;;; placeholder! + :mbql.clause/sum any?]) (mr/def ::aggregations diff --git a/src/metabase/lib/schema/expression/arithmetic.cljc b/src/metabase/lib/schema/expression/arithmetic.cljc index c481f3338ad56e7729f606d95008a99dd8825819..05506b51819e8da33499fe671bbe8363af864aeb 100644 --- a/src/metabase/lib/schema/expression/arithmetic.cljc +++ b/src/metabase/lib/schema/expression/arithmetic.cljc @@ -1,14 +1,94 @@ (ns metabase.lib.schema.expression.arithmetic "Arithmetic expressions like `:+`." (:require + [malli.core :as mc] + [metabase.lib.schema.common :as common] [metabase.lib.schema.expression :as expression] [metabase.lib.schema.mbql-clause :as mbql-clause] - [metabase.types :as types])) + [metabase.lib.schema.temporal-bucketing :as temporal-bucketing] + [metabase.types :as types] + [metabase.util.malli.registry :as mr])) + +(mbql-clause/define-tuple-mbql-clause :interval + :int + ::temporal-bucketing/unit.date-time.interval) + +(defmethod expression/type-of* :interval + [[_tag _opts expr _n _unit]] + (expression/type-of expr)) + +(defn- valid-interval-for-type? [[_tag _opts _n unit :as _interval] expr-type] + (let [unit-schema (cond + (isa? expr-type :type/Date) ::temporal-bucketing/unit.date.interval + (isa? expr-type :type/Time) ::temporal-bucketing/unit.time.interval + (isa? expr-type :type/DateTime) ::temporal-bucketing/unit.date-time.interval)] + (if unit-schema + (mc/validate unit-schema unit) + true))) + +(mr/def ::args.temporal + [:and + [:catn + [:expr [:schema [:ref ::expression/temporal]]] + [:intervals [:+ :mbql.clause/interval]]] + [:fn + {:error/message "Temporal arithmetic expression with valid interval units for the expression type"} + (fn [[expr & intervals]] + (let [expr-type (expression/type-of expr)] + (every? #(valid-interval-for-type? % expr-type) intervals)))]]) + +(mr/def ::args.numbers + [:repeat {:min 2} [:schema [:ref ::expression/number]]]) + +(defn- plus-minus-schema [tag] + [:or + [:and + [:cat + [:= tag] + [:schema [:ref ::common/options]] + [:schema [:ref ::expression/temporal]] + [:repeat {:min 1} [:schema [:ref :mbql.clause/interval]]]] + [:fn + {:error/message "Temporal arithmetic expression with valid interval units for the expression type"} + (fn [[_tag _opts expr & intervals]] + (let [expr-type (expression/type-of expr)] + (every? #(valid-interval-for-type? % expr-type) intervals)))]] + [:cat + [:= tag] + [:schema [:ref ::common/options]] + [:repeat {:min 2} [:schema [:ref ::expression/number]]]]]) + +(mbql-clause/define-mbql-clause :+ + (plus-minus-schema :+)) + +;;; TODO -- should `:-` support just a single arg (for numbers)? What about `:+`? +(mbql-clause/define-mbql-clause :- + (plus-minus-schema :-)) (mbql-clause/define-catn-mbql-clause :* - [:args [:repeat {:min 2} [:schema [:ref ::expression/number]]]]) + [:args ::args.numbers]) -(defmethod expression/type-of* :* - [[_tag _opts & args]] +;;; we always do non-integer real division even if all the expressions are integers, e.g. +;;; +;;; [:/ <int-field> 2] => my_int_field / 2.0 +;;; +;;; so the results are 0.5 as opposed to 0. This is what people expect division to do +(mbql-clause/define-catn-mbql-clause :/ :- :type/Float + [:args ::args.numbers]) + +(defn- type-of-arithmetic-args [args] + ;; Okay to use reduce without an init value here since we know we have >= 2 args #_{:clj-kondo/ignore [:reduce-without-init]} (reduce types/most-specific-common-ancestor (map expression/type-of args))) + +(defmethod expression/type-of* :+ + [[_tag _opts & args]] + (type-of-arithmetic-args args)) + +(defmethod expression/type-of* :- + [[_tag _opts & args]] + (type-of-arithmetic-args args)) + +(defmethod expression/type-of* :* + [[_tag _opts & args]] + (type-of-arithmetic-args args)) diff --git a/src/metabase/lib/schema/expression/conditional.cljc b/src/metabase/lib/schema/expression/conditional.cljc new file mode 100644 index 0000000000000000000000000000000000000000..f4d4a9789c533deb9358cc26737c6ae90f399284 --- /dev/null +++ b/src/metabase/lib/schema/expression/conditional.cljc @@ -0,0 +1,73 @@ +(ns metabase.lib.schema.expression.conditional + "Conditional expressions like `:case` and `:coalesce`." + (:require + [clojure.set :as set] + [metabase.lib.schema.expression :as expression] + [metabase.lib.schema.mbql-clause :as mbql-clause] + [metabase.types :as types] + [metabase.util.malli.registry :as mr])) + +;;; the logic for calculating the return type of a `:case` or similar statement is not optimal nor perfect. But it +;;; should be ok for now and errors on the side of being permissive. See this Slack thread for more info: +;;; https://metaboat.slack.com/archives/C04DN5VRQM6/p1678325996901389 +(defn- best-return-type + "For expressions like `:case` and `:coalesce` that can return different possible expressions, determine the best + return type given all of the various options." + [x y] + (cond + (nil? x) + y + + ;; if both types are keywords return their most-specific ancestor. + (and (keyword? x) + (keyword? y)) + (types/most-specific-common-ancestor x y) + + ;; if one type is a specific type but the other is an ambiguous union of possible types, return the specific + ;; type. A case can't possibly have multiple different return types, so if one expression has an unambiguous + ;; type then the whole thing has to have a compatible type. + (keyword? x) + x + + (keyword? y) + y + + ;; if both types are ambiguous unions of possible types then return the intersection of the two. But if the + ;; intersection is empty, return the union of everything instead. I don't really want to go down a rabbit + ;; hole of trying to find the intersection between the most-specific common ancestors + :else + (or (when-let [intersection (not-empty (set/intersection x y))] + (if (= (count intersection) 1) + (first intersection) + intersection)) + (set/union x y)))) + +;;; believe it or not, a `:case` clause really has the syntax [:case {} [[pred1 expr1] [pred2 expr2] ...]] +(mr/def ::case-subclause + [:tuple + {:error/message "Valid :case [pred expr] pair"} + #_pred [:ref ::expression/boolean] + #_expr [:ref ::expression/expression]]) + + +(mbql-clause/define-tuple-mbql-clause :case + ;; TODO -- we should further constrain this so all of the exprs are of the same type + [:sequential {:min 1} [:ref ::case-subclause]]) + +(defmethod expression/type-of* :case + [[_tag _opts pred-expr-pairs]] + (reduce + (fn [best-guess [_pred expr]] + (let [expr-type (expression/type-of expr)] + (best-return-type best-guess expr-type))) + nil + pred-expr-pairs)) + +;;; TODO -- add constraint that these types have to be compatible +(mbql-clause/define-tuple-mbql-clause :coalesce + #_expr [:ref :metabase.lib.schema.expression/expression] + #_null-value [:ref :metabase.lib.schema.expression/expression]) + +(defmethod expression/type-of* :coalesce + [[_tag _opts expr null-value]] + (best-return-type (expression/type-of expr) (expression/type-of null-value))) diff --git a/src/metabase/lib/schema/expression/temporal.cljc b/src/metabase/lib/schema/expression/temporal.cljc new file mode 100644 index 0000000000000000000000000000000000000000..4198e0cb08696e942592056824c1e3be5eeae94c --- /dev/null +++ b/src/metabase/lib/schema/expression/temporal.cljc @@ -0,0 +1,15 @@ +(ns metabase.lib.schema.expression.temporal + (:require + [metabase.lib.schema.expression :as expression] + [metabase.lib.schema.mbql-clause :as mbql-clause] + [metabase.lib.schema.temporal-bucketing :as temporal-bucketing])) + +;;; TODO -- we should constrain this so that you can only use a Date unit if expr is a date, etc. +(mbql-clause/define-tuple-mbql-clause :datetime-add + #_expr [:ref ::expression/temporal] + #_amount :int + #_unit [:ref ::temporal-bucketing/unit.date-time.interval]) + +(defmethod expression/type-of* :datetime-add + [[_tag _opts expr _amount _unit]] + (expression/type-of expr)) diff --git a/src/metabase/lib/schema/filter.cljc b/src/metabase/lib/schema/filter.cljc index 742620202a4b8618e70134cfa797a8f3835d919d..7c6f0e2137099065e4047e0d721eedf7c0279746 100644 --- a/src/metabase/lib/schema/filter.cljc +++ b/src/metabase/lib/schema/filter.cljc @@ -2,12 +2,9 @@ "Schemas for the various types of filter clauses that you'd pass to `:filter` or use inside something else that takes a boolean expression." (:require - [clojure.set :as set] [metabase.lib.schema.common :as common] [metabase.lib.schema.expression :as expression] - [metabase.lib.schema.mbql-clause :as mbql-clause] - [metabase.types :as types] - [metabase.util.malli.registry :as mr])) + [metabase.lib.schema.mbql-clause :as mbql-clause])) (doseq [op [:and :or]] (mbql-clause/define-catn-mbql-clause op :- :type/Boolean @@ -102,53 +99,3 @@ [:= :segment] ::common/options [:or ::common/int-greater-than-zero ::common/non-blank-string]]) - -;;; believe it or not, a `:case` clause really has the syntax [:case {} [[pred1 expr1] [pred2 expr2] ...]] -(mr/def ::case-subclause - [:tuple - {:error/message "Valid :case [pred expr] pair"} - #_pred [:ref ::expression/boolean] - #_expr [:ref ::expression/expression]]) - -;;; TODO -- this is not really a filter clause and doesn't belong in here. But where does it belong? -(mbql-clause/define-tuple-mbql-clause :case - ;; TODO -- we should further constrain this so all of the exprs are of the same type - [:sequential {:min 1} [:ref ::case-subclause]]) - -;;; the logic for calculating the return type of a `:case` statement is not optimal nor perfect. But it should be ok -;;; for now and errors on the side of being permissive. See this Slack thread for more info: -;;; https://metaboat.slack.com/archives/C04DN5VRQM6/p1678325996901389 -(defmethod expression/type-of* :case - [[_tag _opts pred-expr-pairs]] - (reduce - (fn [best-guess [_pred expr]] - (let [return-type (expression/type-of expr)] - (cond - (nil? best-guess) - return-type - - ;; if both types are keywords return their most-specific ancestor. - (and (keyword? best-guess) - (keyword? return-type)) - (types/most-specific-common-ancestor best-guess return-type) - - ;; if one type is a specific type but the other is an ambiguous union of possible types, return the specific - ;; type. A case can't possibly have multiple different return types, so if one expression has an unambiguous - ;; type then the whole thing has to have a compatible type. - (keyword? best-guess) - best-guess - - (keyword? return-type) - return-type - - ;; if both types are ambiguous unions of possible types then return the intersection of the two. But if the - ;; intersection is empty, return the union of everything instead. I don't really want to go down a rabbit - ;; hole of trying to find the intersection between the most-specific common ancestors - :else - (or (when-let [intersection (not-empty (set/intersection best-guess return-type))] - (if (= (count intersection) 1) - (first intersection) - intersection)) - (set/union best-guess return-type))))) - nil - pred-expr-pairs)) diff --git a/src/metabase/lib/schema/mbql_clause.cljc b/src/metabase/lib/schema/mbql_clause.cljc index a9c49e5341ff2f265ab7c0fc364fd99cc3e99e2e..76683eff1bbc71c9a8583442de35107e83ba9084 100644 --- a/src/metabase/lib/schema/mbql_clause.cljc +++ b/src/metabase/lib/schema/mbql_clause.cljc @@ -108,7 +108,7 @@ (into [:catn {:error/message (str "Valid " tag " clause")} [:tag [:= tag]] - [:options ::common/options]] + [:options [:schema [:ref ::common/options]]]] args)]) (defn tuple-clause-schema @@ -119,7 +119,7 @@ (into [:tuple {:error/message (str "Valid " tag " clause")} [:= tag] - ::common/options] + [:ref ::common/options]] args)) ;;;; Even more convenient functions! diff --git a/src/metabase/lib/schema/ref.cljc b/src/metabase/lib/schema/ref.cljc index 8c5ed513c07ba68618ff59dac6436c357c3eb322..45c89576c57ee1af520056b99e1a65767e14d17f 100644 --- a/src/metabase/lib/schema/ref.cljc +++ b/src/metabase/lib/schema/ref.cljc @@ -64,8 +64,9 @@ (mr/def ::aggregation-options [:merge ::common/options - [:name {:optional true} ::common/non-blank-string] - [:display-name {:optional true}] ::common/non-blank-string]) + [:map + [:name {:optional true} ::common/non-blank-string] + [:display-name {:optional true} ::common/non-blank-string]]]) (mbql-clause/define-mbql-clause :aggregation [:tuple diff --git a/src/metabase/lib/util.cljc b/src/metabase/lib/util.cljc index b1f8ea391ca9216a6d7b6f107005f5dd2ccf2166..511a8f8713a2a0cc5fb7ac37070c9c9f5f8d84b3 100644 --- a/src/metabase/lib/util.cljc +++ b/src/metabase/lib/util.cljc @@ -82,7 +82,7 @@ (mu/defn ^:private non-negative-stage-index :- ::lib.schema.common/int-greater-than-or-equal-to-zero "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 ::lib.schema/stage] + [stages :- [:sequential [:ref ::lib.schema/stage]] stage-number :- :int] (let [stage-number' (if (neg? stage-number) (+ (count stages) stage-number) diff --git a/src/metabase/models/humanization.clj b/src/metabase/models/humanization.clj index 81b34ac18429e765ff49493c435de3a4c53daa1a..782501d03143ee522a04ffd81e3f9040d3f94162 100644 --- a/src/metabase/models/humanization.clj +++ b/src/metabase/models/humanization.clj @@ -10,9 +10,8 @@ There used to also be `:advanced`, which was the default until enough customers complained that we first fixed it and then the fix wasn't good enough so we removed it." (:require - [clojure.string :as str] [metabase.models.setting :as setting :refer [defsetting]] - [metabase.util :as u] + [metabase.util.humanization :as u.humanization] [metabase.util.i18n :refer [deferred-tru trs tru]] [metabase.util.log :as log] [schema.core :as s] @@ -20,55 +19,21 @@ (declare humanization-strategy) -(defmulti ^String name->human-readable-name +(defn name->human-readable-name "Convert a name, such as `num_toucans`, to a human-readable name, such as `Num Toucans`. With one arg, this uses the strategy defined by the Setting `humanization-strategy`. With two args, you may specify a custom strategy (intended mainly for the internal implementation): - (humanization-strategy! :simple) - (name->human-readable-name \"cool_toucans\") ;-> \"Cool Toucans\" - ;; this is the same as: - (name->human-readable-name (humanization-strategy) \"cool_toucans\") ;-> \"Cool Toucans\" - ;; specifiy a different strategy: - (name->human-readable-name :none \"cool_toucans\") ;-> \"cool_toucans\"" - {:arglists '([s] [strategy s])} - (fn - ([_] (keyword (humanization-strategy))) - ([strategy _] (keyword strategy)))) - -(def ^:private ^:const acronyms - #{"id" "url" "ip" "uid" "uuid" "guid"}) - -(defn- capitalize-word [word] - (if (contains? acronyms (u/lower-case-en word)) - (u/upper-case-en word) - ;; We are assuming that ALL_UPPER_CASE means we should be Title Casing - (if (= word (u/upper-case-en word)) - (str/capitalize word) - (str (str/capitalize (subs word 0 1)) (subs word 1))))) - -;; simple replaces hyphens and underscores with spaces and capitalizes -(defmethod name->human-readable-name :simple - ([s] (name->human-readable-name :simple s)) - ([_ ^String s] - ;; explode on hyphens, underscores, and spaces - (when (seq s) - (let [humanized (str/join " " (for [part (str/split s #"[-_\s]+") - :when (not (str/blank? part))] - (capitalize-word part)))] - (if (str/blank? humanized) - s - humanized))))) - -;; actual advanced method has been excised. this one just calls out to simple -(defmethod name->human-readable-name :advanced - ([s] (name->human-readable-name :simple s)) - ([_ ^String s] (name->human-readable-name :simple s))) - -;; :none is just an identity implementation -(defmethod name->human-readable-name :none - ([s] s) - ([_ s] s)) + (humanization-strategy! :simple) + (name->human-readable-name \"cool_toucans\") ;-> \"Cool Toucans\" + ;; this is the same as: + (name->human-readable-name (humanization-strategy) \"cool_toucans\") ;-> \"Cool Toucans\" + ;; specifiy a different strategy: + (name->human-readable-name :none \"cool_toucans\") ;-> \"cool_toucans\"" + ([s] + (name->human-readable-name (humanization-strategy) s)) + ([strategy s] + (u.humanization/name->human-readable-name strategy s))) (defn- re-humanize-names! "Update all non-custom display names of all instances of `model` (e.g. Table or Field)." @@ -95,7 +60,7 @@ (defn- set-humanization-strategy! [new-value] (let [new-strategy (keyword (or new-value :simple))] ;; check to make sure `new-strategy` is a valid strategy, or throw an Exception it is it not. - (when-not (get-method name->human-readable-name new-strategy) + (when-not (get-method u.humanization/name->human-readable-name new-strategy) (throw (IllegalArgumentException. (tru "Invalid humanization strategy ''{0}''. Valid strategies are: {1}" new-strategy (keys (methods name->human-readable-name)))))) @@ -115,4 +80,11 @@ :type :keyword :default :simple :visibility :settings-manager + :getter (fn [] + (let [strategy (setting/get-value-of-type :keyword :humanization-strategy)] + ;; actual advanced method has been excised. Use `:simple` instead if someone had specified + ;; `:advanced`. + (if (= strategy :advanced) + :simple + strategy))) :setter set-humanization-strategy!) diff --git a/src/metabase/util/humanization.cljc b/src/metabase/util/humanization.cljc new file mode 100644 index 0000000000000000000000000000000000000000..f66940ee67075d158426d45206e2f1acda66b05d --- /dev/null +++ b/src/metabase/util/humanization.cljc @@ -0,0 +1,49 @@ +(ns metabase.util.humanization + (:require + [clojure.string :as str] + [metabase.util :as u])) + +(defmulti name->human-readable-name + "Convert a name, such as `num_toucans`, to a human-readable name, such as `Num Toucans`. + + (name->human-readable-name :simple \"cool_toucans\") ;-> \"Cool Toucans\" + + ;; specifiy a different strategy: + (name->human-readable-name :none \"cool_toucans\") ;-> \"cool_toucans\"" + {:arglists '([strategy s])} + (fn [strategy _s] + (keyword strategy))) + +(def ^:private ^:const acronyms + #{"id" "url" "ip" "uid" "uuid" "guid"}) + +(defn- capitalize-word [word] + (if (contains? acronyms (u/lower-case-en word)) + (u/upper-case-en word) + ;; We are assuming that ALL_UPPER_CASE means we should be Title Casing + (if (= word (u/upper-case-en word)) + (str/capitalize word) + (str (str/capitalize (subs word 0 1)) (subs word 1))))) + +;; simple replaces hyphens and underscores with spaces and capitalizes +(defmethod name->human-readable-name :simple + [_strategy s] + ;; explode on hyphens, underscores, and spaces + (when (seq s) + (let [humanized (str/join " " (for [part (str/split s #"[-_\s]+") + :when (not (str/blank? part))] + (capitalize-word part)))] + (if (str/blank? humanized) + s + humanized)))) + +;;; `:none` is just an identity implementation +(defmethod name->human-readable-name :none + [_strategy s] + s) + +;;; `:advanced` doesn't exist anymore, it used to be super fancy and do neat things. On the off chance someone still +;;; tries to use it, just do the same thing `:simple` does. +(defmethod name->human-readable-name :advanced + [strategy s] + ((get-method name->human-readable-name :simple) strategy s)) diff --git a/test/metabase/lib/metadata/calculate/names_test.cljc b/test/metabase/lib/metadata/calculate/names_test.cljc new file mode 100644 index 0000000000000000000000000000000000000000..5b615b297e2f4a7ae6c03ba38dc2e1c15fa2f577 --- /dev/null +++ b/test/metabase/lib/metadata/calculate/names_test.cljc @@ -0,0 +1,115 @@ +(ns metabase.lib.metadata.calculate.names-test + (:require + [clojure.test :refer [are deftest is testing]] + [metabase.lib.core :as lib] + [metabase.lib.metadata.calculate.names :as calculate.names] + [metabase.lib.test-metadata :as meta])) + +(def ^:private venues-query + (lib/query meta/metadata-provider (meta/table-metadata :venues))) + +(defn- field-clause + ([table field] + (field-clause table field nil)) + ([table field options] + [:field (merge {:lib/uuid (str (random-uuid))} options) (meta/id table field)])) + +(deftest ^:parallel display-name-from-name-test + (testing "Use the 'simple humanization' logic to calculate a display name for a Field that doesn't have one (e.g. from results metadata)" + (is (= "Venue ID" + (calculate.names/display-name venues-query -1 {:lib/type :metadata/field + :name "venue_id"}))))) + +(defn- aggregation-display-name [aggregation-clause] + (calculate.names/display-name venues-query -1 aggregation-clause)) + +(defn- aggregation-column-name [aggregation-clause] + (calculate.names/column-name venues-query -1 aggregation-clause)) + +(deftest ^:parallel aggregation-names-test + (are [aggregation-clause expected] (= expected + {:column-name (aggregation-column-name aggregation-clause) + :display-name (aggregation-display-name aggregation-clause)}) + [:count {}] + {:column-name "count", :display-name "Count"} + + [:distinct {} (field-clause :venues :id)] + {:column-name "distinct_id", :display-name "Distinct values of ID"} + + [:sum {} (field-clause :venues :id)] + {:column-name "sum_id", :display-name "Sum of ID"} + + [:+ {} [:count {}] 1] + {:column-name "count_plus_1", :display-name "Count + 1"} + + [:+ + {} + [:min {} (field-clause :venues :id)] + [:* {} 2 [:avg {} (field-clause :venues :price)]]] + {:column-name "min_id_plus_2_times_avg_price" + :display-name "Min of ID + (2 × Average of Price)"} + + [:+ + {} + [:min {} (field-clause :venues :id)] + [:* + {} + 2 + [:avg {} (field-clause :venues :price)] + 3 + [:- {} [:max {} (field-clause :venues :category-id)] 4]]] + {:column-name "min_id_plus_2_times_avg_price_times_3_times_max_category_id_minus_4" + :display-name "Min of ID + (2 × Average of Price × 3 × (Max of Category ID - 4))"} + + ;; user-specified names + [:+ + {:name "generated_name", :display-name "User-specified Name"} + [:min {} (field-clause :venues :id)] + [:* {} 2 [:avg {} (field-clause :venues :price)]]] + {:column-name "generated_name", :display-name "User-specified Name"} + + [:+ + {:name "generated_name"} + [:min {} (field-clause :venues :id)] + [:* {} 2 [:avg {} (field-clause :venues :price)]]] + {:column-name "generated_name", :display-name "Min of ID + (2 × Average of Price)"} + + [:+ + {:display-name "User-specified Name"} + [:min {} (field-clause :venues :id)] + [:* {} 2 [:avg {} (field-clause :venues :price)]]] + {:column-name "min_id_plus_2_times_avg_price" + :display-name "User-specified Name"})) + +(deftest ^:parallel date-interval-test + (let [clause [:datetime-add + {} + (field-clause :checkins :date {:base-type :type/Date}) + -1 + :day]] + (is (= "date_minus_1_day" + (calculate.names/column-name venues-query -1 clause))) + (is (= "Date - 1 day" + (calculate.names/display-name venues-query -1 clause))))) + +(deftest ^:parallel expression-reference-test + (let [query (assoc-in venues-query + [:stages 0 :expressions "double-price"] + [:* + {:lib/uuid (str (random-uuid))} + (field-clause :venues :price {:base-type :type/Integer}) + 2]) + expr [:sum + {:lib/uuid (str (random-uuid))} + [:expression {:lib/uuid (str (random-uuid))} "double-price"]]] + (is (= "Sum of double-price" + (calculate.names/display-name query -1 expr))) + (is (= "sum_double-price" + (calculate.names/column-name query -1 expr))))) + +(deftest ^:parallel coalesce-test + (let [clause [:coalesce {} (field-clause :venues :name) "<Venue>"]] + (is (= "name" + (calculate.names/column-name venues-query -1 clause))) + (is (= "Name" + (calculate.names/display-name venues-query -1 clause))))) diff --git a/test/metabase/lib/metadata/calculate/resolve_test.cljc b/test/metabase/lib/metadata/calculate/resolve_test.cljc new file mode 100644 index 0000000000000000000000000000000000000000..6f7bd427eae2d489c5166db25829afb7de3fd49c --- /dev/null +++ b/test/metabase/lib/metadata/calculate/resolve_test.cljc @@ -0,0 +1,20 @@ +(ns metabase.lib.metadata.calculate.resolve-test + (:require + [clojure.test :refer [deftest is]] + [metabase.lib.core :as lib] + [metabase.lib.metadata.calculate.resolve :as calculate.resolve] + [metabase.lib.test-metadata :as meta])) + +(deftest ^:parallel resolve-join-test + (let [query (lib/query meta/metadata-provider (meta/table-metadata :venues)) + join-clause (-> ((lib/join-clause + (meta/table-metadata :categories) + (lib/= + (lib/field (meta/id :venues :category-id)) + (lib/with-join-alias (lib/field (meta/id :categories :id)) "CATEGORIES__via__CATEGORY_ID"))) + query -1) + ;; TODO -- need a nice way to set the alias of a join. + (assoc :alias "CATEGORIES__via__CATEGORY_ID")) + query (lib/join query join-clause)] + (is (= join-clause + (calculate.resolve/join query -1 "CATEGORIES__via__CATEGORY_ID"))))) diff --git a/test/metabase/lib/metadata/calculate_test.cljc b/test/metabase/lib/metadata/calculate_test.cljc new file mode 100644 index 0000000000000000000000000000000000000000..79ba2de93e253d3dbe61af4d02bf7e677da4d543 --- /dev/null +++ b/test/metabase/lib/metadata/calculate_test.cljc @@ -0,0 +1,359 @@ +(ns metabase.lib.metadata.calculate-test + (:require + [clojure.test :refer [are deftest is testing]] + [malli.core :as mc] + [metabase.lib.metadata.calculate :as calculate] + [metabase.lib.metadata.protocols :as lib.metadata.protocols] + [metabase.lib.schema :as lib.schema] + [metabase.lib.test-metadata :as meta] + #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal])))) + +#?(:cljs + (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me)) + +(def ^:private venues-query + {:lib/type :mbql/query + :lib/metadata meta/metadata-provider + :type :pipeline + :database (meta/id) + :stages [{:lib/type :mbql.stage/mbql + :lib/options {:lib/uuid (str (random-uuid))} + :source-table (meta/id :venues)}]}) + +(defn- venues-query-with-last-stage [m] + (let [query (update-in venues-query [:stages 0] merge m)] + (is (mc/validate ::lib.schema/query query)) + query)) + +(defn- field-clause + ([table field] + (field-clause table field nil)) + ([table field options] + [:field + (merge {:base-type (:base_type (meta/field-metadata table field)) + :lib/uuid (str (random-uuid))} + options) + (meta/id table field)])) + +(deftest ^:parallel col-info-field-ids-test + (testing "make sure columns are comming back the way we'd expect for :field clauses" + (let [query {:lib/type :mbql/query + :type :pipeline + :stages [{:lib/type :mbql.stage/mbql + :lib/options {:lib/uuid "0311c049-4973-4c2a-8153-1e2c887767f9"} + :source-table (meta/id :venues) + :fields [(field-clause :venues :price)]}] + :database (meta/id) + :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)]})] + (calculate/stage-metadata query)))))) + +;;; FIXME +#_(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)}]]}}))))) + +(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" + (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)]})] + (calculate/stage-metadata + {:lib/type :mbql/query + :type :pipeline + :stages [{:lib/type :mbql.stage/mbql + :lib/options {:lib/uuid "fdcfaa06-8e65-471d-be5a-f1e821022482"} + :source-table (meta/id :venues) + :fields [[:field + {:join-alias "CATEGORIES__via__CATEGORY_ID" + :lib/uuid "8704e09b-496e-4045-8148-1eef28e96b51"} + (meta/id :categories :name)]] + :joins [{:lib/type :mbql/join + :lib/options {:lib/uuid "490a5abb-54c2-4e62-9196-7e9e99e8d291"} + :alias "CATEGORIES__via__CATEGORY_ID" + :condition [:= + {:lib/uuid "cc5f6c43-1acb-49c2-aeb5-e3ff9c70541f"} + (field-clause :venues :category-id) + (field-clause :categories :id {:join-alias "CATEGORIES__via__CATEGORY_ID"})] + :strategy :left-join + :fk-field-id (meta/id :venues :category-id) + :stages [{:lib/type :mbql.stage/mbql + :lib/options {:lib/uuid "bbbae500-c972-4550-b100-e0584eb72c4d"} + :source-table (meta/id :categories)}]}]}] + :database (meta/id) + :lib/metadata meta/metadata-provider}))))) + +(defn- grandparent-parent-child-id [field] + (+ (meta/id :venues :id) + (case field + :grandparent 50 + :parent 60 + :child 70))) + +(def ^:private grandparent-parent-child-metadata-provider + "A DatabaseMetadataProvider for a Table that nested Fields: grandparent, parent, and child" + (let [grandparent {:lib/type :metadata/field + :name "grandparent" + :id (grandparent-parent-child-id :grandparent)} + parent {:lib/type :metadata/field + :name "parent" + :parent_id (grandparent-parent-child-id :grandparent) + :id (grandparent-parent-child-id :parent)} + child {:lib/type :metadata/field + :name "child" + :parent_id (grandparent-parent-child-id :parent) + :id (grandparent-parent-child-id :child)}] + (reify lib.metadata.protocols/DatabaseMetadataProvider + (database [_this] + (dissoc meta/metadata :tables)) + (tables [_this] + [(dissoc (meta/table-metadata :venues) :fields)]) + (fields [_this table-id] + (when (= table-id (meta/id :venues)) + (mapv (fn [field-metadata] + (merge (dissoc (meta/field-metadata :venues :id) :display_name) + field-metadata)) + [grandparent parent child])))))) + +(deftest ^:parallel col-info-combine-parent-field-names-test + (letfn [(col-info [a-field-clause] + (#'calculate/metadata-for-ref + {:lib/type :mbql/query + :lib/metadata grandparent-parent-child-metadata-provider + :type :pipeline + :database (meta/id) + :stages [{:lib/type :mbql.stage/mbql + :lib/options {:lib/uuid (str (random-uuid))} + :source-table (meta/id :venues)}]} + -1 + a-field-clause))] + (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} + (col-info [:field {:lib/uuid (str (random-uuid))} (grandparent-parent-child-id :parent)])))) + (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} + (col-info [:field {:lib/uuid (str (random-uuid))} (grandparent-parent-child-id :child)])))))) + +(deftest ^:parallel col-info-field-literals-test + (testing "field literals should get the information from the matching `:lib/stage-metadata` if it was supplied" + (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} + (#'calculate/metadata-for-ref + (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}]}}) + -1 + [:field {:lib/uuid (str (random-uuid)), :base-type :type/Integer} "sum"]))))) + +(deftest ^:parallel col-info-expression-ref-test + (is (=? {:base_type :type/Integer + :name "double-price" + :display_name "double-price" + :field_ref [:expression {:lib/uuid string?} "double-price"]} + (#'calculate/metadata-for-ref + (venues-query-with-last-stage + {:expressions {"double-price" [:* + {:lib/uuid (str (random-uuid))} + (field-clause :venues :price {:base-type :type/Integer}) + 2]}}) + -1 + [:expression {:lib/uuid (str (random-uuid))} "double-price"])))) + +(deftest ^:parallel col-info-for-temporal-expression-test + (is (=? {:base_type :type/DateTime + :name "last-login-plus-2" + :display_name "last-login-plus-2" + :field_ref [:expression {} "last-login-plus-2"]} + (#'calculate/metadata-for-ref + (venues-query-with-last-stage + {:expressions {"last-login-plus-2" [:datetime-add + {:lib/uuid (str (random-uuid))} + (field-clause :users :last-login {:base-type :type/DateTime}) + 2 + :hour]}}) + -1 + [:expression {:lib/uuid (str (random-uuid))} "last-login-plus-2"])))) + +(deftest ^:parallel col-info-for-expression-error-message-test + (testing "if there is no matching expression it should give a meaningful error message" + (is (thrown-with-msg? + #?(:clj Throwable :cljs js/Error) + #"No expression named \"double-price\"" + (#'calculate/metadata-for-ref + (venues-query-with-last-stage + {:expressions {"one-hundred" 100}}) + -1 + [:expression {:lib/uuid (str (random-uuid))} "double-price"]))))) + +(defn- col-info-for-aggregation-clause + ([clause] + (col-info-for-aggregation-clause venues-query clause)) + + ([query clause] + (col-info-for-aggregation-clause query -1 clause)) + + ([query stage clause] + (#'calculate/metadata-for-aggregation query stage clause 0))) + +(deftest ^:parallel col-info-for-aggregation-clause-test + (are [clause expected] (=? expected + (col-info-for-aggregation-clause clause)) + ;; :count, no field + [:/ {} [:count {}] 2] + {:base_type :type/Float + :name "count_divided_by_2" + :display_name "Count ÷ 2"} + + ;; :sum + [:sum {} [:+ {} (field-clause :venues :price) 1]] + {:base_type :type/Integer + :name "sum_price_plus_1" + :display_name "Sum of Price + 1"} + + ;; options map + [:sum + {:name "sum_2", :display-name "My custom name", :base-type :type/BigInteger} + (field-clause :venues :price)] + {:base_type :type/BigInteger + :name "sum_2" + :display_name "My custom name"})) + +(deftest ^:parallel col-info-named-aggregation-test + (testing "col info for an `expression` aggregation w/ a named expression should work as expected" + (is (=? {:base_type :type/Integer + :name "sum_double-price" + :display_name "Sum of double-price"} + (col-info-for-aggregation-clause + (venues-query-with-last-stage + {:expressions {"double-price" [:* + {:lib/uuid (str (random-uuid))} + (field-clause :venues :price {:base-type :type/Integer}) + 2]}}) + [:sum + {:lib/uuid (str (random-uuid))} + [:expression {:base-type :type/Integer, :lib/uuid (str (random-uuid))} "double-price"]]))))) + +(defn- infer-first + ([expr] + (infer-first expr nil)) + + ([expr last-stage] + (#'calculate/metadata-for-ref + (venues-query-with-last-stage + (merge + {:expressions {"expr" expr}} + last-stage)) + -1 + [:expression {:lib/uuid (str (random-uuid))} "expr"]))) + +(deftest ^:parallel infer-coalesce-test + (testing "Coalesce" + (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 + {:lib/uuid (str (random-uuid))} + (field-clause :venues :name) + "bar"]))) + (testing "Does not contain a field id in its analysis (#18513)" + (is (not (contains? (infer-first [:coalesce {:lib/uuid (str (random-uuid))} (field-clause :venues :name) "bar"]) + :id))))) + (testing "Gets the type information from the literal" + (is (=? {:base_type :type/Text + :name "expr" + :display_name "expr" + :field_ref [:expression {} "expr"]} + (infer-first [:coalesce {:lib/uuid (str (random-uuid))} "bar" (field-clause :venues :name)]))))))) + +(deftest ^:parallel infer-case-test + (testing "Case" + (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 + {:lib/uuid (str (random-uuid))} + (field-clause :venues :name) + "bar"]))) + (testing "does not contain a field id in its analysis (#17512)" + (is (false? + (contains? (infer-first [:coalesce {:lib/uuid (str (random-uuid))} (field-clause :venues :name) "bar"]) + :id)))))))) + +(deftest ^:parallel deduplicate-expression-names-in-aggregations-test + (testing "make sure multiple expressions come back with deduplicated names" + (testing "expressions in aggregations" + (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]} + {:base_type :type/Float + :name "0_8_times_avg_price" + :display_name "0.8 × Average of Price" + :field_ref [:aggregation {:lib/uuid string?} 1]}] + (calculate/stage-metadata + (venues-query-with-last-stage + {:aggregation [[:* + {} + 0.9 + [:avg {} (field-clause :venues :price)]] + [:* + {} + 0.8 + [:avg {} (field-clause :venues :price)]]]}))))))) + +(deftest ^:parallel expression-references-in-fields-clause-test + (is (=? [{:name "prev_month" + :display_name "prev_month" + :base_type :type/DateTime + :source :fields + :field_ref [:expression {:lib/uuid string?} "prev_month"]}] + (calculate/stage-metadata + (venues-query-with-last-stage + {:expressions {"prev_month" [:+ + {:lib/uuid (str (random-uuid))} + (field-clause :users :last-login) + [:interval {:lib/uuid (str (random-uuid))} -1 :month]]} + :fields [[:expression {:base-type :type/DateTime, :lib/uuid (str (random-uuid))} "prev_month"]]}))))) diff --git a/test/metabase/lib/schema/expression/conditional_test.cljc b/test/metabase/lib/schema/expression/conditional_test.cljc new file mode 100644 index 0000000000000000000000000000000000000000..5432a51d0036618200e03c8b930d3bb4b6c297b3 --- /dev/null +++ b/test/metabase/lib/schema/expression/conditional_test.cljc @@ -0,0 +1,75 @@ +(ns metabase.lib.schema.expression.conditional-test + (:require + [clojure.test :refer [are deftest is]] + [malli.core :as mc] + [metabase.lib.schema :as lib.schema] + [metabase.lib.schema.expression :as expression] + [metabase.lib.test-metadata :as meta])) + +(defn- case-expr [& args] + (let [clause [:case + {:lib/uuid (str (random-uuid))} + (mapv (fn [arg] + [[:= {:lib/uuid (str (random-uuid))} 1 1] + arg]) + args)]] + (is (mc/validate :mbql.clause/case clause)) + clause)) + +(deftest ^:parallel case-type-of-test + (are [expr expected] (= expected + (expression/type-of expr)) + ;; easy, no ambiguity + (case-expr 1 1) + :type/Integer + + ;; Ambiguous literal types + (case-expr "2023-03-08") + #{:type/Text :type/Date} + + (case-expr "2023-03-08" "2023-03-08") + #{:type/Text :type/Date} + + ;; Ambiguous literal types mixed with unambiguous types + (case-expr "2023-03-08" "abc") + :type/Text + + ;; Literal types that are ambiguous in different ways! `:type/Text` is the only common type between them! + (case-expr "2023-03-08" "05:13") + :type/Text + + ;; Confusion! The "2023-03-08T06:15" is #{:type/String :type/DateTime}, which is less specific than + ;; `:type/DateTimeWithLocalTZ`. Technically this should return `:type/DateTime`, since it's the most-specific + ;; common ancestor type compatible with all args! But calculating that stuff is way too hard! So this will have to + ;; do for now! -- Cam + (case-expr "2023-03-08T06:15" [:field {:lib/uuid (str (random-uuid)), :base-type :type/DateTimeWithLocalTZ} 1]) + :type/DateTimeWithLocalTZ + + ;; Differing types with a common base type that is more specific than `:type/*` + (case-expr 1 1.1) + :type/Number)) + + +(deftest ^:parallel coalesce-test + (is (mc/validate + :mbql.clause/coalesce + [:coalesce + {:lib/uuid "eb39757b-a403-46c7-a1b0-353f21812a87"} + [:field {:base-type :type/Text, :lib/uuid "780aab5a-88ec-4aa6-8a4a-274702273c7a"} (meta/id :venues :name)] + "bar"])) + (is (mc/validate + ::lib.schema/query + {:lib/type :mbql/query + :lib/metadata meta/metadata-provider + :type :pipeline + :database (meta/id) + :stages [{:lib/type :mbql.stage/mbql, + :lib/options {:lib/uuid "455a9f5e-4996-4df9-82aa-01bc083b2efe"} + :source-table (meta/id :venues) + :expressions {"expr" + [:coalesce + {:lib/uuid "455a9f5e-4996-4df9-82aa-01bc083b2efe"} + [:field + {:base-type :type/Text, :lib/uuid "68443c43-f9de-45e3-9f30-8dfd5fef5af6"} + (meta/id :venues :name)] + "bar"]}}]}))) diff --git a/test/metabase/lib/schema/filter_test.cljc b/test/metabase/lib/schema/filter_test.cljc index fea1e2e6e552d79d9ee66123cd335e33de2a4171..ff820ab0adb14cc70bb9c704cb0c1c871bb11296 100644 --- a/test/metabase/lib/schema/filter_test.cljc +++ b/test/metabase/lib/schema/filter_test.cljc @@ -3,57 +3,10 @@ [clojure.test :refer [are deftest is testing]] [clojure.walk :as walk] [malli.core :as mc] - [metabase.lib.schema.expression :as expression] - [metabase.lib.schema.filter] - [metabase.lib.schema.literal] - [metabase.lib.schema.ref])) + [metabase.lib.schema] + [metabase.lib.schema.expression :as expression])) -(comment metabase.lib.schema.filter/keep-me - metabase.lib.schema.literal/keep-me - metabase.lib.schema.ref/keep-me) - -(defn- case-expr [& args] - (let [clause [:case - {:lib/uuid (str (random-uuid))} - (mapv (fn [arg] - [[:= {:lib/uuid (str (random-uuid))} 1 1] - arg]) - args)]] - (is (mc/validate :mbql.clause/case clause)) - clause)) - -(deftest ^:parallel case-type-of-test - (are [expr expected] (= expected - (expression/type-of expr)) - ;; easy, no ambiguity - (case-expr 1 1) - :type/Integer - - ;; Ambiguous literal types - (case-expr "2023-03-08") - #{:type/Text :type/Date} - - (case-expr "2023-03-08" "2023-03-08") - #{:type/Text :type/Date} - - ;; Ambiguous literal types mixed with unambiguous types - (case-expr "2023-03-08" "abc") - :type/Text - - ;; Literal types that are ambiguous in different ways! `:type/Text` is the only common type between them! - (case-expr "2023-03-08" "05:13") - :type/Text - - ;; Confusion! The "2023-03-08T06:15" is #{:type/String :type/DateTime}, which is less specific than - ;; `:type/DateTimeWithLocalTZ`. Technically this should return `:type/DateTime`, since it's the most-specific - ;; common ancestor type compatible with all args! But calculating that stuff is way too hard! So this will have to - ;; do for now! -- Cam - (case-expr "2023-03-08T06:15" [:field {:lib/uuid (str (random-uuid)), :base-type :type/DateTimeWithLocalTZ} 1]) - :type/DateTimeWithLocalTZ - - ;; Differing types with a common base type that is more specific than `:type/*` - (case-expr 1 1.1) - :type/Number)) +(comment metabase.lib.schema/keep-me) (defn- ensure-uuids [filter-expr] (walk/postwalk diff --git a/test/metabase/models/humanization_test.clj b/test/metabase/models/humanization_test.clj index 699f9e52b5a64976de3f6d4fbd42e63f9e89a84f..d13607b8f93157d26d299c2d02838675951122f4 100644 --- a/test/metabase/models/humanization_test.clj +++ b/test/metabase/models/humanization_test.clj @@ -7,87 +7,6 @@ [toucan.db :as db] [toucan.util.test :as tt])) -(deftest simple-humanization-test - (doseq [[input expected] {nil nil - "" nil - "_" "_" - "-" "-" - "_id" "ID" - "uid" "UID" - "uuid" "UUID" - "guid" "GUID" - "ip" "IP" - "url" "URL" - "_agent_invite_migration" "Agent Invite Migration" - "-agent-invite-migration" "Agent Invite Migration" - "fooBar" "FooBar" - "foo-bar" "Foo Bar" - "foo_bar" "Foo Bar" - "foo bar" "Foo Bar" - "dashboardcardsubscription" "Dashboardcardsubscription" - "foo_id" "Foo ID" - "fooID" "FooID" - "receiver_id" "Receiver ID" - "inbox" "Inbox" - "acquirer" "Acquirer" - "auth_authenticator" "Auth Authenticator" - "authprovider" "Authprovider" - "usersocialauth" "Usersocialauth"}] - (testing (pr-str (list 'name->human-readable-name :simple input)) - (is (= expected - (humanization/name->human-readable-name :simple input)))) - ;; there used to be an advanced mode but it should just act as simple mode now - (testing (pr-str (list 'name->human-readable-name :advanced input)) - (is (= expected - (humanization/name->human-readable-name :advanced input)))))) - -(deftest none-humanization-test - (doseq [input [nil - "" - "_" - "-" - "_id" - "uid" - "uuid" - "guid" - "ip" - "url" - "_agent_invite_migration" - "-agent-invite-migration" - "fooBar" - "foo-bar" - "foo_bar" - "foo bar" - "dashboardcardsubscription" - "foo_id" - "receiver_id" - "inbox" - "acquirer" - "auth_authenticator" - "authprovider" - "usersocialauth"]] - (testing (pr-str (list 'name->human-readable-name :none input)) - (is (= input - (humanization/name->human-readable-name :none input)))))) - -(deftest db-inspired-test - (doseq [[input strategy->expected] {"sum(subtotal)" {:simple "Sum(subtotal)" - :none "sum(subtotal)"} - "created_at::date" {:simple "Created At::date" - :none "created_at::date"} - "datecreated" {:simple "Datecreated" - :none "datecreated"} - "createdat" {:simple "Createdat" - :none "createdat"} - "updatedat" {:simple "Updatedat" - :none "updatedat"} - "cast(createdatasdate)" {:simple "Cast(createdatasdate)" - :none "cast(createdatasdate)"}} - [strategy expected] strategy->expected] - (testing (pr-str (list 'name->human-readable-name strategy input)) - (is (= expected - (humanization/name->human-readable-name strategy input)))))) - (defn- get-humanized-display-name [actual-name strategy] (with-redefs [humanization/humanization-strategy (constantly strategy)] (tt/with-temp Table [{table-id :id} {:name actual-name}] diff --git a/test/metabase/query_processor/middleware/visualization_settings_test.clj b/test/metabase/query_processor/middleware/visualization_settings_test.clj index 354a2c5ee77026d573a408456b4327a92f49daf1..26aeeab7d98e08b5343f189bc20d34c90a64ff85 100644 --- a/test/metabase/query_processor/middleware/visualization_settings_test.clj +++ b/test/metabase/query_processor/middleware/visualization_settings_test.clj @@ -114,7 +114,7 @@ {::mb.viz/column-name "TAX"} {::mb.viz/column-title "Tax" ::mb.viz/number-style "currency"}, {::mb.viz/column-name "SUBTOTAL"} {::mb.viz/column-title "Subtotal" ::mb.viz/number-style "currency" ::mb.viz/decimals 2}}}) -(deftest native-query-viz-settings-test +(deftest ^:parallel native-query-viz-settings-test (testing "Viz settings for native queries are pulled out of the query map but not modified" (let [query (test-query [] nil test-native-query-viz-settings :native) result (update-viz-settings query)] diff --git a/test/metabase/util/humanization_test.cljc b/test/metabase/util/humanization_test.cljc new file mode 100644 index 0000000000000000000000000000000000000000..0c7dc0c855426a08c5d8f366f090fb3c38a9e343 --- /dev/null +++ b/test/metabase/util/humanization_test.cljc @@ -0,0 +1,77 @@ +(ns metabase.util.humanization-test + (:require + [clojure.test :refer [are deftest]] + [metabase.util.humanization :as humanization.impl])) + +(deftest ^:parallel simple-humanization-test + (are [input expected] (= expected + (humanization.impl/name->human-readable-name :simple input)) + nil nil + "" nil + "_" "_" + "-" "-" + "_id" "ID" + "uid" "UID" + "uuid" "UUID" + "guid" "GUID" + "ip" "IP" + "url" "URL" + "_agent_invite_migration" "Agent Invite Migration" + "-agent-invite-migration" "Agent Invite Migration" + "fooBar" "FooBar" + "foo-bar" "Foo Bar" + "foo_bar" "Foo Bar" + "foo bar" "Foo Bar" + "dashboardcardsubscription" "Dashboardcardsubscription" + "foo_id" "Foo ID" + "fooID" "FooID" + "receiver_id" "Receiver ID" + "inbox" "Inbox" + "acquirer" "Acquirer" + "auth_authenticator" "Auth Authenticator" + "authprovider" "Authprovider" + "usersocialauth" "Usersocialauth")) + +(deftest ^:parallel none-humanization-test + (are [input] (= input + (humanization.impl/name->human-readable-name :none input)) + nil + "" + "_" + "-" + "_id" + "uid" + "uuid" + "guid" + "ip" + "url" + "_agent_invite_migration" + "-agent-invite-migration" + "fooBar" + "foo-bar" + "foo_bar" + "foo bar" + "dashboardcardsubscription" + "foo_id" + "receiver_id" + "inbox" + "acquirer" + "auth_authenticator" + "authprovider" + "usersocialauth")) + +(deftest ^:parallel db-inspired-test + (are [input strategy expected] (= expected + (humanization.impl/name->human-readable-name strategy input)) + "sum(subtotal)" :simple "Sum(subtotal)" + "sum(subtotal)" :none "sum(subtotal)" + "created_at::date" :simple "Created At::date" + "created_at::date" :none "created_at::date" + "datecreated" :simple "Datecreated" + "datecreated" :none "datecreated" + "createdat" :simple "Createdat" + "createdat" :none "createdat" + "updatedat" :simple "Updatedat" + "updatedat" :none "updatedat" + "cast(createdatasdate)" :simple "Cast(createdatasdate)" + "cast(createdatasdate)" :none "cast(createdatasdate)"))