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

Nested merge-joins query causes NoMethodError with ActiveRecord 6.1.4.4 #119

Closed
yysaki opened this issue Feb 4, 2022 · 3 comments
Closed

Comments

@yysaki
Copy link

yysaki commented Feb 4, 2022

Issue

Like as #117, this problem occurs with ActiveRecord 6.1.4.4 and later, but doesn't occur with ActiveRecord 6.0.4.4 and below.
The following nested merge-joins query causes NoMethodError.

`method_missing': private method `construct_join_dependency' called for #<Post::ActiveRecord_Relation:0x00007f93d9fa2cc8> (NoMethodError)

Reproduction

require 'bundler/inline'
require 'minitest/spec'
require 'minitest/autorun'

gemfile true do
  source 'https://rubygems.org'
  gem 'activerecord', '~> 6.1.4.4' # which Active Record version?
  gem 'sqlite3'
  gem 'baby_squeel'
end

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')

ActiveRecord::Schema.define do
  create_table :authors, force: true do |t|
    t.string :name
  end
  create_table :posts, force: true do |t|
    t.belongs_to :author
    t.string :title
  end
  create_table :comments, force: true do |t|
    t.belongs_to :post
    t.string :body
  end
end

class Author < ActiveRecord::Base
  has_many :posts
end

class Post < ActiveRecord::Base
  belongs_to :author
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post
end

class BabySqueelTest < Minitest::Spec
  it 'works' do
    scope = Author.joins(:posts).merge(Post.joins(:comments).merge(Comment.where(body: 'body')))
    scope.to_sql.must_equal %{
      SELECT "authors".* FROM "authors"
      INNER JOIN "posts" ON "posts"."author_id" = "authors"."id"
      INNER JOIN "comments" ON "comments"."post_id" = "posts"."id"
      WHERE "comments"."body" = 'body'
    }.squish
  end
end
Test result
Run options: --seed 5388

# Running:

Traceback (most recent call last):
        26: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:73:in `block in autorun'
        25: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:146:in `run'
        24: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:169:in `__run'
        23: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:169:in `map'
        22: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:169:in `block in __run'
        21: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:335:in `run'
        20: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:363:in `with_info_handler'
        19: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:376:in `on_signal'
        18: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:336:in `block in run'
        17: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:336:in `each'
        16: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:337:in `block (2 levels) in run'
        15: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:350:in `run_one_method'
        14: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:1042:in `run_one_method'
        13: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:93:in `run'
        12: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:221:in `with_info_handler'
        11: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:376:in `on_signal'
        10: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:94:in `block in run'
         9: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:281:in `time_it'
         8: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:95:in `block (2 levels) in run'
         7: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:195:in `capture_exceptions'
         6: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:98:in `block (3 levels) in run'
         5: from issue.rb:43:in `block in <class:BabySqueelTest>'
         4: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.4/lib/active_record/relation/spawn_methods.rb:35:in `merge'
         3: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.4/lib/active_record/relation/spawn_methods.rb:46:in `merge!'
         2: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.4/lib/active_record/relation/merger.rb:82:in `merge'
         1: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.4/lib/active_record/relation/merger.rb:122:in `merge_joins'
/Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.4/lib/active_record/relation/delegation.rb:110:in `method_missing': private method `construct_join_dependency' called for #<Post::ActiveRecord_Relation:0x00007f93d9fa2cc8> (NoMethodError)
        22: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:73:in `block in autorun'
        21: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:146:in `run'
        20: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:169:in `__run'
        19: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:169:in `map'
        18: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:169:in `block in __run'
        17: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:335:in `run'
        16: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:363:in `with_info_handler'
        15: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:376:in `on_signal'
        14: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:336:in `block in run'
        13: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:336:in `each'
        12: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:337:in `block (2 levels) in run'
        11: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:350:in `run_one_method'
        10: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:1042:in `run_one_method'
         9: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:93:in `run'
         8: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:221:in `with_info_handler'
         7: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:376:in `on_signal'
         6: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:94:in `block in run'
         5: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest.rb:281:in `time_it'
         4: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:95:in `block (2 levels) in run'
         3: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:194:in `capture_exceptions'
         2: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:201:in `rescue in capture_exceptions'
         1: from /Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:205:in `sanitize_exception'
