rom-rb / rom

Data mapping and persistence toolkit for Ruby
https://rom-rb.org
MIT License
2.09k stars 161 forks source link

Stack level too deep when query for a large list of records #341

Closed edwardlftam closed 7 years ago

edwardlftam commented 8 years ago

I am having problem in a query where 138706 records were to be returned, and I am experiencing the following exception:

SystemStackError: stack level too deep
    from /Users/adknowledge/.rbenv/versions/2.1.5/lib/ruby/2.1.0/irb/workspace.rb:86

The following is an example of how I structured my code:

class ExampleRepo <  ROM::Repository::Base
     relations :apples

      def find_by(conditions)
        apples.where(conditions).as(mapper).to_a
      end
end

Simply calling ExampleRepo.new(container).find_by({}) will cause a stack level problem when the number of rows returned is large enough.

Any help would be appreciated. Thanks!

solnic commented 8 years ago

Are you able to load this enormous dataset using plain Sequel? Ie:

apples.dataset.where(conditions).to_a

This would use plain sequel dataset to load hash objects.

I need to know if this happens inside rom or in some lower level, like rom-sql adapter backed by sequel datasets.

In general, you should really not try to load so many objects, it's gonna kill performance completely.

edwardlftam commented 8 years ago

Thanks for your reply. Loading from plain Sequel seems to work.

I don't load so many rows normally, but there is a edge use case that I need to do so. I need to compare the data in my database with the data pulled from a third party API to figure out if anything was deleted remotely, and this only gets run once a day handled in the background.

solnic commented 8 years ago

Does it work without a mapper then? ie apples.where(conditions).to_a?

edwardlftam commented 8 years ago

apples.where(conditions).to_a causes the SystemStackError exception as well

solnic commented 8 years ago

what about apples.relation.where(conditions).to_a?

edwardlftam commented 8 years ago

apples.relation.where(conditions).to_a seemed to work

solnic commented 8 years ago

Could you narrow down the stacktrace to the portion which is related to rom-repository? Seems like it's happening somewhere there.

edwardlftam commented 8 years ago

When I byebug into it, seems to me that the exception is raised from here:

[15, 24] in /Users/adknowledge/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/rom-1.0.0/lib/rom/relation/materializable.rb
   15:       #
   16:       # @return [Array]
   17:       #
   18:       # @api public
   19:       def to_a
=> 20:         call.to_a
   21:       end
   22:       alias_method :to_ary, :to_a
   23:
   24:       # Yield relation tuples
solnic commented 8 years ago

I'll try to reproduce it and get back to you. Thanks for reporting the issue.

edwardlftam commented 8 years ago

Thanks. I think the one step before might be helpful too:

[43, 52] in /Users/adknowledge/.rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/rom-repository-0.2.0/lib/rom/repository/loading_proxy.rb
   43:       # Map this relation with other mappers too
   44:       #
   45:       # @api public
   46:       def map_with(*names)
   47:         if names.size == 1 && names[0].is_a?(Class)
=> 48:           with(meta: meta.merge(model: names[0]))
   49:         else
   50:           mappers = [mapper]+names.map { |name| relation.mappers[name] }
   51:           mappers.reduce(self) { |a, e| a >> e }
   52:         end
solnic commented 8 years ago

Oh ok. What do you use as the model class?

edwardlftam commented 8 years ago

I used dry-data 0.5.1, if you are referring to the mappers.

Update: to be exact, inherited from Dry::Data::Value

solnic commented 8 years ago

Could you try with dry-types (ex dry-data)? If that doesn't help, could you try another class that simply accepts a hash in the constructor and see if that works?

edwardlftam commented 8 years ago

Sure I can try that

solnic commented 8 years ago

@edward6882990 bump :) did you manage to test this with dry-types? is this still a problem? We're releasing rom 2.0 tomorrow so I'm trying to clean up Issues here

edwardlftam commented 8 years ago

I tried try-types and still ran into the same issue. Great to hear 2.0 is coming up soon, I will try again after upgrading

solnic commented 8 years ago

@edward6882990 OK thank you. I'll be investigating this, just need to generate 138706 records first :)