rzane/baby_squeel

Wrong table name when joining a polymorphic table twice

Closed this issue · 2 comments

Thank you for your time and this gem.
I am migrating from sqeel to baby_squeel and I came across a problem with polymorhic table names.

Issue

When I join the same polymorhic table twice in one querry and try to use both joins in the where class the second table name is incorrect.

scope = Food.joining { [animal.of(Dog), animal.of(Cat)] }
.where.has { animal.of(Dog).dog_name.eq('NameOfADog').or(animal.of(Cat).cat_name.eq('NameOfACat')) }
puts scope.to_sql
SELECT "foods".* FROM "foods"
INNER JOIN "dogs" ON "dogs"."id" = "foods"."animal_id" AND "foods"."animal_type" = 'Dog'
INNER JOIN "cats" ON "cats"."id" = "foods"."animal_id" AND "foods"."animal_type" = 'Cat'
WHERE ("dogs"."dog_name" = 'NameOfADog' OR "dogs"."cat_name" = 'NameOfACat')

"dogs"."cat_name" is not correct it should be "cats"."cat_name"

I think the Problem is somewhere in find_join_association in baby_squeel/join_dependency.rb.
But I could not locate it further or fix it.

At the moment I try to use plain Arel for this case Cat.arel_table[:cat_name].eq('NameOfACat') but I think this is wrong / not possible to use when the joined table gets an alias.

Do you have any other suggestions?

Reproduction

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

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

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

ActiveRecord::Schema.define do
  create_table :dogs, force: true do |t|
    t.string :dog_name
  end

  create_table :cats, force: true do |t|
    t.string :cat_name
  end

  create_table :foods, force: true do |t|
    t.string :animal_type
    t.integer :animal_id
  end
end

class Dog < ActiveRecord::Base
end

class Cat < ActiveRecord::Base
end

class Food < ActiveRecord::Base
  belongs_to :animal, polymorphic: true
end

class BabySqueelTest < Minitest::Spec
  it 'double polymorphic joining works for babysqueel' do
    join_scope = Food.joining { [animal.of(Dog), animal.of(Cat)] }
    scope = join_scope.where.has { animal.of(Dog).dog_name.eq('NameOfADog').or(animal.of(Cat).cat_name.eq('NameOfACat')) }
    arel_scope = join_scope.where.has { animal.of(Dog).dog_name.eq('NameOfADog').or(Cat.arel_table[:cat_name].eq('NameOfACat')) }

    scope.to_sql.must_equal(arel_scope.to_sql)
    scope.to_sql.must_equal %{
      SELECT "foods".* FROM "foods"
      INNER JOIN "dogs" ON "dogs"."id" = "foods"."animal_id" AND "foods"."animal_type" = 'Dog'
      INNER JOIN "cats" ON "cats"."id" = "foods"."animal_id" AND "foods"."animal_type" = 'Cat'
      WHERE ("dogs"."dog_name" = 'NameOfADog' OR "cats"."cat_name" = 'NameOfACat')
    }.squish
  end
end

Test result

Run options: --seed 63975

# Running:

DEPRECATED: global use of must_equal from (irb):46. Use _(obj).must_equal instead. This will fail in Minitest 6.
F

Finished in 0.021652s, 46.1851 runs/s, 46.1851 assertions/s.

  1) Failure:
BabySqueelTest#test_0001_double polymorphic joining works for babysqueel [(irb):46]:
--- expected
+++ actual
@@ -1 +1 @@
-"SELECT \"foods\".* FROM \"foods\" INNER JOIN \"dogs\" ON \"dogs\".\"id\" = \"foods\".\"animal_id\" AND \"foods\".\"animal_type\" = 'Dog' INNER JOIN \"cats\" ON \"cats\".\"id\" = \"foods\".\"animal_id\" AND \"foods\".\"animal_type\" = 'Cat' WHERE (\"dogs\".\"dog_name\" = 'NameOfADog' OR \"cats\".\"cat_name\" = 'NameOfACat')"
+"SELECT \"foods\".* FROM \"foods\" INNER JOIN \"dogs\" ON \"dogs\".\"id\" = \"foods\".\"animal_id\" AND \"foods\".\"animal_type\" = 'Dog' INNER JOIN \"cats\" ON \"cats\".\"id\" = \"foods\".\"animal_id\" AND \"foods\".\"animal_type\" = 'Cat' WHERE (\"dogs\".\"dog_name\" = 'NameOfADog' OR \"dogs\".\"cat_name\" = 'NameOfACat')"


1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

The Arel workaround didn't work well so I had to monkey patch baby_squeel.
https://github.com/rzane/baby_squeel/blob/b97fa0e314b50e6cb7855f38a17ff595cce22c79/lib/baby_squeel/join_dependency.rb

I just leave the MP here if someone has similar problems.
I would also provide merge request if any of the gem owners thinks the patch could / should go into the gem.

class BabySqueel::JoinDependency::Builder
  def find_join_association(associations)
    associations.inject(join_dependency.send(:join_root)) do |parent, assoc|
      parent.children.find do |join_association|
        reflections_equal?(
          assoc._reflection,
          join_association.reflection
        ) && klass_equal?(assoc, join_association)
      end
    end
  end

  # If association is not polymorphic return true.
  # If association is polymorphic compare the association polymorphic class with the join association base_klass
  def klass_equal?(assoc, join_association)
    return true unless assoc._reflection.polymorphic?

    assoc._polymorphic_klass == join_association.base_klass
  end
end
rzane commented

Fixed by #108