diff --git a/resources/migrations/001_update_migrations.yaml b/resources/migrations/001_update_migrations.yaml
index a818c18ba144839a7e19c7e40a5adc3f5ae8abf5..85e06338b48e7c8d4e23732d28b2c0d715363283 100644
--- a/resources/migrations/001_update_migrations.yaml
+++ b/resources/migrations/001_update_migrations.yaml
@@ -8719,7 +8719,7 @@ databaseChangeLog:
- not:
- foreignKeyConstraintExists:
foreignKeyName: fk_pulse_channel_channel_id
- foreignKyeTableName: pulse_channel
+ foreignKeyTableName: pulse_channel
changes:
- addForeignKeyConstraint:
@@ -9503,6 +9503,64 @@ databaseChangeLog:
WHERE "KEY" = 'embedding-app-origin';
rollback: # not needed
+ - changeSet:
+ id: v51.2024-10-24T14:23:58
+ author: noahmoss
+ comment: Drop not null constraint on persisted_info.card_id
+ changes:
+ - dropNotNullConstraint:
+ tableName: persisted_info
+ columnName: card_id
+ columnDataType: int
+ rollback:
+ - addNotNullConstraint:
+ tableName: persisted_info
+ columnName: card_id
+ columnDataType: int
+
+ - changeSet:
+ id: v51.2024-10-24T14:24:59
+ author: noahmoss
+ comment: Drop foreign key constraint on persisted_info.card_id
+ preConditions:
+ - foreignKeyConstraintExists:
+ foreignKeyName: fk_persisted_info_card_id
+ foreignKeyTableName: persisted_info
+ changes:
+ - dropForeignKeyConstraint:
+ constraintName: fk_persisted_info_card_id
+ baseTableName: persisted_info
+ rollback:
+ - addForeignKeyConstraint:
+ baseTableName: persisted_info
+ baseColumnNames: card_id
+ referencedTableName: report_card
+ referencedColumnNames: id
+ constraintName: fk_persisted_info_card_id
+ onDelete: CASCADE
+
+ - changeSet:
+ id: v51.2024-10-24T14:25:01
+ author: noahmoss
+ comment: Re-add nullable foreign key constraint on persisted_info.card_id
+ preConditions:
+ - not:
+ - foreignKeyConstraintExists:
+ foreignKeyName: fk_persisted_info_card_id
+ foreignKeyTableName: persisted_info
+ changes:
+ - addForeignKeyConstraint:
+ baseTableName: persisted_info
+ baseColumnNames: card_id
+ referencedTableName: report_card
+ referencedColumnNames: id
+ constraintName: fk_persisted_info_card_id
+ onDelete: SET NULL
+ rollback:
+ - dropForeignKeyConstraint:
+ constraintName: fk_persisted_info_card_id
+ baseTableName: persisted_info
+
# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
########################################################################################################################
diff --git a/src/metabase/task/persist_refresh.clj b/src/metabase/task/persist_refresh.clj
index 3d126719037489f629de955b35cd0c2abe423260..21fad995b29002a6c93fdb2fe1d601fc7eaa4afe 100644
--- a/src/metabase/task/persist_refresh.clj
+++ b/src/metabase/task/persist_refresh.clj
@@ -13,9 +13,7 @@
[metabase.email.messages :as messages]
[metabase.models.card :refer [Card]]
[metabase.models.database :refer [Database]]
- [metabase.models.persisted-info
- :as persisted-info
- :refer [PersistedInfo]]
+ [metabase.models.persisted-info :as persisted-info]
[metabase.models.task-history :as task-history]
[metabase.public-settings :as public-settings]
[metabase.query-processor.middleware.limit :as limit]
@@ -56,12 +54,12 @@
(defn- refresh-with-stats! [refresher database stats persisted-info]
;; Since this could be long running, double check state just before refreshing
- (when (contains? (persisted-info/refreshable-states) (t2/select-one-fn :state PersistedInfo :id (:id persisted-info)))
+ (when (contains? (persisted-info/refreshable-states) (t2/select-one-fn :state :model/PersistedInfo :id (:id persisted-info)))
(log/infof "Attempting to refresh persisted model %s." (:card_id persisted-info))
(let [card (t2/select-one Card :id (:card_id persisted-info))
definition (persisted-info/metadata->definition (:result_metadata card)
(:table_name persisted-info))
- _ (t2/update! PersistedInfo (u/the-id persisted-info)
+ _ (t2/update! :model/PersistedInfo (u/the-id persisted-info)
{:definition definition,
:query_hash (persisted-info/query-hash (:dataset_query card))
:active false,
@@ -75,7 +73,7 @@
(log/infof e "Error refreshing persisting model with card-id %s"
(:card_id persisted-info))
{:state :error :error (ex-message e)}))]
- (t2/update! PersistedInfo (u/the-id persisted-info)
+ (t2/update! :model/PersistedInfo (u/the-id persisted-info)
{:active (= state :success),
:refresh_end :%now,
:state (if (= state :success) "persisted" "error")
@@ -98,7 +96,7 @@
(try
(let [error-details (error-details task-details)
error-details-by-id (m/index-by :persisted-info-id error-details)
- persisted-infos (->> (t2/hydrate (t2/select PersistedInfo :id [:in (keys error-details-by-id)])
+ persisted-infos (->> (t2/hydrate (t2/select :model/PersistedInfo :id [:in (keys error-details-by-id)])
[:card :collection] :database)
(map #(assoc % :error (get-in error-details-by-id [(:id %) :error]))))]
(messages/send-persistent-model-error-email!
@@ -130,7 +128,7 @@
unpersist-fn (fn []
(reduce (fn [stats persisted-info]
;; Since this could be long running, double check state just before deleting
- (let [current-state (t2/select-one-fn :state PersistedInfo :id (:id persisted-info))
+ (let [current-state (t2/select-one-fn :state :model/PersistedInfo :id (:id persisted-info))
card-info (t2/select-one [Card :archived :type]
:id (:card_id persisted-info))]
(if (or (contains? (persisted-info/prunable-states) current-state)
@@ -141,7 +139,7 @@
(try
(unpersist! refresher database persisted-info)
(when-not (= "off" current-state)
- (t2/delete! PersistedInfo :id (:id persisted-info)))
+ (t2/delete! :model/PersistedInfo :id (:id persisted-info)))
(update stats :success inc)
(catch Exception e
(log/infof e "Error unpersisting model with card-id %s" (:card_id persisted-info))
@@ -154,9 +152,10 @@
(defn- deletable-models
"Returns persisted info records that can be unpersisted. Will select records that have moved into a deletable state
after a sufficient delay to ensure no queries are running against them and to allow changing mind. Also selects
- persisted info records pointing to cards that are no longer models and archived cards/models."
+ persisted info records pointing to cards that are no longer models, archived cards/models, and all records where the corresponding
+ card or database has been permanently deleted."
[]
- (t2/select PersistedInfo
+ (t2/select :model/PersistedInfo
{:select [:p.*]
:from [[:persisted_info :p]]
:left-join [[:report_card :c] [:= :c.id :p.card_id]]
@@ -170,12 +169,14 @@
[:< :state_change_at
(sql.qp/add-interval-honeysql-form (mdb/db-type) :%now -1 :hour)]]
[:= :c.type "question"]
- [:= :c.archived true]]}))
+ [:= :c.archived true]
+ ;; card_id is set to null when the corresponding card is deleted
+ [:= :p.card_id nil]]}))
(defn- refreshable-models
"Returns refreshable models for a database id. Must still be models and not archived."
[database-id]
- (t2/select PersistedInfo
+ (t2/select :model/PersistedInfo
{:select [:p.* :c.type :c.archived :c.name]
:from [[:persisted_info :p]]
:left-join [[:report_card :c] [:= :c.id :p.card_id]]
@@ -211,7 +212,7 @@
(defn- refresh-individual!
"Refresh an individual model based on [[PersistedInfo]]."
[persisted-info-id refresher]
- (let [persisted-info (t2/select-one PersistedInfo :id persisted-info-id)
+ (let [persisted-info (t2/select-one :model/PersistedInfo :id persisted-info-id)
database (when persisted-info
(t2/select-one Database :id (:database_id persisted-info)))]
(if (and persisted-info database)
diff --git a/test/metabase/task/persist_refresh_test.clj b/test/metabase/task/persist_refresh_test.clj
index b7ce361fa59d6b2422a9adbd950bfd0ddc1b52b9..617b782eecf2c732780443e1eb550a6355d95696 100644
--- a/test/metabase/task/persist_refresh_test.clj
+++ b/test/metabase/task/persist_refresh_test.clj
@@ -193,7 +193,9 @@
PersistedInfo deletable {:card_id (u/the-id model3) :database_id (u/the-id db)
:state "deletable"
;; need an "old enough" state change
- :state_change_at (t/minus (t/local-date-time) (t/hours 2))}]
+ :state_change_at (t/minus (t/local-date-time) (t/hours 2))}
+ ;; Record not in "deletable" state, but with nil card_id
+ PersistedInfo deletable2 {:card_id nil :database_id (u/the-id db)}]
(let [called-on (atom #{})
test-refresher (reify task.persist-refresh/Refresher
(refresh! [_ _ _ _]
@@ -204,19 +206,20 @@
(let [queued-for-deletion (into #{} (map :id) (#'task.persist-refresh/deletable-models))]
(doseq [deletable-persisted [deletable punmodeled parchived]]
(is (contains? queued-for-deletion (u/the-id deletable-persisted))))))
- ;; we manually pass in the deleteable ones to not catch others in a running instance
+ ;; we manually pass in the deleteable ones to not catch others in a running instance
(#'task.persist-refresh/prune-deletables! test-refresher [deletable parchived punmodeled])
(testing "We delete persisted_info records for all of the pruned"
(let [persisted-records (t2/select :model/PersistedInfo :id [:in (map :id [parchived punmodeled deletable])])
existing (map (comp
(update-keys {parchived 'parchived
punmodeled 'punmodeled
- deletable 'deletable}
+ deletable 'deletable
+ deletable2 'deletable2}
:id)
:id)
persisted-records)]
(is (= [] existing))))
- ;; don't assert equality if there are any deletable in the app db
+ ;; don't assert equality if there are any deletable in the app db
(doseq [deletable-persisted [deletable punmodeled parchived]]
(is (contains? @called-on (u/the-id deletable-persisted))))
(is (partial= {:task "unpersist-tables"