Skip to content
Snippets Groups Projects
Unverified Commit dd1eb6cb authored by Cal Herries's avatar Cal Herries Committed by GitHub
Browse files

Fix fields from Amazon Athena Tables failing to sync if there is not an...

Fix fields from Amazon Athena Tables failing to sync if there is not an underscore in the Table Name (#44032)
parent 27e1ff64
No related branches found
No related tags found
No related merge requests found
......@@ -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
......
......@@ -350,11 +350,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
......
(ns metabase.driver.athena-test
(:require
[clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[clojure.test :refer :all]
[honey.sql :as sql]
......@@ -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"}
......@@ -218,3 +226,30 @@
(let [result (query->native! query)]
(is (string? result))
(is (str/starts-with? result "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% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment