Project

General

Profile

Actions

Patch #36416

closed

Cleanup more dependent objects on project delete

Added by Holger Just about 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Category:
Database
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

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.


Files


Related issues

Related to Redmine - Patch #36844: Cleanup orphaned query and role ids from habtm join table queries_roles ClosedMarius BĂLTEANU

Actions
Related to Redmine - Defect #38286: "Cannot delete enumeration" error may occur when attempting to delete a project with time entriesClosedGo MAEDA

Actions
Actions #1

Updated by Go MAEDA about 2 years ago

  • Target version set to Candidate for next major release
Actions #2

Updated by Holger Just about 2 years ago

Attached is an updated patch which included a migration to delete potentially orphaned database rows.

Actions #3

Updated by Holger Just about 2 years ago

  • File deleted (0001-Cleanup-more-dependent-objects-on-project-delete.patch)
Actions #4

Updated by Marius BĂLTEANU about 2 years ago

  • Target version changed from Candidate for next major release to 5.0.0
Actions #5

Updated by Marius BĂLTEANU about 2 years ago

  • Assignee set to Marius BĂLTEANU

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}....

Caused by:
PG::SyntaxError: ERROR:  syntax error at or near "LEFT" 
LINE 1: DELETE FROM queries_roles LEFT OUTER JOIN queries ON queries...
Actions #6

Updated by Marius BĂLTEANU about 2 years ago

What do you think about the following query migration?

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)" 

it will generate the following query:

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
)

Actions #7

Updated by Marius BĂLTEANU about 2 years ago

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.

Actions #8

Updated by Holger Just about 2 years ago

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.

Actions #9

Updated by Marius BĂLTEANU about 2 years ago

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:

DELETE FROM queries_roles WHERE query_id NOT IN (SELECT DISTINCT(id) FROM queries);

Orphaned by role:

DELETE FROM queries_roles WHERE role_id NOT IN (SELECT DISTINCT(id) FROM roles);

Migration:

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

Do you see any issue with this approach?

Actions #10

Updated by Marius BĂLTEANU about 2 years ago

Holger, did you have a chance to check my last comment?

Actions #11

Updated by Holger Just almost 2 years ago

Sorry for the late reply. Your approach is a nice alternative which should probably work fine. I don't see any obvious issues there.

Actions #12

Updated by Marius BĂLTEANU almost 2 years ago

  • Related to Patch #36844: Cleanup orphaned query and role ids from habtm join table queries_roles added
Actions #13

Updated by Marius BĂLTEANU almost 2 years ago

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 5.0.0 will be released as soon as possible.

Actions #14

Updated by Marius BĂLTEANU almost 2 years ago

  • Status changed from New to Closed
Actions #15

Updated by Go MAEDA about 1 year ago

  • Related to Defect #38286: "Cannot delete enumeration" error may occur when attempting to delete a project with time entries added
Actions

Also available in: Atom PDF