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

Add reselect method #33611

Merged
merged 8 commits into from Mar 1, 2019

Conversation

Projects
None yet
@willianveiga
Copy link
Contributor

willianveiga commented Aug 14, 2018

Allows you to change a previously set select statement:

Post.select(:title, :body).reselect(:created_at) # SELECT `posts.created_at` FROM `posts`

This is short-hand for unscope(:select).select(fields).

Note that we're unscoping the entire select statement.

def reselect(*fields)
  unscope(:select).select(*fields)
end

We already have rewhere and reorder. @brchristian said in #27340: "... I find it a helpful syntax to have, especially after getting used to the feel of the other two ..."

@rails-bot

This comment has been minimized.

Copy link

rails-bot commented Aug 14, 2018

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @pixeltrix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

@oniofchaos

This comment has been minimized.

Copy link
Contributor

oniofchaos commented Sep 28, 2018

Never thought of that before but super cool idea. LGTM after a changelog entry! 👍

@willianveiga

This comment has been minimized.

Copy link
Contributor Author

willianveiga commented Oct 2, 2018

Never thought of that before but super cool idea. LGTM after a changelog entry!

Done, @oniofchaos. Thank you very much! 😄

@simi

This comment has been minimized.

Copy link
Contributor

simi commented Oct 3, 2018

I think it will be great to mention this next to rewhere and reorder in ActiveRecord overriding conditions guides.

@rails-bot rails-bot bot added the docs label Oct 5, 2018

@willianveiga

This comment has been minimized.

Copy link
Contributor Author

willianveiga commented Oct 5, 2018

I've made it, @simi. Thank you very much! 😃

@dcangulo

This comment has been minimized.

Copy link

dcangulo commented Oct 5, 2018

When will this get merged?

@oniofchaos

This comment has been minimized.

Copy link
Contributor

oniofchaos commented Oct 11, 2018

Sadly there's a changelog merge conflict 😭

@willianveiga

This comment has been minimized.

Copy link
Contributor Author

willianveiga commented Oct 11, 2018

Sadly there's a changelog merge conflict

Not anymore! 😄

@g1938703

This comment has been minimized.

Copy link

g1938703 commented Oct 17, 2018

What about now, @oniofchaos? When will it be merged?

@oniofchaos

This comment has been minimized.

Copy link
Contributor

oniofchaos commented Oct 17, 2018

@g1938703 as I am not a maintainer I do not have merge capabilities. I am just helping out by providing reviews and commentary on PRs. It's ultimately up to that group, such as the assignee pixeltrix, when and if the PR gets merged.

Also, there's another merge conflict on the changelog :D

@willianveiga

This comment has been minimized.

Copy link
Contributor Author

willianveiga commented Oct 18, 2018

@oniofchaos, nice job helping people out! 😃

@pixeltrix, when will this get merged?

Thank you very much, guys!

@pixeltrix

This comment has been minimized.

Copy link
Member

pixeltrix commented Oct 18, 2018

@willianveiga looking at the implementation for reorder and rewhere it seems to me that an implementation similar to reorder is more appropriate - unscope(:select).select(args) will create an extra relation instance compared to how reorder does it. The rewhere can't do it in the same way because it can redo a subset of conditions whereas reorder doesn't and reselect is the same.

@willianveiga

This comment has been minimized.

Copy link
Contributor Author

willianveiga commented Oct 24, 2018

Is that what you meant, @pixeltrix?

Thank you very much, Wilian.

@ericgulini

This comment has been minimized.

Copy link

ericgulini commented Jan 27, 2019

When will it be merged? Thank you!

@bobbytables

This comment has been minimized.

Copy link
Contributor

bobbytables commented Feb 28, 2019

Is this ever going to be merged?

@sikachu sikachu requested a review from pixeltrix Mar 1, 2019

@pixeltrix pixeltrix merged commit 0c4bf98 into rails:master Mar 1, 2019

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pixeltrix

This comment has been minimized.

Copy link
Member

pixeltrix commented Mar 1, 2019

@willianveiga thanks! 👍

Fixes #27340.

*Willian Gustavo Veiga*

This comment has been minimized.

@bogdanvlviv

bogdanvlviv Mar 1, 2019

Contributor

Would be great to move this changelog entry to the top of the file to point that this method will be available since the next Rails release.

@willianveiga willianveiga deleted the willianveiga:feature/reselect-method branch Mar 1, 2019

bogdanvlviv added a commit to bogdanvlviv/rails that referenced this pull request Mar 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.