From 1bf0b42abd74a723328ddaced8b4cec4f15a8862 Mon Sep 17 00:00:00 2001 From: Felix Mueller <5407006+enqueue@users.noreply.github.com> Date: Fri, 6 Sep 2019 13:26:36 +0200 Subject: [PATCH] fix is-empty comparison (#32) --- src/metabase/driver/clickhouse.clj | 19 ++++++++++--------- test/metabase/driver/clickhouse_test.clj | 13 +++++++++++++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/metabase/driver/clickhouse.clj b/src/metabase/driver/clickhouse.clj index a826e35..4379de3 100644 --- a/src/metabase/driver/clickhouse.clj +++ b/src/metabase/driver/clickhouse.clj @@ -189,9 +189,11 @@ ;; 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)) + (let [[qual valuevalue fieldinfo] value] + (if (and + (isa? qual :value) + (isa? (:base_type fieldinfo) :type/Text) + (nil? valuevalue)) [:or [:= (sql.qp/->honeysql driver field) (sql.qp/->honeysql driver value)] [:= (hsql/call :empty (sql.qp/->honeysql driver field)) 1]] @@ -200,15 +202,16 @@ ;; 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)) + (let [[qual valuevalue fieldinfo] value] + (if (and + (isa? qual :value) + (isa? (:base_type fieldinfo) :type/Text) + (nil? valuevalue)) [: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 @@ -317,8 +320,6 @@ (defmethod driver/current-db-time :clickhouse [& args] (apply driver.common/current-db-time args)) - - (defmethod driver/display-name :clickhouse [_] "ClickHouse") (defmethod driver/supports? [:clickhouse :foreign-keys] [_ _] false) diff --git a/test/metabase/driver/clickhouse_test.clj b/test/metabase/driver/clickhouse_test.clj index 9e92618..0a127c2 100644 --- a/test/metabase/driver/clickhouse_test.clj +++ b/test/metabase/driver/clickhouse_test.clj @@ -62,12 +62,25 @@ (-> (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]})) qp.test/first-row last)) +(datasets/expect-with-driver :clickhouse + 1 + (-> (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 [:= $mystring "foo"] + :aggregation [:count]})) + qp.test/first-row last)) + (datasets/expect-with-driver :clickhouse [[1 "Я_1"] [3 "Я_2"]