/Users/yysaki/.anyenv/envs/rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/minitest-5.15.0/lib/minitest/test.rb:205:in `dump': instance variable added to NoMethodError instance (RuntimeError)
@rocket-turtle
Copy link
Contributor

Thank you for your great bug report @yysaki.

I'll work on this on monday but I think the fix should be easy. The construct_join_dependency is not private.

@rocket-turtle
Copy link
Contributor

rocket-turtle commented Feb 7, 2022

@rzane this fix was quit small. I didn't pay attention to the method visibility :/

@yysaki Sry for your bad upgrade experiance. I got the baby_squeel tests green but haven't used the version in our application because we are still upgrading to Rails 6.0

Your test is green with:
gem 'baby_squeel', branch: 'ISSUE-119', git: 'https://github.com/rocket-turtle/baby_squeel'

If you find any other bugs please feel free to report them.

@rzane
Copy link
Owner

rzane commented Feb 8, 2022

Fixed in #120. Version 1.4.4 released.

@rzane rzane closed this as completed Feb 8, 2022
graywolf1138 added a commit to TheTalentEnterprise/baby_squeel that referenced this issue Sep 6, 2022
* Find the table

* Use polyamorous from master

* With activerecord-hackery/ransack#1004, all but
one polymorphism test is fixed.

There are still several failures. Basically, any test that does an outer
join is still doing an inner join.

* Explain how this is completely broken

* Add byebug for development/testing

* Have Active Record build tables for their aliases

This may help with rzane#97.

* Fix test for equivalent logic

The new version of Active Record or Polyamorous seems to switch the
order of these two SQL conditions, but the new statement is logically
equivalent to the old one.

* Use `variants` for SQL snapshot change

* Make it possible to get good polymorphic joins

Actually getting polymorphic joins to work correctly will require
either explicitly specifying the master branch of the `ransack`
project (commit c9cc20de9e0f or something with equivalent
functionality) in the project `Gemfile` or the `ransack` project
releasing a new 2.x (> 2.3.2) version with that functionality.

Addresses most of the Active Record 5.2.x (>= 5.2.1) problems in
rzane#97.  The only remaining issues are around inner
joins after left joins.

* Track all associations used to find correct alias

By tracking the associations accumulated within an expression, we
can best even Active Record's own `#where` method's ability to
correctly set conditions on column values via associations.

* Reimplement #find_join_association iteratively

Replacing recursive method calls with plain iteration is almost
certainly more performant.

* Revert unnecessary changes from commit 90e02e2

* Lock down the supported versions of ActiveRecord

* Update rspec, rake, and sqlite3

* Run against Ruby 2.6

* Drop support for Active Record versions that have reached EOL

* Remove old variants

* Remove broken image from the readme

* Install latest bundler

* Explicit require test dependencies

* Remove filewatcher

* Run on GitHub actions

* Require coveralls

* Run as a matrix

* Share the environment variables

* Use the coveralls action

* Not sure what is going on here...

* Run against 5.2.0 and 5.2.5

* Give each job a name

* Remove coverage

* Mark failing tests as pending

* Get 5.2.0 passing again

* Remove coverage

* Bump

* Merge `join_dependency` back into this project, because my dreams did not really come true

* Fix checking for version as string

* Add changes from 1.4.0.beta1 to CHANGELOG.md

* Build pull requests

* remove old code from activerecord < 5.2

* removed BabySqueel::Pluck

* reduce complexety after merging `join_dependency` back into this project

* Fix table alias when joining a polymorphic table twice (rzane#108)

* Update changelog for v1.4.0

* Bump version to v1.4.0

* Tidy up version comparison code

* Bump version to v1.4.1

* add support for activerecord 6.0

* add support for activerecord 6.1

* speed up BabySqueel::ActiveRecord::VersionHelper

* add support for activerecord 7.0

* update .github/workflows/build.yml to test rails 7.0 and ruby 3.0 / 3.1

* Apply version-specific monkey-patches outside of the method definition

* Update changelog for v1.4.2

* v1.4.2

* ISSUE-117: left_joins performs INNER JOIN with ActiveRecord 6.1.4.4

* Update changelog for 1.4.3 release

* Bump to v1.4.3

* ISSUE-119: Nested merge-joins query causes NoMethodError with ActiveRecord 6.1.4.4 rzane#119

* Prepare v1.4.4

* ISSUE-121: Drop support for ActiveRecord older than 6.0

* AR 6.1: fix - FrozenError: can't modify frozen object: []

* Bump v2.0.0

Co-authored-by: Ray Zane <[email protected]>
Co-authored-by: Richard Weeks <[email protected]>
Co-authored-by: Jonas S <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants