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

Chained .joins incorrectly re-ordered during query #36558

Closed
packerbacker89 opened this issue Jun 26, 2019 · 4 comments
Closed

Chained .joins incorrectly re-ordered during query #36558

packerbacker89 opened this issue Jun 26, 2019 · 4 comments
Labels

Comments

@packerbacker89
Copy link

packerbacker89 commented Jun 26, 2019

Steps to reproduce

User.joins(:group).joins('AND groups.id = 1').left_joins(:club)

Expected behavior

SELECT "users".* FROM "users" INNER JOIN "groups" ON "groups"."id" = "users"."group_id" AND groups.id = 1 LEFT OUTER JOIN "clubs" ON "clubs"."id" = "users"."clubs_id"

Actual behavior

SELECT "users".* FROM "users" INNER JOIN "groups" ON "groups"."id" = "users"."group_id" LEFT OUTER JOIN "clubs" ON "clubs"."id" = "users"."clubs_id" AND groups.id = 1

The second .joins (joins('AND groups.id = 1')) gets moved to the end of the chain, resulting in more users retrieved than expected. It becomes part of the LEFT OUTER JOIN, meaning that it is not used as a filter.

System configuration

Tested to be an issue on:
Ruby version: 2.6.3
Also tested on Ruby 2.5.1

Rails version: 6.0.0.rc1
Also tested on Rails 5.2.3

More information

Although a separate problem, I noticed issue #34328 and wonder if the issues are related.

@igor04
Copy link
Contributor

igor04 commented Jul 5, 2019

I have tested this case with Rails 5.2.3 and it works as you have expected, but behaviour is changed in 6.0.0.rc1 due to #35864

what about your example it's sounds a little bit incorrect since joins is about INNER JOIN, LEFT OUTER JOIN or other kind of join but not about condition like AND something = something_else, so when you insert joins('string') it is expected to be JOIN query not a part of previous statement, as a result, the fact it was working in the previous version was just a coincidence

what about issue related to order of joins with different argument types it looks like #34328 is more appropriate case

@st0012
Copy link
Contributor

st0012 commented Aug 6, 2019

I agree with @igor04 that your use case don't look correct. And just to put a note here: #34328 is already fixed by #36805. But I can still reproduce this behavior on master. So maybe this and #34328 are not directly related.

@codeodor
Copy link
Contributor

codeodor commented Sep 2, 2019

I get this problem when using :includes and :references now too (on Rails 6, when it was not present in Rails 5.2 correction: my example does seem to have a problem on Rails 5.2.2.1, so I will have to work harder to figure out why my other app simply rearranges the order rather than removing the includes).

Example:

Client.where("1=0").includes(:time_sheet_entries).references(:time_sheet_entries) produces the query

SELECT DISTINCT `clients`.`id` FROM `clients` LEFT OUTER JOIN `time_sheet_entries` ON `time_sheet_entries`.`client_id` = `clients`.`id` WHERE (1=0)

But Client.where("1=0").includes(:time_sheet_entries).references(:time_sheet_entries).joins("left outer join time_sheets on time_sheets.id = time_sheet_entries.time_sheet_id") now produces the query

SELECT `clients`.* FROM `clients` left outer join time_sheets on time_sheets.id = time_sheet_entries.time_sheet_id WHERE (1=0)

It obliterates the left join produced by includes and references.

I have another app where it is a bit more complicated, but seems to be adding the left join for includes/references to the end of the query, even though I added it before calling joins, which means I can no longer reference the columns from those in my additions joins

@rails-bot
Copy link

rails-bot bot commented Dec 1, 2019

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 team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-0-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot added the stale label Dec 1, 2019
@rails-bot rails-bot bot closed this as completed Dec 8, 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

4 participants