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

Incorrect references to table aliases in Rails 6.1 #1217

Closed
ozzyaaron opened this issue Mar 29, 2021 · 4 comments
Closed

Incorrect references to table aliases in Rails 6.1 #1217

ozzyaaron opened this issue Mar 29, 2021 · 4 comments

Comments

@ozzyaaron
Copy link

ozzyaaron commented Mar 29, 2021

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 (#1182, #1153, #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
Copy link
Author

FYI I think that this may be to do with this issue in Rails rails/rails#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: rails/rails@10b36e8

@ozzyaaron
Copy link
Author

ozzyaaron commented Jul 13, 2021

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

ActiveRecord has identified rails/rails#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

oneiros added a commit to oneiros/ransack that referenced this issue Oct 18, 2023
This simply mirrors ActiveRecord's "deduplication" of joined
tables, to not use table aliases that are not actually in the
final query's JOIN clause.
@oneiros
Copy link
Contributor

oneiros commented Oct 18, 2023

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

deivid-rodriguez pushed a commit to oneiros/ransack that referenced this issue Oct 20, 2023
This simply mirrors ActiveRecord's "deduplication" of joined
tables, to not use table aliases that are not actually in the
final query's JOIN clause.
@deivid-rodriguez
Copy link
Contributor

Fixed by #1447!

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