From 2728ceaafdeb09b0bff5220e800117fe31b0e443 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cam=20Sa=C3=BCl?= <cammsaul@gmail.com>
Date: Fri, 5 Feb 2016 16:07:48 -0800
Subject: [PATCH] Unit tests for filtering datetime column against `nil`
:scream_cat:
---
project.clj | 6 +-----
resources/log4j.properties | 6 +-----
.../driver/generic_sql/query_processor.clj | 11 ++++++-----
src/metabase/driver/mongo/query_processor.clj | 2 ++
test/metabase/driver/query_processor_test.clj | 17 +++++++++++++++++
test/metabase/test/data/datasets.clj | 6 ++++--
6 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/project.clj b/project.clj
index f890b6cdc86..da653beba36 100644
--- a/project.clj
+++ b/project.clj
@@ -79,15 +79,13 @@
[lein-expectations "0.0.8"] ; run unit tests with 'lein expectations'
[lein-instant-cheatsheet "2.1.4"] ; use awesome instant cheatsheet created by yours truly w/ 'lein instant-cheatsheet'
[michaelblume/lein-marginalia "0.9.0"]] ; generate documentation with 'lein marg'
- :global-vars {*warn-on-reflection* true} ; Emit warnings on all reflection calls
:env {:mb-run-mode "dev"}
:jvm-opts ["-Dlogfile.path=target/log"
"-Xms1024m" ; give JVM a decent heap size to start with
"-Xmx2048m" ; hard limit of 2GB so we stop hitting the 4GB container limit on CircleCI
"-XX:+CMSClassUnloadingEnabled" ; let Clojure's dynamically generated temporary classes be GC'ed from PermGen
"-XX:+UseConcMarkSweepGC"]} ; Concurrent Mark Sweep GC needs to be used for Class Unloading (above)
- :expectations {:global-vars {*warn-on-reflection* false}
- :injections [(require 'metabase.test-setup)]
+ :expectations {:injections [(require 'metabase.test-setup)]
:resource-paths ["test_resources"]
:env {:mb-test-setting-1 "ABCDEFG"
:mb-run-mode "test"}
@@ -100,7 +98,6 @@
:generate-sample-dataset {:dependencies [[faker "0.2.2"] ; Fake data generator -- port of Perl/Ruby
[incanter/incanter-core "1.9.0"]] ; Satistical functions like normal distibutions}})
:source-paths ["sample_dataset"]
- :global-vars {*warn-on-reflection* false}
:main ^:skip-aot metabase.sample-dataset.generate}
;; Run reset password from source: MB_DB_PATH=/path/to/metabase.db lein with-profile reset-password run email@address.com
;; Create the reset password JAR: lein with-profile reset-password jar
@@ -108,7 +105,6 @@
;; Run the reset password JAR: MB_DB_PATH=/path/to/metabase.db java -classpath /path/to/metabase-uberjar.jar:/path/to/reset-password.jar \
;; metabase.reset_password.core email@address.com
:reset-password {:source-paths ["reset_password"]
- :global-vars {*warn-on-reflection* false}
:main metabase.reset-password.core
:jar-name "reset-password.jar"
;; Exclude everything except for reset-password specific code in the created jar
diff --git a/resources/log4j.properties b/resources/log4j.properties
index 232cb5c959c..3939e8140e1 100644
--- a/resources/log4j.properties
+++ b/resources/log4j.properties
@@ -1,4 +1,4 @@
-log4j.rootLogger=INFO, console
+log4j.rootLogger=WARN, console
# log to the console
log4j.appender.console=org.apache.log4j.ConsoleAppender
@@ -15,8 +15,4 @@ log4j.appender.file.layout=org.apache.log4j.PatternLayout
log4j.appender.file.layout.ConversionPattern=%d [%t] %-5p%c - %m%n
# customizations to logging by package
-log4j.logger.com.mchange=WARN
-log4j.logger.cz.vutbr.web=WARN
-log4j.logger.liquibase=WARN
log4j.logger.metabase=DEBUG
-log4j.logger.org.mongodb=WARN
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index a60f8410b7f..e3d87470c92 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -30,19 +30,20 @@
(def ^:private ^:dynamic *query* nil)
+(defn- driver [] (:driver *query*))
-;;; ## Formatting
-
-(defprotocol ^:private IGenericSQLFormattable
- (^:private formatted [this]))
-(defn- driver [] (:driver *query*))
+;;; ## Formatting
(defn- as
"Generate a FORM `AS` FIELD alias using the name information of FIELD."
[form field]
[form (name field)])
+
+(defprotocol ^:private IGenericSQLFormattable
+ (^:private formatted [this]))
+
(extend-protocol IGenericSQLFormattable
nil (formatted [_] nil)
diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj
index f72e6f27145..b16b3bf1923 100644
--- a/src/metabase/driver/mongo/query_processor.clj
+++ b/src/metabase/driver/mongo/query_processor.clj
@@ -173,6 +173,8 @@
:year {$year field})))))
(extend-protocol IRValue
+ nil (->rvalue [_] nil)
+
Field
(->rvalue [this]
(str \$ (->lvalue this)))
diff --git a/test/metabase/driver/query_processor_test.clj b/test/metabase/driver/query_processor_test.clj
index cf8d8995187..6e0f17d4507 100644
--- a/test/metabase/driver/query_processor_test.clj
+++ b/test/metabase/driver/query_processor_test.clj
@@ -510,6 +510,23 @@
(ql/filter (ql/inside $latitude $longitude 10.0649 -165.379 10.0641 -165.371)))
rows formatted-venues-rows))
+;;; ## FILTER - `is-null` & `not-null` on datetime columns
+(expect-with-non-timeseries-dbs
+ [1000]
+ (first-row (run-query checkins
+ (ql/aggregation (ql/count))
+ (ql/filter (ql/not-null $date)))))
+
+(expect-with-non-timeseries-dbs
+ ;; Some DBs like Mongo don't return any results at all in this case, and there's no easy workaround
+ true
+ (let [result (first-row (run-query checkins
+ (ql/aggregation (ql/count))
+ (ql/filter (ql/is-null $date))))]
+ (or (= result [0])
+ (nil? result))))
+
+
;; ## "FIELDS" CLAUSE
;; Test that we can restrict the Fields that get returned to the ones specified, and that results come back in the order of the IDs in the `fields` clause
diff --git a/test/metabase/test/data/datasets.clj b/test/metabase/test/data/datasets.clj
index adae579926c..4bd41ae0ddb 100644
--- a/test/metabase/test/data/datasets.clj
+++ b/test/metabase/test/data/datasets.clj
@@ -5,7 +5,8 @@
[colorize.core :as color]
[environ.core :refer [env]]
[expectations :refer [expect]]
- [metabase.driver :as driver]))
+ (metabase [config :as config]
+ [driver :as driver])))
(driver/find-and-load-drivers!)
(def ^:const all-valid-engines (set (keys (driver/available-drivers))))
@@ -40,7 +41,8 @@
By default, this only contains `:h2` but can be overriden by setting env var `ENGINES`."
(let [engines (or (get-engines-from-env)
#{:h2})]
- (log/info (color/green "Running QP tests against these engines: " engines))
+ (when config/is-test?
+ (log/info (color/cyan "Running QP tests against these engines: " engines)))
engines))
--
GitLab