Vote #81779
完了Cleanup more dependent objects on project delete
0%
説明
On project deletion, there are currently some rows in the database which are not correctly destroyed and thus become orphaned.
With this patch, we will destroy overridden time entry activities (and all of their children) of a project on project destroy.
We now also destroy dependent objects of stored queries for a project on destroy. This will cleanup rows from the habtm join table @queries_roles@ which were also orphaned before.
The attached patch is against current trunk but should also apply to older versions. When we apply this, we may also want to add a migration to cleanup previously orphaned rows in the database.
journals
--------------------------------------------------------------------------------
Attached is an updated patch which included a migration to delete potentially orphaned database rows.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
Holger, thanks for the patches. I've committed most of the changes (r21437 and r21438), but the migration for OrphanedQueriesRoles throws the below error on PostgreSQL after I changed from @DELETE #{queries_roles} FROM #{queries_roles}...@ to @DELETE FROM #{queries_roles}...@.
<pre>
Caused by:
PG::SyntaxError: ERROR: syntax error at or near "LEFT"
LINE 1: DELETE FROM queries_roles LEFT OUTER JOIN queries ON queries...
</pre>
--------------------------------------------------------------------------------
What do you think about the following query migration?
<pre><code class="ruby">
queries_roles = "#{Query.table_name_prefix}queries_roles#{Query.table_name_suffix}"
queries = Query.table_name
ActiveRecord::Base.connection.execute "DELETE FROM #{queries_roles} WHERE query_id IN (SELECT #{queries_roles}.query_id FROM #{queries_roles} LEFT OUTER JOIN #{queries} ON #{queries_roles}.query_id = #{queries}.id WHERE #{queries}.id IS NULL)"
</code></pre>
it will generate the following query:
<pre><code class="sql">
DELETE FROM queries_roles WHERE query_id IN (
SELECT queries_roles.query_id FROM queries_roles
LEFT OUTER JOIN queries ON queries_roles.query_id = queries.id WHERE queries.id IS NULL
)
</code></pre>
--------------------------------------------------------------------------------
Also, it seems like the habtm join table @queries_roles@ is not clean up when a role is destroyed and we should fix this case as well.
--------------------------------------------------------------------------------
Marius BALTEANU wrote:
> What do you think about the following query migration?
>
> [...]
This fails on MySQL (or at least older MySQL versions):
> You can't specify target table 'queries_roles' for update in FROM clause
The version I have proposed works there.
> Also, it seems like the habtm join table queries_roles is not clean up when a role is destroyed and we should fix this case as well.
That is true. It might be enough to add a simple habtm relation to the @Role@ model here. If I'm not mistaken, then Rails will automatically cleanup rows from the join table on destroy.
--------------------------------------------------------------------------------
Holger Just wrote:
>
> That is true. It might be enough to add a simple habtm relation to the @Role@ model here. If I'm not mistaken, then Rails will automatically cleanup rows from the join table on destroy.
Yes, it works as you said, I've added this change in r21444.
Regarding the migrations, I tested the following queries on both MySQL and PostgreSQL and it works as expected:
Orphaned by query:
<pre><code class="sql">
DELETE FROM queries_roles WHERE query_id NOT IN (SELECT DISTINCT(id) FROM queries);
</code></pre>
Orphaned by role:
<pre><code class="sql">
DELETE FROM queries_roles WHERE role_id NOT IN (SELECT DISTINCT(id) FROM roles);
</code></pre>
Migration:
<pre><code class="ruby">
class DeleteOrphanedQueriesRoles < ActiveRecord::Migration[6.1]
def self.up
queries_roles = "#{Query.table_name_prefix}queries_roles#{Query.table_name_suffix}"
queries = Query.table_name
roles = Role.table_name
ActiveRecord::Base.connection.execute "DELETE FROM #{queries_roles} WHERE query_id NOT IN (SELECT DISTINCT(id) FROM #{queries})"
ActiveRecord::Base.connection.execute "DELETE FROM #{queries_roles} WHERE role_id NOT IN (SELECT DISTINCT(id) FROM #{roles})"
end
def self.down
# no-op
end
end
</code></pre>
Do you see any issue with this approach?
--------------------------------------------------------------------------------
Holger, did you have a chance to check my last comment?
--------------------------------------------------------------------------------
Sorry for the late reply. Your approach is a nice alternative which should probably work fine. I don't see any obvious issues there.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
Holger Just wrote:
> Sorry for the late reply. Your approach is a nice alternative which should probably work fine. I don't see any obvious issues there.
Thanks Holger! I've created #36844 to track the migration because version:"5.0.0" will be released as soon as possible.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
related_issues
relates,New,36844,Cleanup orphaned query and role ids from habtm join table queries_roles