Fix override existing join types in the query in the where.associated method

[saleh-alhaddad]

Motivation / Background

Currently, the where.associated method in Rails only allows filtering associated records with an inner join, which can potentially override existing join types in the query. The fix ensures that the association is joined using the appropriate join type (either inner join or left outer join) based on the existing joins in the scope. This prevents unintentional overrides of existing join types and ensures consistency in the generated SQL queries.

Example:

 User.left_outer_joins(:orders).where.associated(:orders).count

Old query

SELECT COUNT(*) FROM "users" INNER JOIN "orders" ON "orders"."user_id" = "users"."id" WHERE "orders"."id" IS NOT NULL

after fixing the issue, the query:
New Query

SELECT COUNT(*) FROM "users" LEFT OUTER JOIN "orders" ON "orders"."user_id" = "users"."id" WHERE "orders"."id" IS NOT NULL

This enhancement empowers developers with greater flexibility in querying associated data, facilitating scenarios such as:

• Retrieving posts with authors, even if some posts lack an assigned author (utilizing left_joins).
• Obtaining comments associated with specific users, encompassing orphaned comments without a user (employing multiple joins with varied types).

Detail

The pivotal improvement in this Pull Request is the inclusion of a check ensuring that associations are joined using the appropriate join type (either inner join or left outer join) based on existing joins within the scope. This safeguards against unintentional overrides of existing join types and fosters consistency in the generated SQL queries.

Additional Information

This refinement is informed by common requirements articulated by Rails developers in various forums and discussions. Analogous functionality is prevalent in certain popular ORM extensions, underscoring its utility. The implementation is crafted to preserve clarity and coherence within the existing codebase.

Note: This is a draft description and should be adapted to your specific implementation details and the project's context.

FYI: After merging this PR, I will check the missing method and fix this issue in a separate PR alternative same PR.

Performance:

• Benchmark Before the fix:

Benchmark.ips do |x| 
  x.report('joins') { User.joins(:comments, :orders).where.associated(:orders) }
  x.report('left_joins') { User.left_joins(:comments, :orders).where.associated(:orders) }
  x.report('left_outer_joins') { User.left_outer_joins(:comments, :orders).where.associated(:orders) }
  x.report('with out joins') { User.where.associated(:orders) }
end

Warming up --------------------------------------
  joins                        3.260k i/100ms
  left_joins                3.261k i/100ms
  left_outer_joins     3.234k i/100ms
  with out joins         3.518k i/100ms
Calculating -------------------------------------
  joins                         32.610k (± 0.7%) i/s -    166.260k in   5.098730s
  left_joins                 32.240k (± 1.1%) i/s -    163.050k in   5.058117s
  left_outer_joins      32.395k (± 0.5%) i/s -    164.934k in   5.091447s
  with out joins          35.337k (± 0.6%) i/s -    179.418k in   5.077489s

• Benchmark After the fix:

Benchmark.ips do |x| 
  x.report('joins') { User.joins(:comments, :orders).where.associated(:orders) }
  x.report('left_joins') { User.left_joins(:comments, :orders).where.associated(:orders) }
  x.report('left_outer_joins') { User.left_outer_joins(:comments, :orders).where.associated(:orders) }
  x.report('with out joins') { User.where.associated(:orders) }
end

Warming up --------------------------------------
  joins                          3.278k i/100ms
  left_joins                  3.264k i/100ms
  left_outer_joins       3.245k i/100ms
  with out joins           3.434k i/100ms
Calculating -------------------------------------
  joins                           31.688k (± 3.8%) i/s -    160.622k in   5.076856s
  left_joins                   32.432k (± 1.1%) i/s -    163.200k in   5.032760s
  left_outer_joins        32.398k (± 1.4%) i/s -    162.250k in   5.009054s
  with out joins            34.698k (± 1.3%) i/s -    175.134k in   5.048274s

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[p8]

Thanks for the PR @saleh-alhaddad.
Could you explain the use-case this is trying to solve?
Don't #associated and #missing already cover all cases?

class Post < ActiveRecord::Base
  belongs_to :author
end

class Author < ActiveRecord::Base
  has_many :posts
end

author1 = Author.create! name: 'Nora'
post1 = Post.create! title: "First post", author: author1
post2 = Post.create! title: "Second post"
assert_equal Post.where.associated(:author).to_a, [post1]
assert_equal Post.where.missing(:author).to_a, [post2]

[p8]

@saleh-alhaddad It's clear you want to allow changing the join. It's not clear what would be the benefit.

For example, your added test has the same results as not passing the join_type.

rails/activerecord/test/cases/relation/where_chain_test.rb

Lines 24 to 29 in 9e01d93

	def test_associated_with_child_association
	Comment.where.associated(:children).tap do |relation|
	assert_includes relation, comments(:greetings)
	assert_not_includes relation, comments(:more_greetings)
	end
	end

If this case is already supported, why do you need to specify the join_type?
The following would all return the same results:

Post.associated(:author)
Post.joins(:author)
Post.left_joins(:author).where.not(author_id: nil)

[saleh-alhaddad]

@p8 I updated the PR to fix an issue in the method, please read again the description and reply if something is wrong, thanks for you

[p8]

@saleh-alhaddad please don’t ping commiters. Most are volunteers and contribute in their spare time. Hopefully someone will find the time to look at this PR.

[rafaelfranca]

Suggested change

	The fix ensures that the association is joined using the appropriate join type (either inner join or left outer join) based on the existing joins in the scope.
	The fix ensures that the association is joined using the appropriate join type (either inner join or left outer join) based on the existing joins in the scope.

[rafaelfranca]

Suggested change

	This prevents unintentional overrides of existing join types and ensures consistency in the generated SQL queries.
	This prevents unintentional overrides of existing join types and ensures consistency in the generated SQL queries.

[rafaelfranca]

Suggested change

	Example:
	Example:

[rafaelfranca]

Suggested change

	asoiciated will use left_joins alternative use deafult joins
	```ruby
	Post.left_joins(:author).where.associated(:author)
	```
	```ruby
	# associated will use LEFT JOIN for this query instead of using JOIN
	Post.left_joins(:author).where.associated(:author)
	```

[rafaelfranca]

Suggested change

	# Additionally, you can define joine type in the scope and associated will not use default that is joins
	# Additionally, you can define join type in the scope and +associated+ will not use default that it is `JOIN`.

[rafaelfranca]

We also need to change the Additionally in the topic above, probably removing it.

[rafaelfranca]

Suggested change

	# Example: Post.left_joins(:author).where.associated(:author)
	# Post.left_joins(:author).where.associated(:author)
	# # Put the SQL here

[saleh-alhaddad]

@rafaelfranca thanks updated

[majedbojan]

👏 🚀