Redmine

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
)

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:

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?

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