Skip to content
Snippets Groups Projects
Unverified Commit e3bd7388 authored by Tim Macdonald's avatar Tim Macdonald Committed by GitHub
Browse files

[CSV] Avoid expensive call to fetch DB metadata (#31556)

* Avoid expensive call to fetch DB metadata

Double-checking the schema name is unnecessarily defensive since we
already call include-schema?

* Check that schema is set
parent a03a9e5f
No related branches found
No related tags found
No related merge requests found
......@@ -49,7 +49,6 @@
[metabase.server.middleware.offset-paging :as mw.offset-paging]
[metabase.sync :as sync]
[metabase.sync.analyze.query-results :as qr]
[metabase.sync.fetch-metadata :as fetch-metadata]
[metabase.sync.sync-metadata.fields :as sync-fields]
[metabase.sync.sync-metadata.tables :as sync-tables]
[metabase.task.persist-refresh :as task.persist-refresh]
......@@ -1168,6 +1167,10 @@ saved later when it is ready."
_check_perms (api/check-403 (mi/can-read? database))
driver (driver.u/database->driver database)
schema-name (public-settings/uploads-schema-name)
_check-schema (when (and (str/blank? schema-name)
(driver/database-supports? driver :schemas database))
(throw (ex-info (tru "A schema has not been set.")
{:status-code 422})))
_check-schema (when-not (or (nil? schema-name)
(driver.s/include-schema? database schema-name))
(throw (ex-info (tru "The schema {0} is not syncable." schema-name)
......@@ -1184,14 +1187,7 @@ saved later when it is ready."
(str schema-name "." table-name))
stats (upload/load-from-csv driver db-id schema+table-name csv-file)
;; Syncs are needed immediately to create the Table and its Fields; the scan is settings-dependent and can be async
table-metadata (first (filter (fn [{:keys [name]}] (= (u/lower-case-en name) table-name))
(:tables (fetch-metadata/db-metadata database))))
actual-schema (:schema table-metadata)
_ (when (nil? table-metadata)
(driver/drop-table driver db-id table-name)
(throw (ex-info (tru "The CSV file was uploaded to {0}, but the table could not be created or found." schema+table-name)
{:status-code 422})))
table (sync-tables/create-or-reactivate-table! database {:name table-name :schema actual-schema})
table (sync-tables/create-or-reactivate-table! database {:name table-name :schema (not-empty schema-name)})
_sync (scan-and-sync-table! database table)
card (create-card!
{:collection_id collection-id,
......
......@@ -2879,14 +2879,20 @@
uploads-database-id db-id
uploads-schema-name nil
uploads-table-prefix "uploaded_magic_"]
(let [new-model (upload-example-csv! nil)
new-table (t2/select-one Table :db_id db-id)]
(is (= "Example Csv File" (:name new-model)))
(is (=? {:name #"(?i)uploaded_magic_example(.*)"}
new-table))
(if (= driver/*driver* :mysql)
(is (nil? (:schema new-table)))
(is (=? {:schema #"(?i)public"} new-table))))))))))
(if (= driver/*driver* :mysql)
(let [new-model (upload-example-csv! nil)
new-table (t2/select-one Table :db_id db-id)]
(is (= "Example Csv File" (:name new-model)))
(is (=? {:name #"(?i)uploaded_magic_example(.*)"}
new-table))
(if (= driver/*driver* :mysql)
(is (nil? (:schema new-table)))
(is (=? {:schema #"(?i)public"} new-table))))
;; Else, for drivers that support schemas
(is (thrown-with-msg?
java.lang.Exception
#"^A schema has not been set."
(upload-example-csv! nil))))))))))
(deftest upload-csv!-failure-test
;; Just test with postgres because failure should be independent of the driver
......
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