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

Support descending order for in_batches without block. #45282

Merged
merged 1 commit into from Jun 12, 2022

Conversation

alpaca-tc
Copy link
Contributor

Summary

Follow-up #30590.
In the feature of descending order for batches introduced in #30590 but the case of calling in_batches without a block was not supported.
Perhaps the reason it was not supported was a correction omission.

Other Information

When calling in_batches with block. It works fine.

# Post Pluck (0.1ms)  SELECT "posts"."id" FROM "posts" ORDER BY "posts"."id" DESC LIMIT ?  [["LIMIT", 1000]]
Post.in_batches(order: :desc) {}

When calling in_batches without block.

before: expected DESC, got ASC

# Post Pluck (0.1ms)  SELECT "posts"."id" FROM "posts" ORDER BY "posts"."id" ASC LIMIT ?  [["LIMIT", 1000]]
Post.in_batches(order: :desc).each {}

after

# Post Pluck (0.1ms)  SELECT "posts"."id" FROM "posts" ORDER BY "posts"."id" DESC LIMIT ?  [["LIMIT", 1000]]
Post.in_batches(order: :desc).each {}

Follow-up rails#30590.
In the feature introduced in rails#30590, the case of calling `in_batches` without a block was not supported.
Perhaps the reason it was not supported was a correction omission.
@yahonda
Copy link
Member

yahonda commented Jun 12, 2022

Would you explain why the order option is necessary without a block?
Unless a block is given, all of the records are handled in batches but the order does not look required.

@yahonda
Copy link
Member

yahonda commented Jun 12, 2022

Ah, I understand that without an order specified it may lead to wrong results because each batched record set order is not guaranteed.

@yahonda
Copy link
Member

yahonda commented Jun 12, 2022

Please disregard my previous two comments.

Regardless order by ... desc or order by ... asc without block lead to the same result because all of the batched results are identical, just the order is different.

but this behavior inconsistency may surprise users as you did. Let's merge this pull request.

@yahonda yahonda merged commit c54fd31 into rails:main Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants