Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preloading all ancestors #395

Closed
stem opened this issue Dec 18, 2018 · 3 comments
Closed

Preloading all ancestors #395

stem opened this issue Dec 18, 2018 · 3 comments
Labels

Comments

@stem
Copy link

stem commented Dec 18, 2018

Hi folks,

At solidusio/solidus#3004, we have a use case for which I can't find a good solution and I would like your advices.

We have many taxonomies, and each taxonomy is a nested set of taxons.
Taxons can be any depth level so they have ancestors.

We have an API endpoint where the user can ask for multiple taxons at once and we generate a "pretty name" for each one in the form of Ancestor3 > Ancestor2 > Ancestor1 > LeafTaxon.
To do that, we use the ancestors method, but it generates a new SQL query for each taxon the user asked to retrieve : a typical N+1 problem :/

What I would like to do would be to includes(:ancestors) but it's not defined as an has_many and thus don't work.

Do you have any tips on how to preload all ancestors of multiple taxons with the minimum amount of SQL queries ?

@stem
Copy link
Author

stem commented Dec 20, 2018

I've found associate_parents that helps a lot :)
Thanks to that method, we have a fix for our problem.

I let this issue open in case you want to add something inside the gem to help users do that even more easily. Maybe a class method preload_parents(collection) ?
We've ended up with something like :

      def preload_parents(models)
        parents = ModelClass.none

        models.map do |model|
          parents = parents.or(ModelClass.ancestors_of(model))
        end

        ModelClass.associate_parents(models + parents)
      end

and a scope like :

    scope :ancestors_of, ->(node) do
      left_condition  = arel_table[left_column_name].lt(node.left)
      right_condition = arel_table[right_column_name].gt(node.right)

      where(left_condition).where(right_condition)
    end

Would you consider a PR to add these 2 snippets ?

@VitaliyAdamkov
Copy link

I've found associate_parents that helps a lot :)
Thanks to that method, we have a fix for our problem.

I let this issue open in case you want to add something inside the gem to help users do that even more easily. Maybe a class method preload_parents(collection) ?
We've ended up with something like :

...

Would you consider a PR to add these 2 snippets ?

Great job, thank you!
I used it for my own Solidus-based project and have met "stack level too deep" when was trying to preload parents for more than 194 Spree::Taxon model instances in the database when doing it inside Sidekiq worker.

taxons = all
parents = none
taxons.all.each { |taxon| parents = parents.or(taxon.ancestors)
associate_parents(taxons + parents)

Google did not help me, so I made it in batches:

find_in_batches(batch_size: 100).flat_map do |taxons|
  parents = none
  taxons.each { |taxon| parents = parents.or(taxon.ancestors) }
  associate_parents(taxons + parents)
end

@stale
Copy link

stale bot commented Jan 10, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 10, 2021
@stale stale bot closed this as completed Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants