Skip to content

Commit

Permalink
[backport for 49] Fix fields from Amazon Athena Tables failing to syn…
Browse files Browse the repository at this point in the history
…c if there is not an underscore in the Table Name (metabase#44032) (metabase#44245)
  • Loading branch information
calherries authored Jun 15, 2024
1 parent 31fa70c commit 7e2b7bb
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/drivers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ jobs:
MB_ATHENA_TEST_ACCESS_KEY: ${{ secrets.MB_ATHENA_TEST_ACCESS_KEY }}
MB_ATHENA_TEST_SECRET_KEY: ${{ secrets.MB_ATHENA_TEST_SECRET_KEY }}
MB_ATHENA_TEST_S3_STAGING_DIR: ${{ secrets.MB_ATHENA_TEST_S3_STAGING_DIR }}
# These credentials are used to test the driver when the user does not have the athena:GetTableMetadata permission
MB_ATHENA_TEST_WITHOUT_GET_TABLE_METADATA_ACCESS_KEY: ${{ secrets.MB_ATHENA_TEST_WITHOUT_GET_TABLE_METADATA_ACCESS_KEY }}
MB_ATHENA_TEST_WITHOUT_GET_TABLE_METADATA_SECRET_KEY: ${{ secrets.MB_ATHENA_TEST_WITHOUT_GET_TABLE_METADATA_SECRET_KEY }}
steps:
- uses: actions/checkout@v4
- name: Test Athena driver
Expand Down
9 changes: 6 additions & 3 deletions modules/drivers/athena/src/metabase/driver/athena.clj
Original file line number Diff line number Diff line change
Expand Up @@ -349,11 +349,14 @@

(defn describe-table-fields
"Returns a set of column metadata for `schema` and `table-name` using `metadata`. "
[^DatabaseMetaData metadata database driver {^String schema :schema, ^String table-name :name}, & [^String db-name-or-nil]]
[^DatabaseMetaData metadata database driver {^String schema :schema, ^String table-name :name} catalog]
(try
(with-open [rs (.getColumns metadata db-name-or-nil schema table-name nil)]
(with-open [rs (.getColumns metadata catalog schema table-name nil)]
(let [columns (jdbc/metadata-result rs)]
(if (table-has-nested-fields? columns)
(if (or (table-has-nested-fields? columns)
; If `.getColumns` returns an empty result, try to use DESCRIBE, which is slower
; but doesn't suffer from the bug in the JDBC driver as metabase#43980
(empty? columns))
(describe-table-fields-with-nested-fields database schema table-name)
(describe-table-fields-without-nested-fields driver columns))))
(catch Throwable e
Expand Down
37 changes: 36 additions & 1 deletion modules/drivers/athena/test/metabase/driver/athena_test.clj
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(ns metabase.driver.athena-test
(:require
[clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[clojure.test :refer :all]
[honey.sql :as sql]
Expand All @@ -12,7 +13,14 @@
[metabase.lib.test-util :as lib.tu]
[metabase.public-settings.premium-features :as premium-features]
[metabase.query-processor :as qp]
[metabase.test :as mt]))
[metabase.sync :as sync]
[metabase.test :as mt]
[metabase.test.data.interface :as tx]
[toucan2.core :as t2])
(:import
(java.sql Connection)))

(set! *warn-on-reflection* true)

(def ^:private nested-schema
[{:col_name "key", :data_type "int"}
Expand Down Expand Up @@ -210,3 +218,30 @@
(testing "Query starts with select instead of a remark"
(mt/with-metadata-provider (mock-provider false)
(is (str/starts-with? (query->native query) "SELECT")))))))

(deftest describe-table-works-without-get-table-metadata-permission-test
(testing "`describe-table` works if the AWS user's IAM policy doesn't include athena:GetTableMetadata permissions")
(mt/test-driver :athena
(mt/dataset airports
(let [catalog "AwsDataCatalog" ; The bug only happens when :catalog is not nil
details (assoc (:details (mt/db))
;; these credentials are for a user that doesn't have athena:GetTableMetadata permissions
:access_key (tx/db-test-env-var-or-throw :athena :without-get-table-metadata-access-key)
:secret_key (tx/db-test-env-var-or-throw :athena :without-get-table-metadata-secret-key)
:catalog catalog)]
(mt/with-temp [:model/Database db {:engine :athena, :details details}]
(sync/sync-database! db {:scan :schema})
(let [table (t2/select-one :model/Table :db_id (:id db) :name "airport")]
(testing "Check that .getColumns returns no results, meaning the athena JDBC driver still has a bug"
;; If this test fails and .getColumns returns results, the athena JDBC driver has been fixed and we can
;; undo the changes in https://github.com/metabase/metabase/pull/44032
(is (empty? (sql-jdbc.execute/do-with-connection-with-options
:athena
db
nil
(fn [^Connection conn]
(let [metadata (.getMetaData conn)]
(with-open [rs (.getColumns metadata catalog (:schema table) (:name table) nil)]
(jdbc/metadata-result rs))))))))
(testing "`describe-table` returns the fields anyway"
(is (not-empty (:fields (driver/describe-table :athena db table)))))))))))

0 comments on commit 7e2b7bb

Please sign in to comment.