diff --git a/shared/src/metabase/mbql/schema.cljc b/shared/src/metabase/mbql/schema.cljc index 07d746c51222db57bb6b850a418c4138805e33ac..fc94832dbadc2ef3bfe220e451c037484799da04 100644 --- a/shared/src/metabase/mbql/schema.cljc +++ b/shared/src/metabase/mbql/schema.cljc @@ -1858,7 +1858,8 @@ (mr/validator Query)) (def ^{:arglists '([query])} validate-query - "Validator for an outer query; throw an Exception explaining why the query is invalid if it is." + "Validator for an outer query; throw an Exception explaining why the query is invalid if it is. Returns query if + valid." (let [explainer (mr/explainer Query)] (fn [query] (if (valid-query? query) diff --git a/src/metabase/lib/metadata/cached_provider.cljc b/src/metabase/lib/metadata/cached_provider.cljc index a0eaab0ffa0b5c69b26068297426200848348b20..3f4aa34ab9dffd44f38dbe99f08ef64a35495e9b 100644 --- a/src/metabase/lib/metadata/cached_provider.cljc +++ b/src/metabase/lib/metadata/cached_provider.cljc @@ -9,6 +9,8 @@ [metabase.util.malli :as mu] #?@(:clj ([pretty.core :as pretty])))) +#?(:clj (set! *warn-on-reflection* true)) + (defn- get-in-cache [cache ks] (when-some [cached-value (get-in @cache ks)] (when-not (= cached-value ::nil) @@ -109,6 +111,12 @@ (bulk-metadata [_this metadata-type ids] (bulk-metadata cache metadata-provider metadata-type ids)) + #?(:clj Object :cljs IEquiv) + (#?(:clj equals :cljs -equiv) [_this another] + (and (instance? CachedProxyMetadataProvider another) + (= metadata-provider + (#?(:clj .metadata-provider :cljs .-metadata-provider) ^CachedProxyMetadataProvider another)))) + #?@(:clj [pretty/PrettyPrintable (pretty [_this] diff --git a/src/metabase/lib/metadata/composed_provider.cljc b/src/metabase/lib/metadata/composed_provider.cljc index a5af45dddb075789d4bfc2529c6b4889a34f2f24..90215f585fd1d6cbcd2c5de43922175a648597ca 100644 --- a/src/metabase/lib/metadata/composed_provider.cljc +++ b/src/metabase/lib/metadata/composed_provider.cljc @@ -22,38 +22,46 @@ (m/distinct-by :id)) metadata-providers)) +(deftype ComposedMetadataProvider [metadata-providers] + metadata.protocols/MetadataProvider + (database [_this] (some metadata.protocols/database metadata-providers)) + (table [_this table-id] (object-for-id metadata.protocols/table table-id metadata-providers)) + (field [_this field-id] (object-for-id metadata.protocols/field field-id metadata-providers)) + (card [_this card-id] (object-for-id metadata.protocols/card card-id metadata-providers)) + (metric [_this metric-id] (object-for-id metadata.protocols/metric metric-id metadata-providers)) + (segment [_this segment-id] (object-for-id metadata.protocols/segment segment-id metadata-providers)) + (setting [_this setting-name] (object-for-id metadata.protocols/setting setting-name metadata-providers)) + (tables [_this] (m/distinct-by :id (mapcat metadata.protocols/tables metadata-providers))) + (fields [_this table-id] (objects-for-table-id metadata.protocols/fields table-id metadata-providers)) + (metrics [_this table-id] (objects-for-table-id metadata.protocols/metrics table-id metadata-providers)) + (segments [_this table-id] (objects-for-table-id metadata.protocols/segments table-id metadata-providers)) + + metadata.protocols/CachedMetadataProvider + (cached-database [_this] + (some metadata.protocols/cached-database + (cached-providers metadata-providers))) + (cached-metadata [_this metadata-type id] + (some #(metadata.protocols/cached-metadata % metadata-type id) + (cached-providers metadata-providers))) + (store-database! [_this database-metadata] + (when-first [provider (cached-providers metadata-providers)] + (metadata.protocols/store-database! provider database-metadata))) + (store-metadata! [_this metadata-type id metadata] + (when-first [provider (cached-providers metadata-providers)] + (metadata.protocols/store-metadata! provider metadata-type id metadata))) + + #?(:clj Object :cljs IEquiv) + (#?(:clj equals :cljs -equiv) [_this another] + (and (instance? ComposedMetadataProvider another) + (= metadata-providers + (#?(:clj .metadata-providers :cljs .-metadata-providers ) ^ComposedMetadataProvider another)))) + + clojure.core.protocols/Datafiable + (datafy [_this] + (cons `composed-metadata-provider (map datafy/datafy metadata-providers)))) + (defn composed-metadata-provider "A metadata provider composed of several different `metadata-providers`. Methods try each constituent provider in turn from left to right until one returns a truthy result." [& metadata-providers] - (reify - metadata.protocols/MetadataProvider - (database [_this] (some metadata.protocols/database metadata-providers)) - (table [_this table-id] (object-for-id metadata.protocols/table table-id metadata-providers)) - (field [_this field-id] (object-for-id metadata.protocols/field field-id metadata-providers)) - (card [_this card-id] (object-for-id metadata.protocols/card card-id metadata-providers)) - (metric [_this metric-id] (object-for-id metadata.protocols/metric metric-id metadata-providers)) - (segment [_this segment-id] (object-for-id metadata.protocols/segment segment-id metadata-providers)) - (setting [_this setting-name] (object-for-id metadata.protocols/setting setting-name metadata-providers)) - (tables [_this] (m/distinct-by :id (mapcat metadata.protocols/tables metadata-providers))) - (fields [_this table-id] (objects-for-table-id metadata.protocols/fields table-id metadata-providers)) - (metrics [_this table-id] (objects-for-table-id metadata.protocols/metrics table-id metadata-providers)) - (segments [_this table-id] (objects-for-table-id metadata.protocols/segments table-id metadata-providers)) - - metadata.protocols/CachedMetadataProvider - (cached-database [_this] - (some metadata.protocols/cached-database - (cached-providers metadata-providers))) - (cached-metadata [_this metadata-type id] - (some #(metadata.protocols/cached-metadata % metadata-type id) - (cached-providers metadata-providers))) - (store-database! [_this database-metadata] - (when-first [provider (cached-providers metadata-providers)] - (metadata.protocols/store-database! provider database-metadata))) - (store-metadata! [_this metadata-type id metadata] - (when-first [provider (cached-providers metadata-providers)] - (metadata.protocols/store-metadata! provider metadata-type id metadata))) - - clojure.core.protocols/Datafiable - (datafy [_this] - (cons `composed-metadata-provider (map datafy/datafy metadata-providers))))) + (->ComposedMetadataProvider metadata-providers)) diff --git a/src/metabase/lib/metadata/jvm.clj b/src/metabase/lib/metadata/jvm.clj index 2fcd1f994a6195ad4e146ef3eaf1b7387220c9e7..8ddb18480dd1c8cbfa95d68fa88f8533d4f7d750 100644 --- a/src/metabase/lib/metadata/jvm.clj +++ b/src/metabase/lib/metadata/jvm.clj @@ -19,6 +19,8 @@ [toucan2.core :as t2] [toucan2.model :as t2.model])) +(set! *warn-on-reflection* true) + (def ^:private MetadataType [:enum :metadata/database @@ -336,6 +338,11 @@ (bulk-metadata [_this metadata-type ids] (bulk-instances metadata-type database-id ids)) + Object + (equals [_this another] + (and (instance? UncachedApplicationDatabaseMetadataProvider another) + (= database-id (.database-id ^UncachedApplicationDatabaseMetadataProvider another)))) + pretty/PrettyPrintable (pretty [_this] (list `->UncachedApplicationDatabaseMetadataProvider database-id))) diff --git a/src/metabase/query_processor/middleware/optimize_temporal_filters.clj b/src/metabase/query_processor/middleware/optimize_temporal_filters.clj index 880630beefeb5caf02f9ea21de8585c15dfcaa96..08dd23d8beda29ba8179935e9234a98315f1e6cf 100644 --- a/src/metabase/query_processor/middleware/optimize_temporal_filters.clj +++ b/src/metabase/query_processor/middleware/optimize_temporal_filters.clj @@ -3,10 +3,10 @@ `optimize-temporal-filters` for more details." (:require [clojure.walk :as walk] + [metabase.lib.hierarchy :as lib.hierarchy] [metabase.mbql.util :as mbql.u] [metabase.util :as u] [metabase.util.date-2 :as u.date] - [metabase.util.i18n :refer [trs]] [metabase.util.log :as log] [metabase.util.malli :as mu] [metabase.util.malli.schema :as ms])) @@ -14,54 +14,93 @@ (def ^:private optimizable-units #{:second :minute :hour :day :week :month :quarter :year}) -(defn- temporal-unit [field] - (mbql.u/match-one field [:field _ (opts :guard :temporal-unit)] (:temporal-unit opts))) - -(defn- optimizable-field? [field] - (mbql.u/match-one field - [:field _ (_ :guard (comp optimizable-units :temporal-unit))])) - -(defmulti ^:private can-optimize-filter? - mbql.u/dispatch-by-clause-name-or-class) - -(defn- optimizable-temporal-value? - "Can `temporal-value` clause can be optimized?" - [temporal-value] - (mbql.u/match-one temporal-value - [:relative-datetime (_ :guard #{0 :current})] - true - - [(_ :guard #{:absolute-datetime :relative-datetime}) _ (unit :guard optimizable-units)] - true)) - -(defn- field-and-temporal-value-have-compatible-units? - "Do datetime `field` clause and `temporal-value` clause have 'compatible' units that mean we'll be able to optimize - the filter clause they're in?" - [field temporal-value] - (mbql.u/match-one temporal-value - [:relative-datetime (_ :guard #{0 :current})] - true - - [(_ :guard #{:absolute-datetime :relative-datetime}) _ (unit :guard optimizable-units)] - (= (temporal-unit field) unit))) - -(defmethod can-optimize-filter? :default - [filter-clause] - (mbql.u/match-one filter-clause - [_ - (field :guard optimizable-field?) - (temporal-value :guard optimizable-temporal-value?)] - (field-and-temporal-value-have-compatible-units? field temporal-value))) - -(defmethod can-optimize-filter? :between - [filter-clause] - (mbql.u/match-one filter-clause - [_ - (field :guard optimizable-field?) - (temporal-value-1 :guard optimizable-temporal-value?) - (temporal-value-2 :guard optimizable-temporal-value?)] - (and (field-and-temporal-value-have-compatible-units? field temporal-value-1) - (field-and-temporal-value-have-compatible-units? field temporal-value-2)))) +(defmulti ^:private temporal-unit + {:arglists '([expression])} + mbql.u/dispatch-by-clause-name-or-class + :hierarchy lib.hierarchy/hierarchy) + +(defmethod temporal-unit :default + [_expr] + nil) + +(defmethod temporal-unit :field + [[_field _id-or-name opts]] + (:temporal-unit opts)) + +(defmethod temporal-unit :relative-datetime + [[_relative-datetime _n unit]] + unit) + +(defmethod temporal-unit :absolute-datetime + [[_absolute-datetime _t unit]] + unit) + +(doseq [tag [:+ :-]] + (lib.hierarchy/derive tag ::arithmetic-expr)) + +(defmethod temporal-unit ::arithmetic-expr + [[_tag & args]] + (some temporal-unit args)) + +(defmulti ^:private optimizable? + {:arglists '([expression])} + mbql.u/dispatch-by-clause-name-or-class + :hierarchy lib.hierarchy/hierarchy) + +(defmethod optimizable? :default + [_expr] + false) + +(defmethod optimizable? :field + [[_field _id-or-name opts]] + (contains? optimizable-units (:temporal-unit opts))) + +(defmethod optimizable? :expression + [[_expression _name opts]] + (let [expression-type ((some-fn :effective-type :base-type) opts)] + (isa? expression-type :type/Temporal))) + +(defmethod optimizable? :relative-datetime + [[_relative-datetime n unit]] + (if unit + (contains? optimizable-units unit) + (#{0 :current} n))) + +(defmethod optimizable? :absolute-datetime + [[_absolute-datetime _t unit]] + (contains? optimizable-units unit)) + +(defmethod optimizable? ::arithmetic-expr + [[_tag & args]] + (some optimizable? args)) + +(doseq [tag [:= :!= :< :<= :> :>=]] + (lib.hierarchy/derive tag ::binary-filter)) + +(defn- compatible-units? + "The temporal units for two expressions *x* and *y* are compatible for optimization purposes if + + a. They both have units, and those units are the same + + b. One of them has a unit, but the other does not." + [x-unit y-unit] + (if (and x-unit y-unit) + (= x-unit y-unit) + (or x-unit y-unit))) + +(defmethod optimizable? ::binary-filter + [[_tag lhs rhs]] + (and (optimizable? lhs) + (optimizable? rhs) + (compatible-units? (temporal-unit lhs) (temporal-unit rhs)))) + +(defmethod optimizable? :between + [[_between expr lower-bound upper-bound]] + (and (optimizable? expr) + (optimizable? lower-bound) + (optimizable? upper-bound) + (compatible-units? (temporal-unit expr) (temporal-unit lower-bound)) + (compatible-units? (temporal-unit expr) (temporal-unit upper-bound)))) (mu/defn ^:private temporal-literal-lower-bound [unit t :- (ms/InstanceOfClass java.time.temporal.Temporal)] @@ -71,9 +110,9 @@ [unit t :- (ms/InstanceOfClass java.time.temporal.Temporal)] (:end (u.date/range t unit))) -(defn- change-temporal-unit-to-default [field] - (mbql.u/replace field - [:field _ (_ :guard (comp optimizable-units :temporal-unit))] +(defn- change-temporal-unit-to-default [expression] + (mbql.u/replace expression + [:field _id-or-name (_opts :guard (comp optimizable-units :temporal-unit))] (mbql.u/update-field-options &match assoc :temporal-unit :default))) (defmulti ^:private temporal-value-lower-bound @@ -108,26 +147,25 @@ "Optimize a filter clause against a temporal-bucketed `:field` clause and `:absolute-datetime` or `:relative-datetime` value by converting to an unbucketed range." {:arglists '([clause])} - mbql.u/dispatch-by-clause-name-or-class) + mbql.u/dispatch-by-clause-name-or-class + :hierarchy lib.hierarchy/hierarchy) (defmethod optimize-filter := - [[_tag field temporal-value]] - (let [temporal-unit (mbql.u/match-one field [:field _ (opts :guard :temporal-unit)] (:temporal-unit opts))] - (when (field-and-temporal-value-have-compatible-units? field temporal-value) - (let [field' (change-temporal-unit-to-default field)] - [:and - [:>= field' (temporal-value-lower-bound temporal-value temporal-unit)] - [:< field' (temporal-value-upper-bound temporal-value temporal-unit)]])))) + [[_tag lhs rhs]] + (let [lhs' (change-temporal-unit-to-default lhs)] + [:and + [:>= lhs' (temporal-value-lower-bound rhs (temporal-unit lhs))] + [:< lhs' (temporal-value-upper-bound rhs (temporal-unit lhs))]])) (defmethod optimize-filter :!= [filter-clause] (mbql.u/negate-filter-clause ((get-method optimize-filter :=) filter-clause))) (defn- optimize-comparison-filter - [optimize-temporal-value-fn [_filter-type field temporal-value] new-filter-type] + [optimize-temporal-value-fn [_tag lhs rhs] new-filter-type] [new-filter-type - (change-temporal-unit-to-default field) - (optimize-temporal-value-fn temporal-value (temporal-unit field))]) + (change-temporal-unit-to-default lhs) + (optimize-temporal-value-fn rhs (temporal-unit lhs))]) (defmethod optimize-filter :< [filter-clause] @@ -146,16 +184,16 @@ (optimize-comparison-filter temporal-value-lower-bound filter-clause :>=)) (defmethod optimize-filter :between - [[_ field lower-bound upper-bound]] - (let [field' (change-temporal-unit-to-default field)] + [[_between expr lower-bound upper-bound]] + (let [expr' (change-temporal-unit-to-default expr)] [:and - [:>= field' (temporal-value-lower-bound lower-bound (temporal-unit field))] - [:< field' (temporal-value-upper-bound upper-bound (temporal-unit field))]])) + [:>= expr' (temporal-value-lower-bound lower-bound (temporal-unit expr))] + [:< expr' (temporal-value-upper-bound upper-bound (temporal-unit expr))]])) (defn- optimize-temporal-filters* [query] (mbql.u/replace query (_ :guard (partial mbql.u/is-clause? (set (keys (methods optimize-filter))))) - (or (when (can-optimize-filter? &match) + (or (when (optimizable? &match) (u/prog1 (optimize-filter &match) (if <> (when-not (= &match <>) @@ -163,7 +201,8 @@ ;; if for some reason `optimize-filter` doesn't return an optimized filter clause, log and error and use ;; the original. `can-optimize-filter?` shouldn't have said we could optimize this filter in the first ;; place - (log/error (trs "Error optimizing temporal filter clause") (pr-str &match))))) + (log/error "Error optimizing temporal filter clause: optimize-filter returned nil" + (pr-str &match))))) &match))) (defn optimize-temporal-filters @@ -194,13 +233,16 @@ (if (not= query-type :query) query ;; walk query, looking for inner-query forms that have a `:filter` key - (walk/postwalk - (fn [form] - (if-not (and (map? form) (seq (:filter form))) - form - ;; optimize the filters in this inner-query form. - (let [optimized (optimize-temporal-filters* form)] - ;; if we did some optimizations, we should flatten/deduplicate the filter clauses afterwards. - (cond-> optimized - (not= optimized form) (update :filter mbql.u/combine-filter-clauses))))) - query))) + (let [query' (walk/postwalk + (fn [form] + (if-not (and (map? form) (seq (:filter form))) + form + ;; optimize the filters in this inner-query form. + (let [optimized (optimize-temporal-filters* form)] + ;; if we did some optimizations, we should flatten/deduplicate the filter clauses afterwards. + (cond-> optimized + (not= optimized form) (update :filter mbql.u/combine-filter-clauses))))) + query)] + (when-not (= query query') + (log/debugf "Optimized temporal filters:\n%s" (u/pprint-to-str query))) + query'))) diff --git a/src/metabase/query_processor/middleware/wrap_value_literals.clj b/src/metabase/query_processor/middleware/wrap_value_literals.clj index 6cc88177a8ebe63d772d0430f0cc700436955370..171e07832ad29a218493c4cf912bb0c8b08b660f 100644 --- a/src/metabase/query_processor/middleware/wrap_value_literals.clj +++ b/src/metabase/query_processor/middleware/wrap_value_literals.clj @@ -7,9 +7,11 @@ [metabase.mbql.util :as mbql.u] [metabase.query-processor.store :as qp.store] [metabase.query-processor.timezone :as qp.timezone] + [metabase.shared.util.internal.time-common :as shared.ut.common] [metabase.types :as types] [metabase.util :as u] - [metabase.util.date-2 :as u.date]) + [metabase.util.date-2 :as u.date] + [metabase.util.log :as log]) (:import (java.time LocalDate LocalDateTime LocalTime OffsetDateTime OffsetTime ZonedDateTime))) @@ -45,6 +47,10 @@ (when (:base-type opts) {:base_type (:base-type opts)}))) +(defmethod type-info :expression + [[_expression _expression-name opts]] + (u/snake-keys (select-keys opts [:base-type :effective-type :temporal-unit]))) + ;;; ------------------------------------------------- add-type-info -------------------------------------------------- @@ -89,19 +95,27 @@ [this info & _] [:absolute-datetime this (get info :unit :default)]) +(defn- temporal-value? [info] + (or (:unit info) + (some-> ((some-fn :effective_type :base_type) info) (isa? :type/Temporal)))) + (defmethod add-type-info String - [this {:keys [unit], :as info} & {:keys [parse-datetime-strings?] - :or {parse-datetime-strings? true}}] - (if-let [temporal-value (when (and unit + [s {:keys [unit], :as info} & {:keys [parse-datetime-strings?] + :or {parse-datetime-strings? true}}] + (if-let [temporal-value (when (and (temporal-value? info) parse-datetime-strings? - (string? this)) + (string? s)) ;; TIMEZONE FIXME - I think this should actually use ;; (qp.timezone/report-timezone-id-if-supported) instead ? - (u.date/parse this (qp.timezone/results-timezone-id)))] - (if (some #(instance? % temporal-value) [LocalTime OffsetTime]) - [:time temporal-value unit] - [:absolute-datetime temporal-value unit]) - [:value this info])) + (u.date/parse s (qp.timezone/results-timezone-id)))] + (let [unit (or unit + (if (shared.ut.common/matches-date? s) + :day + :default))] + (if (some #(instance? % temporal-value) [LocalTime OffsetTime]) + [:time temporal-value unit] + [:absolute-datetime temporal-value unit])) + [:value s info])) ;;; -------------------------------------------- wrap-literals-in-clause --------------------------------------------- @@ -163,5 +177,8 @@ [{query-type :type, :as query}] (if-not (= query-type :query) query - (mbql.s/validate-query - (update query :query wrap-value-literals-in-mbql-query nil)))) + (let [query' (update query :query wrap-value-literals-in-mbql-query nil)] + (when-not (= query query') + (log/debugf "Wrapped literals in :value clauses:\n%s" (u/pprint-to-str query')) + (mbql.s/validate-query query')) + query'))) diff --git a/test/metabase/lib/metadata/cached_provider_test.cljc b/test/metabase/lib/metadata/cached_provider_test.cljc index c0b0df2d906aac43f21002d2b4060345e10fe6ff..b42d1b35a26de2498e7adc05aadb60cb1d4e2fcc 100644 --- a/test/metabase/lib/metadata/cached_provider_test.cljc +++ b/test/metabase/lib/metadata/cached_provider_test.cljc @@ -52,3 +52,7 @@ (lib.metadata.protocols/bulk-metadata provider :metadata/table #{1 2}))) (is (= 2 @fetch-count)))))) + +(deftest ^:parallel equality-test + (is (= (lib.metadata.cached-provider/cached-metadata-provider nil) + (lib.metadata.cached-provider/cached-metadata-provider nil)))) diff --git a/test/metabase/lib/metadata/composed_provider_test.cljc b/test/metabase/lib/metadata/composed_provider_test.cljc index e9a6a6ef56aded801f12c4dab665a07c3c20e06b..5a5bae2d11c8a864ce56f4889c4c5cd49a6b6a7c 100644 --- a/test/metabase/lib/metadata/composed_provider_test.cljc +++ b/test/metabase/lib/metadata/composed_provider_test.cljc @@ -25,3 +25,7 @@ (lib.metadata/field metadata-provider (meta/id :people :birth-date))))))) + +(deftest ^:parallel equality-test + (is (= (lib/composed-metadata-provider meta/metadata-provider) + (lib/composed-metadata-provider meta/metadata-provider)))) diff --git a/test/metabase/lib/metadata/jvm_test.clj b/test/metabase/lib/metadata/jvm_test.clj index 4cf981fe197fcff9d5ce45feaf8f1e88c9da9a52..bc434fcaf065796f29473939b4a04e1721e75205 100644 --- a/test/metabase/lib/metadata/jvm_test.clj +++ b/test/metabase/lib/metadata/jvm_test.clj @@ -193,3 +193,7 @@ (lib.metadata/card (lib.metadata.jvm/application-database-metadata-provider (mt/id)) card-id))))) + +(deftest ^:parallel equality-test + (is (= (lib.metadata.jvm/application-database-metadata-provider (mt/id)) + (lib.metadata.jvm/application-database-metadata-provider (mt/id))))) diff --git a/test/metabase/lib/test_metadata/graph_provider.cljc b/test/metabase/lib/test_metadata/graph_provider.cljc index 4b691e54afdec292f7d940cc49cbea2628cdb0e1..eea2c2a62249532d3539e4139f7abd5f64c4db82 100644 --- a/test/metabase/lib/test_metadata/graph_provider.cljc +++ b/test/metabase/lib/test_metadata/graph_provider.cljc @@ -1,6 +1,7 @@ (ns metabase.lib.test-metadata.graph-provider (:require [clojure.core.protocols] + [clojure.test :refer [deftest is]] [medley.core :as m] [metabase.lib.metadata.protocols :as lib.metadata.protocols] #?@(:clj @@ -73,9 +74,19 @@ (datafy [_this] (list `->SimpleGraphMetadataProvider metadata-graph)) + #?(:clj Object :cljs IEquiv) + (#?(:clj equals :cljs -equiv) [_this another] + (and (instance? SimpleGraphMetadataProvider another) + (= metadata-graph + (#?(:clj .metadata-graph :cljs .-metadata-graph) ^SimpleGraphMetadataProvider another)))) + #?@(:clj [pretty/PrettyPrintable (pretty [_this] - (if (identical? metadata-graph @(requiring-resolve 'metabase.lib.test-metadata/metadata)) - 'metabase.lib.test-metadata/metadata-provider - (list `->SimpleGraphMetadataProvider metadata-graph)))])) + (if (identical? metadata-graph @(requiring-resolve 'metabase.lib.test-metadata/metadata)) + 'metabase.lib.test-metadata/metadata-provider + (list `->SimpleGraphMetadataProvider metadata-graph)))])) + +(deftest ^:parallel equality-test + (is (= (->SimpleGraphMetadataProvider {}) + (->SimpleGraphMetadataProvider {})))) diff --git a/test/metabase/lib/test_util/metadata_providers/mock.cljc b/test/metabase/lib/test_util/metadata_providers/mock.cljc index 8dc447c101bed6a6783513836f352e5fda736a42..0d6dc7c2f27b1c28b5792a2806859adc0e082437 100644 --- a/test/metabase/lib/test_util/metadata_providers/mock.cljc +++ b/test/metabase/lib/test_util/metadata_providers/mock.cljc @@ -1,10 +1,12 @@ (ns metabase.lib.test-util.metadata-providers.mock (:require [clojure.core.protocols] + [clojure.test :refer [deftest is]] [medley.core :as m] [metabase.lib.core :as lib] [metabase.lib.metadata :as lib.metadata] [metabase.lib.metadata.protocols :as metadata.protocols] + [metabase.lib.test-metadata :as meta] [metabase.util.malli :as mu])) (defn- with-optional-lib-type @@ -27,6 +29,47 @@ [:segments {:optional true} [:maybe [:sequential (with-optional-lib-type lib.metadata/SegmentMetadata :metadata/segment)]]] [:settings {:optional true} [:maybe [:map-of :keyword any?]]]]) +(deftype MockMetadataProvider [metadata] + metadata.protocols/MetadataProvider + (database [_this] (some-> (:database metadata) + (assoc :lib/type :metadata/database) + (dissoc :tables))) + (table [_this table-id] (some-> (m/find-first #(= (:id %) table-id) (:tables metadata)) + (assoc :lib/type :metadata/table) + (dissoc :fields))) + (field [_this field-id] (some-> (m/find-first #(= (:id %) field-id) (:fields metadata)) + (assoc :lib/type :metadata/column))) + (card [_this card-id] (some-> (m/find-first #(= (:id %) card-id) (:cards metadata)) + (assoc :lib/type :metadata/card))) + (metric [_this metric-id] (some-> (m/find-first #(= (:id %) metric-id) (:metrics metadata)) + (assoc :lib/type :metadata/metric))) + (segment [_this segment-id] (some-> (m/find-first #(= (:id %) segment-id) (:segments metadata)) + (assoc :lib/type :metadata/segment))) + (tables [_this] (for [table (:tables metadata)] + (-> (assoc table :lib/type :metadata/table) + (dissoc :fields)))) + (fields [_this table-id] (for [field (:fields metadata) + :when (= (:table-id field) table-id)] + (assoc field :lib/type :metadata/column))) + (metrics [_this table-id] (for [metric (:metrics metadata) + :when (= (:table-id metric) table-id)] + (assoc metric :lib/type :metadata/metric))) + (segments [_this table-id] (for [segment (:segments metadata) + :when (= (:table-id segment) table-id)] + (assoc segment :lib/type :metadata/segment))) + + (setting [_this setting] (get-in metadata [:settings (keyword setting)])) + + #?(:clj Object :cljs IEquiv) + (#?(:clj equals :cljs -equiv) [_this another] + (and (instance? MockMetadataProvider another) + (= metadata + (#?(:clj .metadata :cljs .-metadata) ^MockMetadataProvider another)))) + + clojure.core.protocols/Datafiable + (datafy [_this] + (list `mock-metadata-provider metadata))) + (mu/defn mock-metadata-provider :- lib.metadata/MetadataProvider "Create a mock metadata provider to facilitate writing tests. All keys except `:database` should be a sequence of maps e.g. @@ -42,43 +85,19 @@ (lib.tu/mock-metadata-provider parent-metadata-provider {...}) => (lib/composed-metadata-provider (lib.tu/mock-metadata-provider {...}) parent-metadata-provider)" - ([{:keys [database tables fields cards metrics segments settings] :as m} :- MockMetadata] - (reify - metadata.protocols/MetadataProvider - (database [_this] (some-> database - (assoc :lib/type :metadata/database) - (dissoc :tables))) - (table [_this table-id] (some-> (m/find-first #(= (:id %) table-id) tables) - (assoc :lib/type :metadata/table) - (dissoc :fields))) - (field [_this field-id] (some-> (m/find-first #(= (:id %) field-id) fields) - (assoc :lib/type :metadata/column))) - (card [_this card-id] (some-> (m/find-first #(= (:id %) card-id) cards) - (assoc :lib/type :metadata/card))) - (metric [_this metric-id] (some-> (m/find-first #(= (:id %) metric-id) metrics) - (assoc :lib/type :metadata/metric))) - (segment [_this segment-id] (some-> (m/find-first #(= (:id %) segment-id) segments) - (assoc :lib/type :metadata/segment))) - (tables [_this] (for [table tables] - (-> (assoc table :lib/type :metadata/table) - (dissoc :fields)))) - (fields [_this table-id] (for [field fields - :when (= (:table-id field) table-id)] - (assoc field :lib/type :metadata/column))) - (metrics [_this table-id] (for [metric metrics - :when (= (:table-id metric) table-id)] - (assoc metric :lib/type :metadata/metric))) - (segments [_this table-id] (for [segment segments - :when (= (:table-id segment) table-id)] - (assoc segment :lib/type :metadata/segment))) - - (setting [_this setting] (get settings (keyword setting))) - - clojure.core.protocols/Datafiable - (datafy [_this] - (list `mock-metadata-provider m)))) + ([m :- MockMetadata] + (->MockMetadataProvider m)) ([parent-metadata-provider mock-metadata] (lib/composed-metadata-provider (mock-metadata-provider mock-metadata) parent-metadata-provider))) + +(deftest ^:parallel equality-test + (let [time-field (assoc (meta/field-metadata :people :birth-date) + :base-type :type/Time + :effective-type :type/Time)] + (is (= (mock-metadata-provider + {:fields [time-field]}) + (mock-metadata-provider + {:fields [time-field]}))))) diff --git a/test/metabase/query_processor/middleware/optimize_temporal_filters_test.clj b/test/metabase/query_processor/middleware/optimize_temporal_filters_test.clj index 22cf7fd9a8c2d2ff7ccf763d1dd9225743d8a246..21cd48ab91ee7cc8ed1dcc9d03f40eed5da7ac0c 100644 --- a/test/metabase/query_processor/middleware/optimize_temporal_filters_test.clj +++ b/test/metabase/query_processor/middleware/optimize_temporal_filters_test.clj @@ -21,7 +21,7 @@ (-> (optimize-temporal-filters/optimize-temporal-filters query) (get-in [:query :filter])))) -(deftest optimize-day-bucketed-filter-test +(deftest ^:parallel optimize-day-bucketed-filter-test (testing "Make sure we aren't doing anything wacky when optimzing filters against fields bucketed by day" (letfn [(optimize [filter-type] (#'optimize-temporal-filters/optimize-filter @@ -87,7 +87,7 @@ :lower (u.date/parse "2019-01-01" "UTC") :upper (u.date/parse "2020-01-01" "UTC")}]) -(deftest optimize-temporal-filters-test +(deftest ^:parallel optimize-temporal-filters-test (driver/with-driver ::timezone-driver (doseq [{:keys [unit filter-value lower upper]} test-units-and-values] (let [lower [:absolute-datetime lower :default] @@ -152,7 +152,7 @@ (-> (optimize-temporal-filters/optimize-temporal-filters query) (get-in [:query :filter])))) -(deftest timezones-test +(deftest ^:parallel timezones-test (driver/with-driver ::timezone-driver (doseq [timezone-id ["UTC" "US/Pacific"]] (testing (format "%s timezone" timezone-id) @@ -176,7 +176,7 @@ (format "= %s in the %s timezone should be optimized to range %s -> %s" t timezone-id lower upper)))))))))) -(deftest skip-optimization-test +(deftest ^:parallel skip-optimization-test (let [clause [:= [:field 1 {:temporal-unit :day}] [:absolute-datetime #t "2019-01-01" :month]]] (is (= clause (optimize-temporal-filters clause)) @@ -192,36 +192,46 @@ (str/replace #"\"" "") (str/replace #"PUBLIC\." ""))))) -(deftest e2e-test +(deftest ^:parallel e2e-test (testing := (is (= {:query "WHERE (CHECKINS.DATE >= ?) AND (CHECKINS.DATE < ?)" :params [(t/zoned-date-time (t/local-date 2019 9 24) (t/local-time 0) "UTC") (t/zoned-date-time (t/local-date 2019 9 25) (t/local-time 0) "UTC")]} (mt/$ids checkins - (filter->sql [:= !day.date "2019-09-24T12:00:00.000Z"]))))) + (filter->sql [:= !day.date "2019-09-24T12:00:00.000Z"])))))) + +(deftest ^:parallel e2e-test-2 (testing :< (is (= {:query "WHERE CHECKINS.DATE < ?" :params [(t/zoned-date-time (t/local-date 2019 9 24) (t/local-time 0) "UTC")]} (mt/$ids checkins - (filter->sql [:< !day.date "2019-09-24T12:00:00.000Z"]))))) + (filter->sql [:< !day.date "2019-09-24T12:00:00.000Z"])))))) + +(deftest ^:parallel e2e-test-3 (testing :between (is (= {:query "WHERE (CHECKINS.DATE >= ?) AND (CHECKINS.DATE < ?)" :params [(t/zoned-date-time (t/local-date 2019 9 1) (t/local-time 0) "UTC") (t/zoned-date-time (t/local-date 2019 11 1) (t/local-time 0) "UTC")]} (mt/$ids checkins - (filter->sql [:between !month.date "2019-09-02T12:00:00.000Z" "2019-10-05T12:00:00.000Z"])))) - (is (= {:query "WHERE (CHECKINS.DATE >= ?) AND (CHECKINS.DATE < ?)" - :params [(t/zoned-date-time "2019-09-01T00:00Z[UTC]") - (t/zoned-date-time "2019-10-02T00:00Z[UTC]")]} + (filter->sql [:between !month.date "2019-09-02T12:00:00.000Z" "2019-10-05T12:00:00.000Z"])))))) + +(deftest ^:parallel e2e-test-4 + (testing :between + (is (= {:query "WHERE (CHECKINS.DATE >= ?) AND (CHECKINS.DATE < ?)" + :params [(t/zoned-date-time "2019-09-01T00:00Z[UTC]") + (t/zoned-date-time "2019-10-02T00:00Z[UTC]")]} (mt/$ids checkins (filter->sql [:between !day.date "2019-09-01" "2019-10-01"])))))) -(deftest optimize-relative-datetimes-test +(def ^:private relative-datetime-units + "second/millisecond are not currently allowed in `:relative-datetime`, but if we add them we can re-enable their + tests." + [#_:millisecond #_:second :minute :hour :day :week :month :quarter :year]) + +(deftest ^:parallel optimize-relative-datetimes-last-test (testing "Should optimize relative-datetime clauses (#11837)" (mt/dataset attempted-murders - ;; second/millisecond are not currently allowed in `:relative-datetime`, but if we add them we can re-enable - ;; their tests - (doseq [unit [#_:millisecond #_:second :minute :hour :day :week :month :quarter :year]] + (doseq [unit relative-datetime-units] (testing (format "last %s" unit) (is (= (mt/mbql-query attempts {:aggregation [[:count]] @@ -238,7 +248,12 @@ {:aggregation [[:count]] :filter [:= [:field %datetime {:temporal-unit unit}] - [:relative-datetime -1 unit]]}))))) + [:relative-datetime -1 unit]]}))))))))) + +(deftest ^:parallel optimize-relative-datetimes-this-test + (testing "Should optimize relative-datetime clauses (#11837)" + (mt/dataset attempted-murders + (doseq [unit relative-datetime-units] (testing (format "this %s" unit) ;; test the various different ways we might refer to 'now' (doseq [clause [[:relative-datetime 0] @@ -259,7 +274,12 @@ {:aggregation [[:count]] :filter [:= [:field %datetime {:temporal-unit unit}] - clause]}))))))) + clause]}))))))))))) + +(deftest ^:parallel optimize-relative-datetimes-next-test + (testing "Should optimize relative-datetime clauses (#11837)" + (mt/dataset attempted-murders + (doseq [unit relative-datetime-units] (testing (format "next %s" unit) (is (= (mt/mbql-query attempts {:aggregation [[:count]] @@ -277,7 +297,7 @@ [:field %datetime {:temporal-unit unit}] [:relative-datetime 1 unit]]}))))))))) -(deftest optimize-mixed-temporal-values-test +(deftest ^:parallel optimize-mixed-temporal-values-test (testing "We should be able to optimize mixed usages of `:absolute-datetime` and `:relative-datetime`" (mt/dataset attempted-murders (testing "between month(2021-01-15) and month(now) [inclusive]" @@ -299,7 +319,7 @@ [:absolute-datetime #t "2021-01-15T00:00:00Z" :month] [:relative-datetime 0]]})))))))) -(deftest optimize-relative-datetimes-e2e-test +(deftest ^:parallel optimize-relative-datetimes-e2e-test (testing "Should optimize relative-datetime clauses (#11837)" (mt/dataset attempted-murders (is (= (str "SELECT COUNT(*) AS \"count\" " @@ -316,7 +336,7 @@ {:aggregation [[:count]] :filter [:time-interval $datetime :last :month]})))))))) -(deftest flatten-filters-test +(deftest ^:parallel flatten-filters-test (testing "Should flatten the `:filter` clause after optimizing" (mt/$ids checkins (is (= [:and @@ -328,7 +348,7 @@ [:= $venue_id 1] [:= [:field %date {:temporal-unit :month}] [:relative-datetime -1 :month]]])))))) -(deftest deduplicate-filters-tets +(deftest ^:parallel deduplicate-filters-tets (testing "Should deduplicate the optimized filters with any existing ones" (mt/$ids checkins (is (= [:and @@ -339,7 +359,7 @@ [:< [:field %date {:temporal-unit :default}] [:relative-datetime 0 :month]] [:= [:field %date {:temporal-unit :month}] [:relative-datetime -1 :month]]])))))) -(deftest optimize-filters-all-levels-test +(deftest ^:parallel optimize-filters-all-levels-test (testing "Should optimize filters at all levels of the query" (mt/$ids checkins (is (= (mt/mbql-query checkins @@ -354,7 +374,7 @@ {:source-table $$checkins :filter [:= [:field %date {:temporal-unit :month}] [:relative-datetime -1 :month]]}}))))))) -(deftest optimize-filter-with-nested-compatible-field +(deftest ^:parallel optimize-filter-with-nested-compatible-field (testing "Should optimize fields when starting n :temporal-unit ago (#25378)" (mt/$ids users (is (= (mt/mbql-query users @@ -377,3 +397,31 @@ :source-query {:source-table $$users :aggregation [[:count]] :breakout [[:field %last_login {:temporal-unit :month}]]}}))))))) + +(deftest ^:parallel optimize-filter-with-expression-ref-test + (is (= [:and + [:>= + [:expression "CC Created At" {:base-type :type/DateTimeWithLocalTZ}] + [:absolute-datetime #t "2017-10-07" :default]] + [:< + [:expression "CC Created At" {:base-type :type/DateTimeWithLocalTZ}] + [:absolute-datetime #t "2017-10-08" :default]]] + (#'optimize-temporal-filters/optimize-filter + [:= + [:expression "CC Created At" {:base-type :type/DateTimeWithLocalTZ}] + [:absolute-datetime #t "2017-10-07" :day]])))) + +(deftest ^:parallel optimize-filter-with-expression-ref-test-2 + (testing ":between is inclusive" + (is (= [:and + [:>= + [:expression "CC Created At" {:base-type :type/DateTimeWithLocalTZ}] + [:absolute-datetime #t "2017-10-07" :default]] + [:< + [:expression "CC Created At" {:base-type :type/DateTimeWithLocalTZ}] + [:absolute-datetime #t "2017-10-09" :default]]] + (#'optimize-temporal-filters/optimize-filter + [:between + [:expression "CC Created At" {:base-type :type/DateTimeWithLocalTZ}] + [:absolute-datetime #t "2017-10-07" :day] + [:absolute-datetime #t "2017-10-08" :day]]))))) diff --git a/test/metabase/query_processor/middleware/wrap_value_literals_test.clj b/test/metabase/query_processor/middleware/wrap_value_literals_test.clj index ce7ec108a4958cba693fd22cb078173b9bccbf16..bac3097b17ebf6074a55cb373266d4e6692352f8 100644 --- a/test/metabase/query_processor/middleware/wrap_value_literals_test.clj +++ b/test/metabase/query_processor/middleware/wrap_value_literals_test.clj @@ -3,6 +3,9 @@ [clojure.test :refer :all] [java-time.api :as t] [metabase.driver :as driver] + [metabase.lib.convert :as lib.convert] + [metabase.lib.core :as lib] + [metabase.lib.metadata :as lib.metadata] [metabase.lib.test-metadata :as meta] [metabase.lib.test-util :as lib.tu] [metabase.lib.test-util.macros :as lib.tu.macros] @@ -232,3 +235,32 @@ :filter [:not [:starts-with [:field "A" {:base-type :type/Text}] "f"]], :source-query {:native "select 'foo' as a union select null as a union select 'bar' as a"}} nil))))) + +(deftest ^:parallel expression-test + (testing "Value literals compared to :expression refs should get wrapped" + (mt/dataset sample-dataset + (qp.store/with-metadata-provider (mt/id) + (let [people (lib.metadata/table (qp.store/metadata-provider) (mt/id :people)) + created-at (lib.metadata/field (qp.store/metadata-provider) (mt/id :people :created_at)) + query (as-> (lib/query (qp.store/metadata-provider) people) query + (lib/expression query "CC Created At" created-at) + (lib/filter query (lib/= + (lib/expression-ref query "CC Created At") + "2017-10-07")) + (lib/aggregate query (lib/count)))] + + (is (=? {:stages [{:filters + [[:= + {} + [:expression {:base-type :type/DateTimeWithLocalTZ} "CC Created At"] + "2017-10-07"]]}]} + query)) + (is (=? {:stages [{:lib/type :mbql.stage/mbql, + :filters [[:= + {} + [:expression {:base-type :type/DateTimeWithLocalTZ} "CC Created At"] + [:absolute-datetime {} #t "2017-10-07T00:00Z[UTC]" :day]]]}]} + (->> query + lib.convert/->legacy-MBQL + wrap-value-literals + (lib/query query))))))))) diff --git a/test/metabase/query_processor_test/filter_test.clj b/test/metabase/query_processor_test/filter_test.clj index 96e05d579c50b8d8c15536e67175b2053e856a55..83ba26543ade6d1eb3c0f5f7a86561ac09c5082f 100644 --- a/test/metabase/query_processor_test/filter_test.clj +++ b/test/metabase/query_processor_test/filter_test.clj @@ -4,10 +4,14 @@ [clojure.set :as set] [clojure.test :refer :all] [metabase.driver :as driver] + [metabase.lib.core :as lib] + [metabase.lib.metadata :as lib.metadata] [metabase.query-processor :as qp] [metabase.query-processor-test.timezones-test :as timezones-test] + [metabase.query-processor.store :as qp.store] [metabase.query-processor.test-util :as qp.test-util] - [metabase.test :as mt])) + [metabase.test :as mt] + [metabase.util :as u])) (deftest ^:parallel and-test (mt/test-drivers (mt/normal-drivers) @@ -656,3 +660,20 @@ (mt/run-mbql-query bird-count {:order-by [[:asc $count] [:asc $id]] :limit 3})))))))) + +(deftest ^:parallel date-filter-on-datetime-column-test + (testing "Filtering a DATETIME expression by a DATE literal string should do something sane (#17807)" + (mt/dataset sample-dataset + (qp.store/with-metadata-provider (mt/id) + (let [people (lib.metadata/table (qp.store/metadata-provider) (mt/id :people)) + created-at (lib.metadata/field (qp.store/metadata-provider) (mt/id :people :created_at)) + query (as-> (lib/query (qp.store/metadata-provider) people) query + (lib/expression query "CC Created At" created-at) + (lib/filter query (lib/= + (lib/expression-ref query "CC Created At") + "2017-10-07")) + (lib/aggregate query (lib/count)))] + (testing (str "\nquery =\n" (u/pprint-to-str query)) + (mt/with-native-query-testing-context query + (is (= [[2]] + (mt/rows (qp/process-query query)))))))))))