From 99cf8cdf46e99111bb1fab67aa35e7be10db450a Mon Sep 17 00:00:00 2001 From: Felix Mueller <5407006+enqueue@users.noreply.github.com> Date: Sun, 23 Jun 2019 13:57:26 +0200 Subject: [PATCH] Test for (non-) empty String (#18) Fix for #4 --- .circleci/config.yml | 6 +-- project.clj | 1 + src/metabase/driver/clickhouse.clj | 23 ++++++++++ test/metabase/driver/clickhouse_test.clj | 58 +++++++++++++++--------- test/metabase/test/data/clickhouse.clj | 3 +- 5 files changed, 65 insertions(+), 26 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4ffcb92..e54bbbe 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,6 +1,6 @@ defaults: &defaults docker: - - image: circleci/clojure:lein-2.8.1-node-browsers + - image: circleci/clojure:lein-2.9.1-node-browsers version: 2.1 @@ -8,7 +8,7 @@ jobs: checkout: docker: - - image: circleci/clojure:lein-2.8.1-node-browsers + - image: circleci/clojure:lein-2.9.1-node-browsers steps: - checkout: path: metabase-clickhouse-driver @@ -90,7 +90,7 @@ jobs: tests-clickhouse: docker: - - image: circleci/clojure:lein-2.8.1-node-browsers + - image: circleci/clojure:lein-2.9.1-node-browsers - image: yandex/clickhouse-server:latest environment: TZ: "/usr/share/zoneinfo/UTC" diff --git a/project.clj b/project.clj index 50aec95..42710dc 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,5 @@ (defproject metabase/clickhouse-driver "1.0.0-SNAPSHOT-0.1.54" + :description "Metabase ClickHouse Driver" :min-lein-version "2.5.0" :aliases diff --git a/src/metabase/driver/clickhouse.clj b/src/metabase/driver/clickhouse.clj index 5d5f085..fcd961d 100644 --- a/src/metabase/driver/clickhouse.clj +++ b/src/metabase/driver/clickhouse.clj @@ -188,6 +188,29 @@ (hsql/call :toFloat64 (sql.qp/->honeysql driver arg)))] ((get-method sql.qp/->honeysql [:sql :/]) driver args))) +;; the filter criterion reads "is empty" +;; also see desugar.clj +(defmethod sql.qp/->honeysql [:clickhouse :=] [driver [_ field value]] + (let [base-type (:base_type (last value)) value-value (:value value)] + (if (and (isa? base-type :type/Text) + (nil? value-value)) + [:or + [:= (sql.qp/->honeysql driver field) (sql.qp/->honeysql driver value)] + [:= (hsql/call :empty (sql.qp/->honeysql driver field)) 1]] + ((get-method sql.qp/->honeysql [:sql :=]) driver [_ field value])))) + +;; the filter criterion reads "not empty" +;; also see desugar.clj +(defmethod sql.qp/->honeysql [:clickhouse :!=] [driver [_ field value]] + (let [base-type (:base_type (last value)) value-value (:value value)] + (if (and (isa? base-type :type/Text) + (nil? value-value)) + [:and + [:!= (sql.qp/->honeysql driver field) (sql.qp/->honeysql driver value)] + [:= (hsql/call :notEmpty (sql.qp/->honeysql driver field)) 1]] + ((get-method sql.qp/->honeysql [:sql :!=]) driver [_ field value])))) + + ;; I do not know why the tests expect nil counts for empty results ;; but that's how it is :-) ;; metabase.query-processor-test.count-where-test diff --git a/test/metabase/driver/clickhouse_test.clj b/test/metabase/driver/clickhouse_test.clj index f4ec141..5c17d9a 100644 --- a/test/metabase/driver/clickhouse_test.clj +++ b/test/metabase/driver/clickhouse_test.clj @@ -16,13 +16,11 @@ (datasets/expect-with-driver :clickhouse 21.0 - (-> (data/with-db-for-dataset - [_ - (tx/dataset-definition "ClickHouse with Decimal Field" - ["test-data" - [{:field-name "my_money", :base-type {:native "Decimal(12,3)"}}] - [[1.0] [23.0] [42.0] [42.0]]])] - (data/run-mbql-query test-data + (-> (data/dataset (tx/dataset-definition "metabase_tests_decimal" + ["test-data-decimal" + [{:field-name "my_money", :base-type {:native "Decimal(12,3)"}}] + [[1.0] [23.0] [42.0] [42.0]]]) + (data/run-mbql-query test-data-decimal {:expressions {:divided [:/ $my_money 2]} :filter [:> [:expression :divided] 1.0] :breakout [[:expression :divided]] @@ -32,24 +30,42 @@ (datasets/expect-with-driver :clickhouse "['foo','bar']" - (-> (data/with-db-for-dataset - [_ - (tx/dataset-definition "ClickHouse with String Array" - ["test-data-array" - [{:field-name "my_array", :base-type {:native "Array(String)"}}] - [[(into-array (list "foo" "bar"))]]])] - (data/run-mbql-query test-data-array {:limit 1})) + (-> (data/dataset (tx/dataset-definition "metabase_tests_array_string" + ["test-data-array-string" + [{:field-name "my_array", :base-type {:native "Array(String)"}}] + [[(into-array (list "foo" "bar"))]]]) + (data/run-mbql-query test-data-array-string {:limit 1})) first-row last)) (datasets/expect-with-driver :clickhouse "[23,42]" - (-> (data/with-db-for-dataset - [_ - (tx/dataset-definition "ClickHouse with UInt64 Array" - ["test-data-array" - [{:field-name "my_array", :base-type {:native "Array(UInt64)"}}] - [[(into-array (list 23 42))]]])] - (data/run-mbql-query test-data-array {:limit 1})) + (-> (data/dataset (tx/dataset-definition "metabase_tests_array_uint64" + ["test-data-array-uint64" + [{:field-name "my_array", :base-type {:native "Array(UInt64)"}}] + [[(into-array (list 23 42))]]]) + (data/run-mbql-query test-data-array-uint64 {:limit 1})) + first-row last)) + +(datasets/expect-with-driver :clickhouse + 2 + (-> (data/dataset (tx/dataset-definition "metabase_tests_nullable_strings" + ["test-data-nullable-strings" + [{:field-name "mystring", :base-type :type/Text}] + [["foo"], ["bar"], [" "], [""], [nil]]]) + (data/run-mbql-query test-data-nullable-strings + {:filter [:is-null $mystring] + :aggregation [:count]})) + first-row last)) + +(datasets/expect-with-driver :clickhouse + 3 + (-> (data/dataset (tx/dataset-definition "metabase_tests_nullable_strings" + ["test-data-nullable-strings" + [{:field-name "mystring", :base-type :type/Text}] + [["foo"], ["bar"], [" "], [""], [nil]]]) + (data/run-mbql-query test-data-nullable-strings + {:filter [:not-null $mystring] + :aggregation [:count]})) first-row last)) (expect diff --git a/test/metabase/test/data/clickhouse.clj b/test/metabase/test/data/clickhouse.clj index d1c1f3f..87c5fe1 100644 --- a/test/metabase/test/data/clickhouse.clj +++ b/test/metabase/test/data/clickhouse.clj @@ -19,9 +19,8 @@ (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Date] [_ _] "DateTime") (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/DateTime] [_ _] "DateTime") (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Float] [_ _] "Float64") -;; Nullable is kind of experimental, only required for one test (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Integer] [_ _] "Nullable(Int32)") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Text] [_ _] "String") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Text] [_ _] "Nullable(String)") (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/UUID] [_ _] "UUID") (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Time] [_ _] "DateTime")