Skip to content

Commit

Permalink
fix is-empty comparison (ClickHouse#32)
Browse files Browse the repository at this point in the history
  • Loading branch information
enqueue authored Sep 6, 2019
1 parent a650e6f commit 1bf0b42
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 9 deletions.
19 changes: 10 additions & 9 deletions src/metabase/driver/clickhouse.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions test/metabase/driver/clickhouse_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down

0 comments on commit 1bf0b42

Please sign in to comment.