has_many relation with joins adding redundant inverse inner join

[segiddins]

Steps to reproduce

Have the following set of related models, and use the reverse_dependencies relation. Observe (via the inline comments in the test case below) that there is an unnecessary/redundant join

#!/usr/bin/env ruby
# frozen_string_literal: true

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  
  gem "rails", "~> 7.1.0"
  # If you want to test against edge Rails replace the previous line with this:
# gem "rails", github: "rails/rails", branch: "main"
  
  gem "sqlite3"
  
  gem 'anbt-sql-formatter', require: "anbt-sql-formatter/formatter"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :rubygems, force: true do |t|
  end
  
  create_table :versions, force: true do |t|
    t.belongs_to :rubygem
    t.integer :position
    t.boolean :indexed
  end
  
  create_table :gem_downloads, force: true do |t|
    t.belongs_to :rubygem
    t.belongs_to :version, null: true
    t.integer :count
  end
  
  create_table :dependencies, force: true do |t|
    t.belongs_to :rubygem
    t.belongs_to :version
  end
end

class Rubygem < ActiveRecord::Base
  has_many :incoming_dependencies, lambda {
                                      joins(:version).where(versions: { indexed: true, position: 0 })
                                    }, class_name: "Dependency", inverse_of: :rubygem
  has_many :reverse_dependencies, through: :incoming_dependencies, source: :version_rubygem
  
  has_many :versions
  
  has_many :incoming_dependencies_no_join, lambda {
                                      where(versions: { indexed: true, position: 0 })
                                    }, class_name: "Dependency", inverse_of: :rubygem
  has_many :reverse_dependencies_no_join, through: :incoming_dependencies_no_join, source: :version_rubygem
end

class Dependency < ActiveRecord::Base
  belongs_to :rubygem, optional: true
  belongs_to :version
  has_one :version_rubygem, through: :version, source: :rubygem
end

class Version < ActiveRecord::Base
  belongs_to :rubygem, touch: true
  has_many :dependencies, -> { order("rubygems.name ASC").includes(:rubygem) }, dependent: :destroy, inverse_of: :version
end

