diff --git a/src/metabase/query_processor/middleware/binning.clj b/src/metabase/query_processor/middleware/binning.clj index 9dbdb046eb43d362a4ffdaed7ed739c9131f31bc..16374bf8efec39cd96501ee16d268965d1490d4b 100644 --- a/src/metabase/query_processor/middleware/binning.clj +++ b/src/metabase/query_processor/middleware/binning.clj @@ -53,7 +53,7 @@ (apply min user-maxes)) global-max)] (when-not (and min-value max-value) - (throw (ex-info (tru "Unable to bin Field without a min/max value") + (throw (ex-info (tru "Unable to bin Field without a min/max value (missing or incomplete fingerprint)") {:type qp.error-type/invalid-query :field-id field-id :fingerprint fingerprint}))) diff --git a/test/metabase/lib/drill_thru/distribution_test.cljc b/test/metabase/lib/drill_thru/distribution_test.cljc index 8cbd37a373c05378db7e3e6ea32532af082811a5..a79601d01f7b0a72ced5c9afe2d4b9c382d32220 100644 --- a/test/metabase/lib/drill_thru/distribution_test.cljc +++ b/test/metabase/lib/drill_thru/distribution_test.cljc @@ -1,4 +1,5 @@ (ns metabase.lib.drill-thru.distribution-test + "See also [[metabase.query-processor-test.drill-thru-e2e-test/distribution-drill-on-longitude-from-sql-source-card-test]]" (:require [clojure.test :refer [deftest is testing]] [medley.core :as m] diff --git a/test/metabase/lib/drill_thru/quick_filter_test.cljc b/test/metabase/lib/drill_thru/quick_filter_test.cljc index 29ef6890ef3cf349671044a526d2e65ad846351f..52ea6581fe93d2a7a0c02cff13fc888e30324816 100644 --- a/test/metabase/lib/drill_thru/quick_filter_test.cljc +++ b/test/metabase/lib/drill_thru/quick_filter_test.cljc @@ -1,4 +1,5 @@ (ns metabase.lib.drill-thru.quick-filter-test + "See also [[metabase.query-processor-test.drill-thru-e2e-test/quick-filter-on-bucketed-date-test]]" (:require [clojure.test :refer [deftest testing]] [metabase.lib.drill-thru.test-util :as lib.drill-thru.tu] diff --git a/test/metabase/query_processor/test_util.clj b/test/metabase/query_processor/test_util.clj index a48a2960bb00a7455e3460ee74a24084d027d56c..822afa1844fdf2d2442fdae15208cba2f41274e3 100644 --- a/test/metabase/query_processor/test_util.clj +++ b/test/metabase/query_processor/test_util.clj @@ -398,7 +398,7 @@ (assoc outer-query :query {:source-query (:query outer-query)}))] (recur nested (dec n-levels))))) -(deftest nest-query-test +(deftest ^:parallel nest-query-test (testing "MBQL" (is (= {:database 1, :type :query, :query {:source-table 2}} {:database 1, :type :query, :query {:source-table 2}})) diff --git a/test/metabase/query_processor_test/breakout_test.clj b/test/metabase/query_processor_test/breakout_test.clj index a9aeb0e0a1229c010cd6459a38ac8e81dfef3c05..8950fec2118eb86d44de14c2ecdc7d205bfd20de 100644 --- a/test/metabase/query_processor_test/breakout_test.clj +++ b/test/metabase/query_processor_test/breakout_test.clj @@ -239,7 +239,7 @@ :fingerprint {:type {:type/Number {:min nil, :max nil}}}}]}) (is (=? {:status :failed :class (partial = clojure.lang.ExceptionInfo) - :error "Unable to bin Field without a min/max value"} + :error "Unable to bin Field without a min/max value (missing or incomplete fingerprint)"} (qp/process-userland-query (mt/mbql-query venues {:aggregation [[:count]] diff --git a/test/metabase/query_processor_test/drill_thru_e2e_test.clj b/test/metabase/query_processor_test/drill_thru_e2e_test.clj index bf3aace8b74c7794b6528eac23ef1b4975ce0d9e..fbfbacf426c90bbec80dfcbee0564c205931f6cf 100644 --- a/test/metabase/query_processor_test/drill_thru_e2e_test.clj +++ b/test/metabase/query_processor_test/drill_thru_e2e_test.clj @@ -3,8 +3,11 @@ [clojure.test :refer :all] [medley.core :as m] [metabase.lib.core :as lib] + [metabase.lib.field :as lib.field] [metabase.lib.metadata :as lib.metadata] + [metabase.lib.metadata.jvm :as lib.metadata.jvm] [metabase.lib.ref :as lib.ref] + [metabase.lib.test-util :as lib.tu] [metabase.query-processor :as qp] [metabase.query-processor.store :as qp.store] [metabase.test :as mt])) @@ -34,3 +37,44 @@ (mt/with-native-query-testing-context query' (is (= [["2016-05-30T00:00:00Z" 2]] (mt/rows (qp/process-query query'))))))))))) + +(deftest ^:parallel distribution-drill-on-longitude-from-sql-source-card-test + (testing "#16672" + (mt/dataset sample-dataset + (let [metadata-provider (lib.metadata.jvm/application-database-metadata-provider (mt/id)) + card-query (lib/native-query metadata-provider "SELECT * FROM PEOPLE ORDER BY ID DESC LIMIT 100;") + results (qp/process-query card-query) + results-metadata (get-in results [:data :results_metadata :columns]) + _ (is (seq results-metadata)) + metadata-provider (lib.tu/mock-metadata-provider + metadata-provider + {:cards [{:id 1 + :name "Card 1" + :database-id (mt/id) + :dataset-query card-query + :result-metadata results-metadata}]}) + query (lib/query metadata-provider (lib.metadata/card metadata-provider 1)) + longitude (lib.field/resolve-column-name-in-metadata "LATITUDE" (lib/returned-columns query)) + _ (is (=? {:name "LATITUDE" + :effective-type :type/Float + :fingerprint {:type {:type/Number {:min number?, :max number?}}}} + longitude)) + drill-context {:column longitude, :column-ref (lib.ref/ref longitude), :value nil} + distribution-drill (m/find-first + #(= (:type %) :drill-thru/distribution) + (lib/available-drill-thrus query drill-context)) + _ (is (=? {:column {:name "LATITUDE"}} + distribution-drill)) + query' (lib/drill-thru query -1 distribution-drill)] + (is (=? {:stages [{:source-card 1 + :aggregation [[:count {}]] + :breakout [[:field {:binning {:strategy :default}} "LATITUDE"]]}]} + query')) + (qp.store/with-metadata-provider metadata-provider + (is (= [[20.0 2] + [30.0 54] + [40.0 42] + [50.0 1] + [60.0 1]] + (mt/formatted-rows [double long] + (qp/process-query query'))))))))) diff --git a/test/metabase/query_processor_test/expressions_test.clj b/test/metabase/query_processor_test/expressions_test.clj index 0a48b2d2cbfad8c9a00ed0a32acff0a22fde6fbd..2027142013b0c953608a9b11bb662bb72693e097 100644 --- a/test/metabase/query_processor_test/expressions_test.clj +++ b/test/metabase/query_processor_test/expressions_test.clj @@ -37,8 +37,10 @@ (mt/run-mbql-query venues {:expressions {:my_cool_new_field [:/ $price 2]} :limit 3 - :order-by [[:asc $id]]}))))) + :order-by [[:asc $id]]}))))))) +(deftest ^:parallel floating-point-division-for-expressions-test + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) (testing "Make sure FLOATING POINT division is done when dividing by expressions/fields" (is (= [[0.6] [0.5] @@ -87,29 +89,43 @@ :limit 3 :order-by [[:asc $id]]}))))))) +(defn- dont-return-expressions-if-fields-is-explicit-query [] + ;; bigquery doesn't let you have hypthens in field, table, etc names + (let [priceplusone (if (= driver/*driver* :bigquery-cloud-sdk) "price_plus_1" "Price + 1") + oneplusone (if (= driver/*driver* :bigquery-cloud-sdk) "one_plus_one" "1 + 1") + query (mt/mbql-query venues + {:expressions {priceplusone [:+ $price 1] + oneplusone [:+ 1 1]} + :fields [$price [:expression oneplusone]] + :order-by [[:asc $id]] + :limit 3})] + {:priceplusone priceplusone + :oneplusone oneplusone + :query query})) + (deftest ^:parallel dont-return-expressions-if-fields-is-explicit-test (mt/test-drivers (mt/normal-drivers-with-feature :expressions) - ;; bigquery doesn't let you have hypthens in field, table, etc names - (let [priceplusone (if (= driver/*driver* :bigquery-cloud-sdk) "price_plus_1" "Price + 1") - oneplusone (if (= driver/*driver* :bigquery-cloud-sdk) "one_plus_one" "1 + 1") - query (mt/mbql-query venues - {:expressions {priceplusone [:+ $price 1] - oneplusone [:+ 1 1]} - :fields [$price [:expression oneplusone]] - :order-by [[:asc $id]] - :limit 3})] + (let [{:keys [query]} (dont-return-expressions-if-fields-is-explicit-query)] (testing "If an explicit `:fields` clause is present, expressions *not* in that clause should not come back" (is (= [[3 2] [2 2] [2 2]] (mt/formatted-rows [int int] - (qp/process-query query))))) + (qp/process-query query)))))))) +(deftest ^:parallel dont-return-expressions-if-fields-is-explicit-test-2 + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) + ;; bigquery doesn't let you have hypthens in field, table, etc names + (let [{:keys [query]} (dont-return-expressions-if-fields-is-explicit-query)] (testing "If `:fields` is not explicit, then return all the expressions" (is (= [[1 "Red Medicine" 4 10.0646 -165.374 3 4 2] [2 "Stout Burgers & Beers" 11 34.0996 -118.329 2 3 2] [3 "The Apple Pan" 11 34.0406 -118.428 2 3 2]] (mt/formatted-rows [int str int 4.0 4.0 int int int] - (qp/process-query (m/dissoc-in query [:query :fields])))))) + (qp/process-query (m/dissoc-in query [:query :fields]))))))))) +(deftest ^:parallel dont-return-expressions-if-fields-is-explicit-test-3 + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) + ;; bigquery doesn't let you have hypthens in field, table, etc names + (let [{:keys [priceplusone oneplusone]} (dont-return-expressions-if-fields-is-explicit-query)] (testing "When aggregating, expressions that aren't used shouldn't come back" (is (= [[2 22] [3 59] [4 13]] (mt/formatted-rows [int int] @@ -131,7 +147,10 @@ (mt/run-mbql-query venues {:expressions {:x [:+ $price $id]} :limit 3 - :order-by [[:desc [:expression :x]]]}))))) + :order-by [[:desc [:expression :x]]]}))))))) + +(deftest ^:parallel expressions-in-order-by-test-2 + (mt/test-drivers (mt/normal-drivers-with-feature :expressions) (testing "Can we refer to expressions inside an ORDER BY clause with a secondary order by?" (is (= [[81 "Tanoshi Sushi & Sake Bar" 40 40.7677 -73.9533 4 85.0] [79 "Sushi Yasuda" 40 40.7514 -73.9736 4 83.0] @@ -170,16 +189,14 @@ (is (isa? (name->base-type (mt/format-name "category_id")) :type/Number))))))))) +(defn- calculate-bird-scarcity* + "\"bird scarcity\" is a \"scientific metric\" based on the number of birds seen in a given day (at least for the + purposes of the tests below). -;;; +----------------------------------------------------------------------------------------------------------------+ -;;; | HANDLING NULLS AND ZEROES | -;;; +----------------------------------------------------------------------------------------------------------------+ + e.g. -;; "bird scarcity" is a scientific metric based on the number of birds seen in a given day -;; (at least for the purposes of the tests below) -;; -;; e.g. scarcity = 100.0 / num-birds -(defn- calculate-bird-scarcity* [formula filter-clause] + scarcity = 100.0 / num-birds" + [formula filter-clause] (mt/formatted-rows [2.0] (mt/dataset daily-bird-counts (mt/run-mbql-query bird-count @@ -192,62 +209,86 @@ (defmacro ^:private calculate-bird-scarcity [formula & [filter-clause]] `(mt/dataset ~'daily-bird-counts (mt/$ids ~'bird-count - (calculate-bird-scarcity* ~formula ~filter-clause)))) + (calculate-bird-scarcity* ~formula ~filter-clause)))) + +(defn- nulls-and-zeroes-test-drivers [] + (disj (mt/normal-drivers-with-feature :expressions) + ;; bigquery doesn't let you have hypthens in field, table, etc names + ;; therefore a different macro is tested in bigquery driver tests + :bigquery-cloud-sdk)) -(deftest ^:parallel nulls-and-zeroes-test - (mt/test-drivers (disj (mt/normal-drivers-with-feature :expressions) - ;; bigquery doesn't let you have hypthens in field, table, etc names - ;; therefore a different macro is tested in bigquery driver tests - :bigquery-cloud-sdk) +(deftest ^:parallel nulls-and-zeroes-test-1 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing (str "hey... expressions should work if they are just a Field! (Also, this lets us take a peek at the " "raw values being used to calculate the formulas below, so we can tell at a glance if they're right " "without referring to the EDN def)") (is (= [[nil] [0.0] [0.0] [10.0] [8.0] [5.0] [5.0] [nil] [0.0] [0.0]] - (calculate-bird-scarcity $count)))) + (calculate-bird-scarcity $count)))))) +(deftest ^:parallel nulls-and-zeroes-test-2 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing (str "do expressions automatically handle division by zero? Should return `nil` " - "in the results for places where that was attempted") - (is (= [[nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [9.09] [7.14]] - (calculate-bird-scarcity [:/ 100.0 $count] - [:!= $count nil])))) - - (testing (str "do expressions handle division by `nil`? Should return `nil` in the results for places where that " - "was attempted") + "in the results for places where that was attempted") + (is (= [[nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [9.09] [7.14]] + (calculate-bird-scarcity [:/ 100.0 $count] [:!= $count nil])))))) + +(deftest ^:parallel nulls-and-zeroes-test-3 + (mt/test-drivers (nulls-and-zeroes-test-drivers) + (testing (str + "do expressions handle division by `nil`? Should return `nil` in the results for places where that " + "was attempted") (is (= [[nil] [10.0] [12.5] [20.0] [20.0] [nil] [9.09] [7.14] [12.5] [7.14]] - (calculate-bird-scarcity [:/ 100.0 $count] - [:or - [:= $count nil] - [:!= $count 0]])))) - - (testing "can we handle BOTH NULLS AND ZEROES AT THE SAME TIME????" - (is (= [[nil] [nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [nil]] - (calculate-bird-scarcity [:/ 100.0 $count])))) - + (calculate-bird-scarcity [:/ 100.0 $count] [:or [:= $count nil] [:!= $count 0]])))))) + +(deftest ^:parallel nulls-and-zeroes-test-4 + (mt/test-drivers + (nulls-and-zeroes-test-drivers) + (testing + "can we handle BOTH NULLS AND ZEROES AT THE SAME TIME????" + (is + (= + [[nil] [nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [nil]] + (calculate-bird-scarcity [:/ 100.0 $count])))))) +(deftest ^:parallel nulls-and-zeroes-test-5 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing "can we handle dividing by literal 0?" - (is (= [[nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil]] - (calculate-bird-scarcity [:/ $count 0])))) + (is (= [[nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil]] + (calculate-bird-scarcity [:/ $count 0])))))) +(deftest ^:parallel nulls-and-zeroes-test-6 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing "ok, what if we use multiple args to divide, and more than one is zero?" - (is (= [[nil] [nil] [nil] [1.0] [1.56] [4.0] [4.0] [nil] [nil] [nil]] - (calculate-bird-scarcity [:/ 100.0 $count $count])))) + (is (= [[nil] [nil] [nil] [1.0] [1.56] [4.0] [4.0] [nil] [nil] [nil]] + (calculate-bird-scarcity [:/ 100.0 $count $count])))))) +(deftest ^:parallel nulls-and-zeroes-test-7 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing "are nulls/zeroes still handled appropriately when nested inside other expressions?" - (is (= [[nil] [nil] [nil] [20.0] [25.0] [40.0] [40.0] [nil] [nil] [nil]] - (calculate-bird-scarcity [:* [:/ 100.0 $count] 2])))) - - (testing (str "if a zero is present in the NUMERATOR we should return ZERO and not NULL " - "(`0 / 10 = 0`; `10 / 0 = NULL`, at least as far as MBQL is concerned)") + (is (= [[nil] [nil] [nil] [20.0] [25.0] [40.0] [40.0] [nil] [nil] [nil]] + (calculate-bird-scarcity [:* [:/ 100.0 $count] 2])))))) + +(deftest ^:parallel nulls-and-zeroes-test-8 + (mt/test-drivers (nulls-and-zeroes-test-drivers) + (testing (str + "if a zero is present in the NUMERATOR we should return ZERO and not NULL " + "(`0 / 10 = 0`; `10 / 0 = NULL`, at least as far as MBQL is concerned)") (is (= [[nil] [0.0] [0.0] [1.0] [0.8] [0.5] [0.5] [nil] [0.0] [0.0]] - (calculate-bird-scarcity [:/ $count 10])))) + (calculate-bird-scarcity [:/ $count 10])))))) +(deftest ^:parallel nulls-and-zeroes-test-9 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing "can addition handle nulls & zeroes?" (is (= [[nil] [10.0] [10.0] [20.0] [18.0] [15.0] [15.0] [nil] [10.0] [10.0]] - (calculate-bird-scarcity [:+ $count 10])))) + (calculate-bird-scarcity [:+ $count 10])))))) +(deftest ^:parallel nulls-and-zeroes-test-10 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing "can subtraction handle nulls & zeroes?" (is (= [[nil] [10.0] [10.0] [0.0] [2.0] [5.0] [5.0] [nil] [10.0] [10.0]] - (calculate-bird-scarcity [:- 10 $count])))) + (calculate-bird-scarcity [:- 10 $count])))))) +(deftest ^:parallel nulls-and-zeroes-test-11 + (mt/test-drivers (nulls-and-zeroes-test-drivers) (testing "can multiplications handle nulls & zeros?" (is (= [[nil] [0.0] [0.0] [10.0] [8.0] [5.0] [5.0] [nil] [0.0] [0.0]] (calculate-bird-scarcity [:* 1 $count])))))) @@ -281,7 +322,12 @@ :fields [[:expression :prev_month]] :limit 3 :order-by [[:asc $name]]}) - mt/rows))))) + mt/rows)))))))) + +(deftest temporal-arithmetic-test-2 + (mt/test-drivers (mt/normal-drivers-with-feature :expressions :date-arithmetics) + (doseq [[op interval] [[:+ [:interval -31 :day]] + [:- [:interval 31 :day]]]] (testing (str "Test interaction of datetime arithmetics with truncation using " op " operator") (is (= (robust-dates ["2014-09-02T00:00:00" diff --git a/test/metabase/query_processor_test/native_test.clj b/test/metabase/query_processor_test/native_test.clj index 7ef4a07e8c91831b4a80b19a6768555b2d802161..a0ecf633898be5d5e1449a1330797c22559b18aa 100644 --- a/test/metabase/query_processor_test/native_test.clj +++ b/test/metabase/query_processor_test/native_test.clj @@ -37,7 +37,7 @@ (mt/native-query {:query "select name from users;"})))))) -(deftest native-referring-question-referring-question-test +(deftest ^:parallel native-referring-question-referring-question-test (testing "Should be able to run native query referring a question referring a question (#25988)" (mt/with-driver :h2 (mt/dataset sample-dataset