Exasol data types (#19)

* Allow skipping build when running integration tests

* Add known issue for scanning GEOMETRY columns

* Change special types to :type/*

* Speedup setup of Exasol db

* Deactivate flaky test

#21

* Review finding: add link to user guide

Co-authored-by: jakobbraun <jakob.braun@posteo.de>

.clj-kondo/config.edn

@@ -1,5 +1,11 @@
-{:linters {:unresolved-var {
-                            ; False positive
+{:linters {:unresolved-var {; False positive
                             :exclude [honeysql.core/call
                                       honeysql.core/raw
-                                      metabase.util.honeysql-extensions/with-type-info]}}}
+                                      metabase.util.honeysql-extensions/with-type-info]}
+           :unresolved-symbol {
+                               :exclude [
+                                         ; The defdataset macro expects a symbol as dataset name
+                                         (metabase.test.data.interface/defdataset)
+                                         ; The run-mbql-query macro allows using $field shortcut syntax
+                                         (metabase.test/run-mbql-query)
+                                         ]}}}

.github/workflows/integration-tests.yml

@@ -89,7 +89,7 @@ jobs:
 
       - name: Spawn Exasol environment
         run: |
-          ./start-test-env spawn-test-environment --environment-name test --database-port-forward 8563
+          ./start-test-env spawn-test-environment --environment-name test --database-port-forward 8563 --deactivate-database-setup
         working-directory: integration-test-docker-environment
 
       - name: Run integration tests

doc/changes/changelog.md

@@ -1,4 +1,4 @@
 # Changes
 
-* [0.1.1](changes_0.1.1.md)
+* [0.1.1](changes_0.2.0.md)
 * [0.1.0](changes_0.1.0.md)

doc/changes/changes_0.2.0.md

@@ -0,0 +1,21 @@
+# metabase-driver 0.2.0, released 2022-01-17
+
+Code name: Exasol specific data types and fix for `TIMESTAMP` columns
+
+## Summary
+
+This release fixes two issues when reading `TIMESTAMP` columns:
+
+* Reading null values caused a `NullPointerException` in the driver.
+* Timestamp values are now read in UTC timezone instead of the local time zone.
+
+The release also adds support for Exasol specific data types `INTERVAL`, `GEOMETRY` and `HASHTYPE`. As Metabase does not support these types, they are returned as strings of Metabase type `:type/*`. There is an issue with scanning values of `GEOMETRY` columns, see [issue #20](https://github.com/exasol/metabase-driver/issues/20) and [user guide](../user_guide/user_guide.md#scanning-field-values-logs-an-exception-for-geometry-columns) for details.
+
+## Bugfixes
+
+* #17: Fixed reading `TIMESTAMP` columns
+* #6: Added support for Exasol specific data types `INTERVAL`, `GEOMETRY` and `HASHTYPE`
+
+## Dependency Updates
+
+(none)

doc/user_guide/user_guide.md

@@ -46,6 +46,22 @@ This is a known issue in Metabase. See [this ticket](https://github.com/exasol/m
 
 Queries involving `TIMESTAMP WITH TIMEZONE` columns my return wrong results depending on the timezone set for the session. See [this ticket](https://github.com/exasol/metabase-driver/issues/9) for details.
 
+### Scanning field values logs an exception for `GEOMETRY` columns
+
+When Metabase scans field values of a table with a GEOMETRY column (e.g. when you click the "Re-scan field values now" button on the Database page) it logs the following exception:
+
+```
+2022-01-17 09:01:18,009 ERROR models.field-values :: Error fetching field values
+clojure.lang.ExceptionInfo: Error executing query {:sql "-- Metabase\nSELECT \"META\".\"DATA_TYPES\".\"GEO\" AS \"GEO\" FROM \"META\".\"DATA_TYPES\" GROUP BY \"META\".\"DATA_TYPES\".\"GEO\" ORDER BY \"META\".\"DATA_TYPES\".\"GEO\" ASC LIMIT 5000", :params nil, :type :invalid-query}
+...
+Caused by: java.sql.SQLException: Feature not supported: GEOMETRY type in GROUP BY (Session: 1722185677957169152)
+...
+```
+
+There seem to be no consequences of this error, everything seems to work fine.
+
+See issue [#20](https://github.com/exasol/metabase-driver/issues/20) for details and a workaround for avoiding this error message.
+
 ## Troubleshooting
 
 ### Exasol Driver Not Available

scripts/exclude_tests.diff

@@ -1,3 +1,39 @@
+diff --git a/test/metabase/api/session_test.clj b/test/metabase/api/session_test.clj
+index b1042e9723..c92e26206b 100644
+--- a/test/metabase/api/session_test.clj
++++ b/test/metabase/api/session_test.clj
+@@ -160,29 +160,8 @@
+         (is (re= #"^Too many attempts! You must wait \d+ seconds before trying again\.$"
+                  (error)))))))
+
+-(deftest failure-threshold-per-request-source
+-  (testing "The same as above, but ensure that throttling is done on a per request source basis."
+-    (with-redefs [session-api/login-throttlers          (cleaned-throttlers #'session-api/login-throttlers
+-                                                                            [:username :ip-address])
+-                  public-settings/source-address-header (constantly "x-forwarded-for")]
+-      (dotimes [n 50]
+-        (let [response    (send-login-request (format "user-%d" n)
+-                                              {"x-forwarded-for" "10.1.2.3"})
+-              status-code (:status response)]
+-          (assert (= status-code 401) (str "Unexpected response status code:" status-code))))
+-      (dotimes [n 50]
+-        (let [response    (send-login-request (format "round2-user-%d" n)) ; no x-forwarded-for
+-              status-code (:status response)]
+-          (assert (= status-code 401) (str "Unexpected response status code:" status-code))))
+-      (let [error (fn []
+-                    (-> (send-login-request "last-user" {"x-forwarded-for" "10.1.2.3"})
+-                        :body
+-                        json/parse-string
+-                        (get-in ["errors" "username"])))]
+-        (is (re= #"^Too many attempts! You must wait 1\d seconds before trying again\.$"
+-                 (error)))
+-        (is (re= #"^Too many attempts! You must wait 4\d seconds before trying again\.$"
+-                 (error)))))))
++; Deactivated flaky test
++; https://github.com/exasol/metabase-driver/issues/21
+
+ (deftest logout-test
+   (testing "DELETE /api/session"
 diff --git a/test/metabase/api/table_test.clj b/test/metabase/api/table_test.clj
 index a348540ca2..c81ac9b109 100644
 --- a/test/metabase/api/table_test.clj

scripts/run-integration-tests.sh

@@ -8,6 +8,7 @@ jdbc_driver_version=7.1.3
 exasol_driver_dir="$( cd "$(dirname "$0")/.." >/dev/null 2>&1 ; pwd -P )"
 metabase_dir=$(cd "$exasol_driver_dir/../metabase"; pwd)
 metabase_plugin_dir="$metabase_dir/plugins/"
+skip_build=${skip_build:-false}
 
 log_color() {
     local color="$1"
@@ -44,7 +45,7 @@ check_preconditions() {
     fi
 }
 
-symlink_driver() {
+symlink_driver_sources() {
     local symlink_target="$metabase_dir/modules/drivers/exasol"
     if [[ -L "$symlink_target" && -d "$symlink_target" ]]; then
         log_trace "Symlink already exists at $symlink_target"
@@ -136,20 +137,30 @@ get_exasol_certificate_fingerprint() {
                 | openssl x509 -fingerprint -sha256 -noout -in /dev/stdin \
                 | sed 's/SHA256 Fingerprint=//' \
                 | sed 's/://g')
+
+    if [ -z "${fingerprint}" ]; then
+        >&2 log_error "Error getting certificate from $EXASOL_HOST:$EXASOL_PORT"
+        exit 1
+    fi
     echo "$fingerprint"
 }
 
 ###
 
 check_preconditions
-symlink_driver
+symlink_driver_sources
 patch_metabase_deps
 patch_excluded_tests
 install_jdbc_driver
 install_metabase_jar
 log_info "Getting certificate fingerprint from $EXASOL_HOST:$EXASOL_PORT..."
 fingerprint=$(get_exasol_certificate_fingerprint)
-build_and_install_driver
+
+if [ "$skip_build" == "true" ]; then
+    log_error "Skipping driver build"
+else
+    build_and_install_driver
+fi
 
 log_info "Using Exasol database $EXASOL_HOST:$EXASOL_PORT with certificate fingerprint '$fingerprint'"
 log_info "Starting integration tests in $metabase_dir..."

src/metabase/driver/exasol.clj

@@ -51,17 +51,17 @@
     [#"^BOOLEAN$"                         :type/Boolean]
     [#"^CHAR$"                            :type/Text]
     [#"^VARCHAR$"                         :type/Text]
-    [#"^HASHTYPE$"                        :type/Text]
     [#"^BIGINT$"                          :type/Decimal]
     [#"^DECIMAL$"                         :type/Decimal]
     [#"^DOUBLE PRECISION$"                :type/Float]
     [#"^DOUBLE$"                          :type/Float]
     [#"^DATE$"                            :type/Date]
     [#"^TIMESTAMP$"                       :type/DateTime]
     [#"^TIMESTAMP WITH LOCAL TIME ZONE$"  :type/DateTime]
-    [#"^INTERVAL DAY TO SECOND$"          :type/Text]
-    [#"^INTERVAL YEAR TO MONTH$"          :type/Text]
-    [#"^GEOMETRY$"                        :type/Text]]))
+    [#"^HASHTYPE$"                        :type/*]
+    [#"^INTERVAL DAY TO SECOND$"          :type/*]
+    [#"^INTERVAL YEAR TO MONTH$"          :type/*]
+    [#"^GEOMETRY$"                        :type/*]]))
 
 (defmethod sql-jdbc.sync/database-type->base-type :exasol
   [_ column-type]

test/metabase/driver/exasol_test.clj

@@ -0,0 +1,29 @@
+(ns metabase.driver.exasol-test
+  (:require [clojure.test :refer [deftest testing is]]
+            [metabase.test :as mt]
+            [metabase.test.util :as tu]
+            [metabase.test.data :as td]
+            [metabase.test.data.dataset-definitions :as dataset]
+            [metabase.test.data.exasol-dataset-definitions :as exasol-dataset]))
+
+(deftest timezone-id-test
+  (mt/test-driver :exasol
+                  (is (= "UTC"
+                         (tu/db-timezone-id)))))
+
+(deftest text-equals-empty-string-test
+  (mt/test-driver :exasol
+                  (testing ":= with empty string should work correctly"
+                    (mt/dataset dataset/airports
+                                (is (= [1]
+                                       (mt/first-row
+                                        (mt/run-mbql-query "airport" {:aggregation [:count], :filter [:= $code ""]}))))))))
+
+(deftest exasol-data-types
+  (mt/test-driver :exasol
+                  (td/dataset exasol-dataset/exasol-data-types
+                              (is (= [["null-values"      nil                                nil      nil                nil]
+                                      ["non-null-values" "550e8400e29b11d4a716446655440000" "+05-03" "+02 12:50:10.123" "POINT (2 5)"]]
+                                     (mt/rows (mt/run-mbql-query "data_types" {:fields [$name $hash $interval_ytm $interval_dts $geo]
+                                                                               :order-by [[:asc $row_order]]})))))))
+

test/metabase/test/data/exasol_dataset_definitions.clj

@@ -0,0 +1,34 @@
+(ns metabase.test.data.exasol-dataset-definitions
+  (:require
+   [metabase.test.data.interface :as tx]))
+
+(tx/defdataset exasol-data-types
+  "Test data with all Exasol specific data types"
+  [["data_types"
+    ; Columns
+    [{:field-name "row_order",     :base-type :type/Integer}
+     {:field-name "name",          :base-type :type/Text}
+     {:field-name "hash",          :base-type {:native "HASHTYPE"}}
+     {:field-name "interval_ytm",  :base-type {:native "INTERVAL YEAR TO MONTH"}}
+     {:field-name "interval_dts",  :base-type {:native "INTERVAL DAY TO SECOND"}}
+     {:field-name "geo",           :base-type {:native "GEOMETRY"}}]
+    [; Rows
+     [0 "null-values"      nil                                    nil  nil              nil]
+     [1 "non-null-values" "550e8400-e29b-11d4-a716-446655440000" "5-3" "2 12:50:10.123" "POINT(2 5)"]]]])
+
+
+(tx/defdataset geometry
+  "Test data for testing geometry functions, see https://docs.exasol.com/sql_references/geospatialdata/geospatialdata_overview.htm#GeospatialObjects"
+  [["geo"
+    ; Columns
+    [{:field-name "type",     :base-type :type/Text}
+     {:field-name "geo",      :base-type {:native "GEOMETRY"}}]
+    [; Rows
+     ["point"   "POINT(2 5)"]
+     ["linestring" "LINESTRING(11 1, 15 2, 15 10)"]
+     ["linearring" "LINEARRING(2 1, 3 3, 4 1, 2 1)"]
+     ["polygon" "POLYGON((5 1, 5 5, 9 7, 10 1, 5 1), (6 2, 6 3, 7 3, 7 2, 6 2))"]
+     ["geometrycollection" "GEOMETRYCOLLECTION(POINT(2 5), LINESTRING(1 1, 15 2, 15 10))"]
+     ["multipoint" "MULTIPOINT(0.1 1.4, 2.2 3, 1 6.4)"]
+     ["multilinestring" "MULTILINESTRING((0 1, 2 3, 1 6), (4 4, 5 5))"]
+     ["multipolygon" "MULTIPOLYGON(((0 0, 0 2, 2 2, 3 1, 0 0)), ((4 6, 8 9, 12 5, 4 6), (8 6, 9 6, 9 7, 8 7, 8 6)))"]]]])

test_unit/metabase/driver/exasol_test.clj

@@ -57,21 +57,21 @@
     (doseq [[db-type expected-type] [["BOOLEAN"                         :type/Boolean]
                                      ["CHAR"                            :type/Text]
                                      ["VARCHAR"                         :type/Text]
-                                     ["HASHTYPE"                        :type/Text]
                                      ["BIGINT"                          :type/Decimal]
                                      ["DECIMAL"                         :type/Decimal]
                                      ["DOUBLE PRECISION"                :type/Float]
                                      ["DOUBLE"                          :type/Float]
                                      ["DATE"                            :type/Date]
                                      ["TIMESTAMP"                       :type/DateTime]
                                      ["TIMESTAMP WITH LOCAL TIME ZONE"  :type/DateTime]
-                                     ["INTERVAL DAY TO SECOND"          :type/Text]
-                                     ["INTERVAL YEAR TO MONTH"          :type/Text]
-                                     ["GEOMETRY"                        :type/Text]]]
+                                     ["INTERVAL DAY TO SECOND"          :type/*]
+                                     ["INTERVAL YEAR TO MONTH"          :type/*]
+                                     ["GEOMETRY"                        :type/*]
+                                     ["HASHTYPE"                        :type/*]]]
       (is (= expected-type (sql-jdbc.sync/database-type->base-type :exasol db-type))
           (format "Database type %s returns %s" db-type expected-type))))
   (testing "Unknown types return nil"
-    (doseq [db-type ["unknown" "boolean" "" nil]]
+    (doseq [db-type ["unknown" "boolean" "" nil " CHAR" "CHAR " "INTERVAL"]]
       (is (= nil (sql-jdbc.sync/database-type->base-type :exasol db-type))
           (format "Database type %s returns nil" db-type)))))