ransack icon indicating copy to clipboard operation
ransack copied to clipboard

Incorrect references to table aliases in Rails 6.1

Open ozzyaaron opened this issue 4 years ago • 2 comments

I've gotta start this with thanks so much for Ransack, really the only time it ever gets in the way are during Rails upgrades 😄

I've seen other issues like the one I'm reporting (https://github.com/activerecord-hackery/ransack/issues/1182, https://github.com/activerecord-hackery/ransack/issues/1153, https://github.com/activerecord-hackery/ransack/issues/1144) but they're on Rails 6.0 or have been fixed in the Rails 6.0 stable branch that will probably be released as Rails 6.0.4. We are upgrading to Rails 6.1 and am hitting table alias issues as part of that.

  • If I use gem 'rails', github: 'rails/rails', branch: '6-0-stable' then the below example will work.
  • If I use Rails 6.0.3.x then the first 2 assertions work and the last 2 generate a where clause using a table alias that does not exist.
  • If I use Rails 6.1.x then the first 2 assertions work and the last 2 generate a where clause using a table alias that does not exist.
  • If I use gem 'rails', github: 'rails/rails', branch: '6-1-stable' then the first 2 assertions work and the last 2 generate a where clause using a table alias that does not exist.

In general the error raised is

ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: user_applications_activities.name

As the table user_applications_activities does not exist in the gathering part of the query.

The only difference between the first 3 assertions and the last 3 are the ordering of the search models & attributes (user_name_or_user_application_name vs user_application_name_or_user_name).

I added another has relationship (to somethings) and started to query on that and it appears that it doesn't break the SQL as badly if that information helps debugging 🤷

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  # Issue DOES occur with Rails 6.1.x, 6-1-stable & 6.0.3
  gem "activerecord", "~> 6.1.0", require: "active_record"
  # gem "activerecord", github: "rails/rails", branch: "6-1-stable", require: "active_record"
  # gem "activerecord", "~> 6.0.0", require: "active_record"

  # Issue does NOT occur with Rails 6.0-stable (future 6.0.4)
  # gem "activerecord", github: "rails/rails", branch: "6-0-stable", require: "active_record"

  gem "ransack", github: "activerecord-hackery/ransack"
  gem "niceql" # Just makes it nicer to see the SQL
  gem "sqlite3"

  gem "pry"
  gem "byebug"
end

require "minitest/autorun"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table "activities", force: :cascade do |t|
    t.integer "user_application_id"
    t.string "content"
  end

  create_table "user_applications", force: :cascade do |t|
    t.integer "user_id"
    t.integer "something_id"
    t.string "name"
  end

  create_table "users", force: :cascade do |t|
    t.string "name"
  end

  create_table "somethings", force: :cascade do |t|
    t.string "name"
  end
end

class UserApplication < ActiveRecord::Base
  belongs_to :user
  belongs_to :something
end

class Something < ActiveRecord::Base
end

class Activity < ActiveRecord::Base
  belongs_to :user_application
  has_one :user, through: :user_application
  has_one :something, through: :user_application
end

class User < ActiveRecord::Base
  has_many :activities, through: :user_applications
  has_many :user_applications
end

class BugTest < Minitest::Test
  def setup
    user1 = User.create!(name: "First User")
    user2 = User.create!(name: "Last User")

    first_something = Something.create!(name: "First Something")
    user_1_application = UserApplication.create!(name: "First User Application", user: user1, something: first_something)
    user_2_application = UserApplication.create!(name: "Last User Application", user: user2)

    user_1_activity = Activity.create!(user_application: user_1_application, content: "First User Activity")
    user_2_activity = Activity.create!(user_application: user_2_application, content: "Last User Activity")
  end

  def teardown
    User.destroy_all
    UserApplication.destroy_all
    Activity.destroy_all
  end

  def test_join
    # Tests swapping order of the search e.g. user_name_or_user_application_name vs user_application_name_or_user_name
    #
    # If user_application_name is first it works with Rails 6.0.3, 6.0.4 and 6.1
    # If user_name is first it works with Rails 6.0.4 but breaks on Rails 6.0.3 and 6.1
    #
    # I included another has_* relationship to somethings as this seems to generate better SQL
    # and not use an non-existant table alias.
    #
    # e.g. the SQL generated under Rails 6.1 for 
    #
    # Activity.ransack(user_name_or_user_application_name_or_something_name_cont: "First").result.niceql
    #
    # SELECT "activities".*
    # FROM "activities"
    # LEFT OUTER JOIN "user_applications" ON "user_applications"."id" = "activities"."user_application_id"
    # LEFT OUTER JOIN "users" ON "users"."id" = "user_applications"."user_id"
    # LEFT OUTER JOIN "somethings" ON "somethings"."id" = "user_applications"."something_id"
    # WHERE (("users"."name" LIKE '%First%' OR "user_applications_activities"."name" LIKE '%First%') OR "somethings"."name" LIKE '%First%')
    #
    results = Activity.ransack(user_application_name_or_user_name_cont: "something").result
    assert_equal [], results.map(&:content)

    results = Activity.ransack(user_application_name_or_user_name_cont: "First").result
    assert_equal ["First User Activity"], results.map(&:content)

    results = Activity.ransack(user_application_name_or_something_name_cont: "First").result
    assert_equal ["First User Activity"], results.map(&:content)

    results = Activity.ransack(user_name_or_user_application_name_cont: "something").result
    assert_equal [], results.map(&:content)

    results = Activity.ransack(user_name_or_user_application_name_cont: "First").result
    assert_equal ["First User Activity"], results.map(&:content)

    results = Activity.ransack(user_name_or_user_application_name_or_something_name_cont: "First").result
    assert_equal ["First User Activity"], results.map(&:content)
  end
end

ozzyaaron avatar Mar 29 '21 17:03 ozzyaaron

FYI I think that this may be to do with this issue in Rails https://github.com/rails/rails/issues/41498

Which links to this commit where the claim is made that an attempt to de-duplicate JOINS was made in an effort to fix eager loading: https://github.com/rails/rails/commit/10b36e81a357f8d7fa3665630c4d41c057fe59d9

ozzyaaron avatar Jul 12 '21 18:07 ozzyaaron

Here is a patch that might leave eager loading a little broken but does fix all of our joining issues.

ActiveRecord has identified https://github.com/rails/rails/issues/41498 as a bug so hopefully they will fix the issue in Rails. The issue is they incorrectly de-duplicate OUTER JOINS.

module ActiveRecord
  module Associations
    class JoinDependency
      private def make_constraints(parent, child, join_type)
        foreign_table = parent.table
        foreign_klass = parent.base_klass
        child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) do |reflection|
          alias_tracker.aliased_table_for(reflection.klass.arel_table) do
            table_alias_for(reflection, parent, reflection != child.reflection)
          end
        end.concat child.children.flat_map { |c| make_constraints(child, c, join_type) }
      end

      private def table_alias_for(reflection, parent, join)
        name = reflection.alias_candidate(parent.table_name)
        join ? "#{name}_join" : name
      end
    end
  end
end

ozzyaaron avatar Jul 13 '21 14:07 ozzyaaron

I encountered the same issue and came up with a "solution" that keeps the fix for eager loading in place: #1447

oneiros avatar Oct 18 '23 11:10 oneiros

Fixed by #1447!

deivid-rodriguez avatar Oct 23 '23 14:10 deivid-rodriguez