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

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

Comments

Projects
None yet
1 participant
@lighthouse-import

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

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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

looking into it.

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

looking into it.

@lighthouse-import

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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 :-)

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

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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.

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

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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

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

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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.

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

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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.

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

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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?

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

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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

2.0.x backported patch attached.

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

2.0.x backported patch attached.

@lighthouse-import

This comment has been minimized.

Show comment Hide comment
@lighthouse-import

lighthouse-import May 16, 2011

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

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

tomstuart pushed a commit to econsultancy/rails that referenced this issue May 17, 2011

Add primary_key option to belongs_to association
[#765 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

tomstuart pushed a commit to econsultancy/rails that referenced this issue May 17, 2011

Fix honoring :primary_key option when joining or eager loading a belo…
…ngs_to association

[#765 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

svenfuchs pushed a commit to svenfuchs/rails that referenced this issue Jul 2, 2011

use supplied primary key when eager-loading belongs_to associations r…
…ather than default primary key

[#765]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

hisas pushed a commit to hisas/rails that referenced this issue May 9, 2017

Merge pull request #765 from zendesk/grosser/version
Move version designation into lib/mail/version.rb itself rather than using an external VERSION file. Simpler, less code, and nothing depends on the externals VERSION file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment