diff --git a/.github/workflows/drivers.yml b/.github/workflows/drivers.yml index 604ae6299967059c8b7c3c55e7b131c2b41f0dba..f975ebf640adadfbbfdcc7a01412a249b3cf86ce 100644 --- a/.github/workflows/drivers.yml +++ b/.github/workflows/drivers.yml @@ -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 diff --git a/modules/drivers/athena/src/metabase/driver/athena.clj b/modules/drivers/athena/src/metabase/driver/athena.clj index bdd705815cd1d932c489f51ebdd0068a76a04940..800e98bbdfc1630cc4ddba2caa8d64bd3ce45a6c 100644 --- a/modules/drivers/athena/src/metabase/driver/athena.clj +++ b/modules/drivers/athena/src/metabase/driver/athena.clj @@ -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 diff --git a/modules/drivers/athena/test/metabase/driver/athena_test.clj b/modules/drivers/athena/test/metabase/driver/athena_test.clj index 5887c4a8cdf453a2149983dbff4ad18d90723a20..914448f55706ef1bfd202bb899a1986293682a8f 100644 --- a/modules/drivers/athena/test/metabase/driver/athena_test.clj +++ b/modules/drivers/athena/test/metabase/driver/athena_test.clj @@ -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] @@ -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)))))))))))