Don't load all records into memory in the import rake task #471

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@alan
Contributor
alan commented Oct 8, 2012

Since #381 the rake task to bulk import all the records loads all the records into memory to count the number of records. The change was done because what looks like what was a bug in ActiveRecord when using default_scope and sql grouping.

I'd like to get that change reverted back, otherwise there is no point on paginating the records from the DB since the all the records have been previously loaded into memory. I've also tried to replicate the default_scope bug with ActiveRecord v3.2.8 (latest released version) and it looks like the issue with count and grouping has been fixed.

We'll be soon doing a bulk import and the current task with this behaviour of loading all the records into memory is not practical

@karmi
Owner
karmi commented Oct 8, 2012

Hi, yes, makes sense -- will it work with other storage engines such as MongoDB etc?

@karmi karmi was assigned Oct 8, 2012
@alan
Contributor
alan commented Oct 8, 2012

I don't have a setup with mongoDB and Tire, but I suspect that as long as the mongo client implements ActiveModel it should work.

I've tested Model.count with a Mongoid Model and it works so I see no reason why it wouldn't work.

@alan
Contributor
alan commented Oct 15, 2012

Any updates on this PR @karmi ? I believe Mongo based ORM's should keep working as expected. Let me know if you want me to check anything else to get this merged in.

@karmi
Owner
karmi commented Oct 15, 2012

Hi, will test it agains a Mongoid based app, so we don't break it for them. Should get to it this week.

@karmi karmi added a commit that closed this pull request Oct 16, 2012
@alan @karmi alan + [FIX] Use `MyModel.count`, not `MyModel.all.count` in the Rake import…
… task

Previously, all records had to be loaded into memory (errr),
better to use the `Model.count` method

Closes #471
879a048
@karmi karmi closed this in 879a048 Oct 16, 2012
@karmi
Owner
karmi commented Oct 16, 2012

Thanks!, verified, merged, pushed.

@NOX73 NOX73 added a commit to NOX73/tire that referenced this pull request Oct 23, 2012
@alan @NOX73 alan + NOX73 [FIX] Use `MyModel.count`, not `MyModel.all.count` in the Rake import…
… task

Previously, all records had to be loaded into memory (errr),
better to use the `Model.count` method

Closes #471
027ad53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment