rebuild! on a scoped collection empties the entire table

[xob]

It would appear that executing rebuild! on a scoped collection empties the whole table before rebuilding the few selected elements.

In our case, the model is called BusinessUnit.

When you run BusinessUnit.where(id: [1, 2]).each { |bu| bu.rebuild! }, the following SQL queries are executed (amongst the INSERT queries):

DELETE FROM "business_unit_hierarchies" WHERE descendant_id IN (SELECT DISTINCT descendant_id FROM (SELECT descendant_id FROM "business_unit_hierarchies" WHERE ancestor_id = 2 OR descendant_id = 2) AS x)
DELETE FROM "business_unit_hierarchies" WHERE descendant_id IN (SELECT DISTINCT descendant_id FROM (SELECT descendant_id FROM "business_unit_hierarchies" WHERE ancestor_id = 1 OR descendant_id = 1) AS x)

When you run BusinessUnit.where(id: [1, 2]).rebuild!, the following SQL queries are executed (again, amongst the INSERT queries):

DELETE FROM "business_unit_hierarchies"
DELETE FROM "business_unit_hierarchies" WHERE descendant_id IN (SELECT DISTINCT descendant_id FROM (SELECT descendant_id FROM "business_unit_hierarchies" WHERE ancestor_id = 2 OR descendant_id = 2) AS x)
DELETE FROM "business_unit_hierarchies" WHERE descendant_id IN (SELECT DISTINCT descendant_id FROM (SELECT descendant_id FROM "business_unit_hierarchies" WHERE ancestor_id = 1 OR descendant_id = 1) AS x)

You will notice the extraneous DELETE FROM "business_unit_hierarchies", which completely empties the table before rebuilding only the two selected BusinessUnits.

We unfortunately experienced this problem in production, but were quickly able to recover by doing a full BusinessUnit.rebuild!. However, I think it would be nice to fix this, to avoid the same situation from happening to another unfortunate soul.

[mceachen]

Ugh, that's rough, and I'm glad you thought to rebuild the full table. (I'd also suggest taking backups before launching a prod console as a standard operating procedure, speaking from prior scarring production emergencies).

I hate to say it, but the ActiveRecord API is so expansive that documenting every way not to use every function is simply not tenable.

I'd be happy to accept a PR that can detect when rebuild! is being called from a scoped collection, and either raise an error, or remove the table truncation. Either way, at least we avoid your pain.

Thanks for taking the time to report the issue.

[kazalt]

What’s the point of the table truncation in the rebuild! class method? The only thing I can think of is to remove hierarchy elements that are not in the main table anymore. I changed the code to only remove those elements, but it seems useless since we had foreign_keys which restricted us from removing those elements (you too in your spec).

[mceachen]

@kazalt perhaps I'm misunderstanding you, but the tests don't truncate the hierarchies table to get around FK constraints. In other words, there aren't FK constraints to the _hierarchies tables, which would prevent rows being deleted from _hierarchies tables.

The class-level .rebuild! is to rebuild the hierarchies table from a "clean slate." If you don't start from an empty table, you can't make assertions against the contents.

[kazalt]

@mceachen the delete_hierarchy_references should be enough ? I mean, if i rebuild the whole table, each row will delete their own references before adding them back.

for the foreign keys, i'm talking about those :
add_foreign_key(:tag_hierarchies, :tags, :column => 'ancestor_id')
add_foreign_key(:tag_hierarchies, :tags, :column => 'descendant_id')

I can't delete an instance without deleting their hierarchies, so the truncation seems useless. I talk about that truncation : hierarchy_class.delete_all # not destroy_all -- we just want a simple truncate.

[mceachen]

@kazalt yes, if your FK constraints are all valid, the truncation shouldn't be necessary, but realize when I wrote this gem, rails 2 didn't even support foreign keys. Perhaps the truncate could be an option?

.delete_all doesn't call any callbacks, which is why I used it instead of destroy_all.

If you have time, please read, comment, and vote on #277 !

[gr8bit]

I have the same problem with STI. Given the example from the README, if I do

class Tag < ActiveRecord::Base
  has_closure_tree
end
class WhenTag < Tag ; end
class WhereTag < Tag ; end
class WhatTag < Tag ; end

and call WhatTag.rebuild!, it will first empty the hierarchy table, thus taking all the Tag hierarchies with it.

For the moment, I worked around it by overloading the rebuild! class methods in all subclasses like this:

class WhatTag < Tag
  def self.rebuild!
    Tag.rebuild!
  end
end

[mceachen]

@gr8bit thanks! I think that would be great to add to the docs. If you have time, I'd be happy to take a pr!

[gr8bit]

PR #287
Thank you for your great work! 👍

[mceachen]

Perfect, thanks!