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

Relation#each_batch doesn't respect limited datasets #329

Open
ianks opened this issue Apr 25, 2019 · 2 comments
Open

Relation#each_batch doesn't respect limited datasets #329

ianks opened this issue Apr 25, 2019 · 2 comments
Labels

Comments

@ianks
Copy link
Collaborator

ianks commented Apr 25, 2019

Describe the bug

When using Relation#each_batch, if you pass in a relation that already has a limit applied, Relation#each_batch will not respect it and process the batches as if the limit did not exist.

To Reproduce

users = ROM.env.relations[:users]
1000.times { create_user }
top_100 = users.limit(10)

i = 0
top_100.each_batch(size: 10) do |batch|
   i += 1
  raise 'This is unexpected...' if i > 10
end

Expected behavior

Relation#each_batch should respect the passed in limit, ideally by wrapping the input dataset in a subquery such that it will not be manipulated.

Your environment

  • Affects my production application: YES
  • Ruby version: 2.6.2
  • OS: Linux
@ianks ianks added the bug label Apr 25, 2019
@flash-gordon
Copy link
Member

Why do you think wrapping in a subquery is ideal here? In my opinion, taking current limit into consideration makes more sense.

@ianks
Copy link
Collaborator Author

ianks commented Apr 25, 2019

That's good as well. The subquery is nice because you do not have to worry about twiddling around calculating the correct limits, and the user would not have to worry bout accidentally overriding the limit after the batch is yielded. However, it would add some complexity w/r/t remapping the current current clauses (i.e. making sure the exact same where clauses, etc) are added back to the yielded dataset. Either option is an improvement :)

@solnic solnic added this to the v3.0.1 milestone Apr 25, 2019
@solnic solnic removed this from the v3.0.1 milestone May 5, 2019
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

3 participants