class BugTest < Minitest::Test
  def test_assoc_1
    rubygem1 = Rubygem.create!
    v1 = Version.create!(indexed: true, position: 0, rubygem: rubygem1)
    rubygem2 = Rubygem.create!
    v2 = Version.create!(indexed: true, position: 0, rubygem: rubygem2)
    d = v2.dependencies.create!(rubygem: rubygem1)
    
    puts Rails.version
    
    # SELECT
    #   "rubygems" . *
    #   FROM
    #   "rubygems"
    #   INNER JOIN "versions"
    #     ON "rubygems"."id" = "versions"."rubygem_id"
    #   INNER JOIN "dependencies"
    #     ON "versions"."id" = "dependencies"."version_id"
    #   INNER JOIN "versions" "versions_dependencies"
    #     ON "versions_dependencies"."id" = "dependencies"."version_id"
    #   WHERE
    #   "dependencies"."rubygem_id" = ?
    #   AND "versions"."indexed" = ?
    #   AND "versions"."position" = ?
      
    puts mu_pp(rubygem1.reverse_dependencies.arel)
    
    assert_equal [rubygem2], rubygem1.reverse_dependencies
    assert_equal rubygem1.reverse_dependencies_no_join, rubygem1.reverse_dependencies
    
    # Failure:
    # BugTest#test_assoc_1 [Untitled.rb:118]:
    # --- expected
    # +++ actual
    # @@ -109,6 +109,53 @@
    #           #<ActiveRecord::TypeCaster::Map:0xXXXXXX
    #           @klass=
    #           Dependency(id: integer, rubygem_id: integer, version_id: integer)>>,
    # +               name="version_id">>>>,
    # +        #<Arel::Nodes::InnerJoin:0xXXXXXX
    # +         @left=
    # +          #<Arel::Nodes::TableAlias:0xXXXXXX
    # +           @left=
    # +            #<Arel::Table:0xXXXXXX
    # +             @klass=
    # +              Version(id: integer, rubygem_id: integer, position: integer, indexed: boolean),
    # +             @name="versions",
    # +             @table_alias=nil,
    # +             @type_caster=
    # +              #<ActiveRecord::TypeCaster::Map:0xXXXXXX
    # +               @klass=
    # +                Version(id: integer, rubygem_id: integer, position: integer, indexed: boolean)>>,
    # +           @right="versions_dependencies">,
    # +         @right=
    # +          #<Arel::Nodes::On:0xXXXXXX
    # +           @expr=
    # +            #<Arel::Nodes::Equality:0xXXXXXX
    # +             @left=
    # +              #<struct Arel::Attributes::Attribute
    # +               relation=
    # +                #<Arel::Nodes::TableAlias:0xXXXXXX
    # +                 @left=
    # +                  #<Arel::Table:0xXXXXXX
    # +                   @klass=
    # +                    Version(id: integer, rubygem_id: integer, position: integer, indexed: boolean),
    # +                   @name="versions",
    # +                   @table_alias=nil,
    # +                   @type_caster=
    # +                    #<ActiveRecord::TypeCaster::Map:0xXXXXXX
    # +                     @klass=
    # +                      Version(id: integer, rubygem_id: integer, position: integer, indexed: boolean)>>,
    # +                 @right="versions_dependencies">,
    # +               name="id">,
    # +             @right=
    # +              #<struct Arel::Attributes::Attribute
    # +               relation=
    # +                #<Arel::Table:0xXXXXXX
    # +                 @klass=
    # +                  Dependency(id: integer, rubygem_id: integer, version_id: integer),
    # +                 @name="dependencies",
    # +                 @table_alias=nil,
    # +                 @type_caster=
    # +                  #<ActiveRecord::TypeCaster::Map:0xXXXXXX
    # +                   @klass=
    # +                    Dependency(id: integer, rubygem_id: integer, version_id: integer)>>,
    #         name="version_id">>>>]>,
    #   @wheres=
    #     [#<Arel::Nodes::And:0xXXXXXX
    # @@ -213,6 +260,8 @@
    #     ON "rubygems"."id" = "versions"."rubygem_id"
    #   INNER JOIN "dependencies"
    #     ON "versions"."id" = "dependencies"."version_id"
    # +    INNER JOIN "versions" "versions_dependencies"
    # +      ON "versions_dependencies"."id" = "dependencies"."version_id"
    #   WHERE
    #   "dependencies"."rubygem_id" = ?
    #   AND "versions"."indexed" = ?
      
    assert_equal rubygem1.reverse_dependencies_no_join.arel.ast, rubygem1.reverse_dependencies.arel.ast
  end
  

  def test_assoc_2
    rubygem1 = Rubygem.create!
    v1 = Version.create!(indexed: true, position: 0, rubygem: rubygem1)
    rubygem2 = Rubygem.create!
    v2 = Version.create!(indexed: true, position: 0, rubygem: rubygem2)
    d = v2.dependencies.create!(rubygem: rubygem1)
    
    assert_equal [d], rubygem1.incoming_dependencies
    # shows that the joins is necessary
    assert_raises(ActiveRecord::StatementInvalid) { rubygem1.incoming_dependencies_no_join.to_a }
  end
  
  def mu_pp(obj)
    case obj
    when Arel::Nodes::SelectStatement
      rule = AnbtSql::Rule.new
      rule.keyword = AnbtSql::Rule::KEYWORD_UPPER_CASE
      rule.kw_multi_words << "INNER JOIN"
      rule.kw_nl_x << "INNER JOIN"
      rule.indent_string = "  "
      
      ["AST:", obj.pretty_inspect, "SQL:",
        AnbtSql::Formatter.new(rule).format(obj.to_sql)].join("\n\n")
    else
      obj.pretty_inspect
    end
  end
end

Expected behavior

The additional join should not be added, since it's an inner join and the "versions"."id" = "dependencies"."version_id" restriction guarantees it does not add any additional filtering

Actual behavior

There is an unused join to "versions" "versions_dependencies"

System configuration

Rails version: 7.1.3.2

Ruby version: 3.3.0