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.

Sign up for GitHub

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

Jump to bottom

update_all ignores conditions, when :orders and :limit options are supplied #765

Closed
lighthouse-import opened this issue May 16, 2011 · 9 comments
Closed

update_all ignores conditions, when :orders and :limit options are supplied #765

lighthouse-import opened this issue May 16, 2011 · 9 comments

Comments

@lighthouse-import
Copy link

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/6058
Created by Valentine Bichkovsky - 2010-11-24 18:03:39 UTC

Rails version: 3.0.2 and 3.0.3 (no such bug in Rails 3.0.1)
Ruby 1.8.7
Adapter: postgres

In code:

Reservation.update_all("status = 'ready'", 
  ["date = ? and crossing_point_id = ? and position > ?", 
   Time.zone.today, crossing_point.id, last_crossed],
  :order => :position, :limit => params[:count])

In log file:

AREL (15.0ms) UPDATE "reservations" SET status = 'ready' WHERE "reservations"."id" IN (SELECT "reservations"."id" FROM "reservations" ORDER BY position LIMIT 4)

h3. After removing :order and :limit options

In code:

Reservation.update_all("status = 'ready'", 
  ["date = ? and crossing_point_id = ? and position > ?", 
   Time.zone.today, crossing_point.id, last_crossed])

In log file:

AREL (29.7ms)  UPDATE "reservations" SET status = 'ready' WHERE (date = '2010-11-24' and crossing_point_id = 2 and position > 182)
@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Neeraj Singh - 2010-11-24 18:56:36 UTC

looking into it.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Neeraj Singh - 2010-11-24 19:24:39 UTC

Attached is a failing test.

Relation is being built right. Look like issue is with Arel. Studying the code. Patch might take a while :-)

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by rails - 2011-02-25 00:00:06 UTC

This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Howard Yeh - 2011-03-03 10:26:11 UTC

[state:open]

this problem persists for 3.0.5.

Ruby 1.9.2
Adapter: postgres

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Hugo Peixoto - 2011-03-05 23:12:54 UTC

Looks like the bug is in ARel indeed. When a subquery is generated, all other WHERE clauses are dropped.

I attached a patch that fixes this bug and adds a test case.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Hugo Peixoto - 2011-03-06 10:53:38 UTC

The previous patch kept the where conditions on the UPDATE statement. That is not the correct behavior, as the order/limit clauses wouldn't be applied with the WHERE clauses in mind.

This one passes the WHERE clauses to the generated subquery.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Aaron Patterson - 2011-03-21 21:56:22 UTC

@hugo I've applied this patch to ARel master which is the unreleased 2.1.0 version.

Would you mind backporting this to 2.0.x?

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Hugo Peixoto - 2011-03-24 06:41:27 UTC

2.0.x backported patch attached.

@lighthouse-import
Copy link
Author

Attachments saved to Gist: http://gist.github.com/971734

svenfuchs pushed a commit to svenfuchs/rails that referenced this issue Jul 2, 2011
@joeyAghion @jeremy
use supplied primary key when eager-loading belongs_to associations r…
715b34f 
…ather than default primary key

[rails#765]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lighthouse-import