diff --git a/src/metabase/db/migrations.clj b/src/metabase/db/migrations.clj index d9938492df009ae1643040005323ca683d260091..918ce9280763fcde961e7ca7f6f2ee9f1ef5e2e0 100644 --- a/src/metabase/db/migrations.clj +++ b/src/metabase/db/migrations.clj @@ -279,17 +279,26 @@ (doseq [database-id (db/select-ids Database :engine "bigquery")] ;; For each Card belonging to that BigQuery database... (doseq [{query :dataset_query, card-id :id} (db/select [Card :id :dataset_query] :database_id database-id)] - ;; If the Card isn't native, ignore it - (when (= (keyword (:type query)) :native) - (let [sql (get-in query [:native :query])] - ;; if the Card already contains a #standardSQL or #legacySQL (both are case-insenstive) directive, ignore it - (when-not (re-find #"(?i)#(standard|legacy)sql" sql) - ;; if it doesn't have a directive it would have (under old behavior) defaulted to legacy SQL, so give it a - ;; #legacySQL directive... - (let [updated-sql (str "#legacySQL\n" sql)] - ;; and save the updated dataset_query map - (db/update! Card (u/get-id card-id) - :dataset_query (assoc-in query [:native :query] updated-sql))))))))) + (try + ;; If the Card isn't native, ignore it + (when (= (keyword (:type query)) :native) + ;; there apparently are cases where we have a `:native` query with no `:query`. See #8924 + (when-let [sql (get-in query [:native :query])] + ;; if the Card already contains a #standardSQL or #legacySQL (both are case-insenstive) directive, ignore it + (when-not (re-find #"(?i)#(standard|legacy)sql" sql) + ;; if it doesn't have a directive it would have (under old behavior) defaulted to legacy SQL, so give it a + ;; #legacySQL directive... + (let [updated-sql (str "#legacySQL\n" sql)] + ;; and save the updated dataset_query map + (db/update! Card (u/get-id card-id) + :dataset_query (assoc-in query [:native :query] updated-sql)))))) + ;; if for some reason something above fails (as in #8924) let's log the error and proceed. It's not mission + ;; critical that we migrate existing queries anyway, and for ones that are impossible to migrate (e.g. ones + ;; that are invalid in the first place) it's best to fail gracefully and proceed rather than nuke someone's MB + ;; instance + (catch Throwable e + (log/error e (trs "Error adding legacy SQL directive to BigQuery saved Question"))))))) + ;; Before 0.30.0, we were storing the LDAP user's password in the `core_user` table (though it wasn't used). This ;; migration clears those passwords and replaces them with a UUID. This is similar to a new account setup, or how we diff --git a/test/metabase/db/migrations_test.clj b/test/metabase/db/migrations_test.clj index 3c8c3b5e14ebc33ed36c0c92fd04cd82891a8c1c..c6430ed6a25b8110aa9bde6202e4f00d5344b9d6 100644 --- a/test/metabase/db/migrations_test.clj +++ b/test/metabase/db/migrations_test.clj @@ -47,6 +47,19 @@ (->> (db/select-field->field :name :dataset_query Card :id [:in (map u/get-id [card-1 card-2])]) (m/map-vals #(update % :database integer?))))) +;; if for some reason we have a BigQuery native query that does not actually have any SQL, ignore it rather than +;; barfing (#8924) (No idea how this was possible, but clearly it was) +(expect + {:database true, :type :native, :native {:query 1000}} + (tt/with-temp* [Database [database {:engine "bigquery"}] + Card [card {:database_id (u/get-id database) + :dataset_query {:database (u/get-id database) + :type :native + :native {:query 1000}}}]] + (#'migrations/add-legacy-sql-directive-to-bigquery-sql-cards) + (-> (db/select-one-field :dataset_query Card :id (u/get-id card)) + (update :database integer?)))) + ;; Test clearing of LDAP user local passwords (expect [false true]