Test for (non-) empty String (ClickHouse#18)

Fix for ClickHouse#4
deathalt · Jun 23, 2019 · 99cf8cd · 99cf8cd
1 parent 457573b
commit 99cf8cd
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 26 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -1,14 +1,14 @@
 defaults: &defaults
   docker:
-    - image: circleci/clojure:lein-2.8.1-node-browsers
+    - image: circleci/clojure:lein-2.9.1-node-browsers
 
 version: 2.1
 
 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
@@ -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
@@ -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
@@ -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
@@ -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")