has_many through with scope block, STI and polymorphic associations that worked in 5.0 breaks in 6.0+

[henrik]

We have a has_many …, through: that also involves a scope block, STI and polymorphic associations (bingo!).

Joining on this association worked fine in Rails 5.0.7.2, but broke when updating to 5.1.7. We've checked and it also fails on 5.2.4.3 and 6.0.3.2.

Steps to reproduce

UPDATE: See reproduction script in https://github.com/rails/rails/issues/40109#issuecomment-680799482 instead.

Set up associations similar to

class Item < ApplicationRecord
  has_many :invoice_lines, as: :invoicable

  has_many :item_invoices,
    -> { where("buyer_invoices.type" => ItemInvoice.name).distinct },  # STI stores leaf class.
    through: :invoice_lines,
    source: :invoice,
    source_type: "BuyerInvoice"  # Polymorphic stores base class.

class InvoiceLine < ApplicationRecord
  belongs_to :invoicable, polymorphic: true  # E.g. Item
  belongs_to :invoice, polymorphic: true  # BuyerInvoice or SellerInvoice
end

For context, ItemInvoice is a STI subclass of BuyerInvoice. There is a buyer_invoices table. SellerInvoice/seller_invoices are a completely separate table and inheritance chain.

Now, try this:

Item.joins(:item_invoices).to_a

Expected behavior

It works, returning an array of invoices.

Actual behavior

It raises

ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: missing FROM-clause entry for table "buyer_invoices"

The reason is that it applies the scope block twice. Both when joining invoice_lines and when joining buyer_invoices.

But the condition in the scope block is not valid when joining invoice_lines (there's no buyer_invoices table to reference yet) and so it causes this error.

This is the query (I renamed some things to simplify for this example, believe it or not, so I hope I got it right):

SELECT "items".* FROM "items"
INNER JOIN "invoice_lines"
  ON "invoice_lines"."invoicable_type" = 'Item'
  AND "invoice_lines"."invoice_type" = 'BuyerInvoice'
  AND "invoice_lines"."invoicable_id" = "items"."id"
  AND "invoices"."type" = 'ItemInvoice'  -- This shouldn't be here!
INNER JOIN "invoices"
  ON "invoices"."type" = 'ItemInvoice'
  AND "invoices"."id" = "invoice_lines"."invoice_id"

Note that the "invoices"."type" = 'ItemInvoice' condition is included both when joining invoice lines and when joining invoices. In Rails 5.0, it was only included when joining invoices, and then the query worked fine.

some_item.item_invoices works fine. The issue happens when joining (or eager_loading, or includeing if it chooses to use JOINs).

We've worked around it by joining an association without the condition, and then adding the condition on top: https://stackoverflow.com/a/63506810/6962 But this makes our code uglier and adds duplication.

System configuration

Rails version: 6.0.3.2

Ruby version: 2.6.6

[KapilSachdev]

Can you provide a reproduction script as well.

[pixeltrix]

Is this the same issue as #34613 ?

[henrik]

@pixeltrix It sure looks that way! Thank you. I took the repro script in that issue, updated the AR version, and then updated the tests to check that they exhibit the same behaviour as I've seen.

I kept the model names from that other issue.

I removed the assertions on the return values, so this just reproduces that the issue causes an exception:

# frozen_string_literal: true

require "bundler/inline"

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

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "6.0.3.2" # See e.g. https://github.com/rails/rails/blob/7-0-stable/RAILS_VERSION or equivalent for the branch in question
  gem "sqlite3"
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 :pages, force: true do |t|
  end

  create_table :elements, force: true do |t|
    t.integer :page_id
  end

  create_table :contents, force: true do |t|
    t.integer :element_id
    t.integer :essence_id
    t.string  :essence_type
  end

  create_table :essence_texts, force: true do |t|
    t.boolean :searchable
  end
end

class Page < ActiveRecord::Base
  has_many :elements

  has_many :contents,
           through: :elements

  has_many :essence_texts,
           through: :contents,
           source: :essence,
           source_type: 'EssenceText'

  has_many :searchable_essence_texts,
           -> { where(searchable: true) },
           through: :contents,
           source: :essence,
           source_type: 'EssenceText',
           class_name: 'EssenceText'
end

class Element < ActiveRecord::Base
  belongs_to :page,
             inverse_of: :elements

  has_many :contents
end

class Content < ActiveRecord::Base
  belongs_to :essence,
             polymorphic: true

  belongs_to :element,
             inverse_of: :contents

  has_one :page,
          through: :element
end

class EssenceText < ActiveRecord::Base
  has_one :content,
          as: :essence

  has_one :element,
          through: :content

  has_one :page,
          through: :element
end

class BugTest < Minitest::Test
  def test_association_stuff_that_does_not_raise
    Page.joins(:essence_texts).where("essence_texts.searchable": true).to_a

    page = Page.create!
    page.essence_texts.to_a
  end

  def test_association_stuff_that_raises
    Page.joins(:searchable_essence_texts).to_a
  end
end

UPDATE 2022-11-07: Added a comment about where to find the Gem version.

[pixeltrix]

@henrik thanks for the reproduction script

[rails-bot[bot]]

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 6-0-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[henrik]

Could still reproduce with 6.0.3.3 which seems to be the latest released version, as well as current edge (6.1.0.rc1).

For 6.0.3.3, I just modified the version in the repro script above, ran the file (e.g. ruby myscript.rb) and saw it fail as expected:

  1) Error:
BugTest#test_association_stuff_that_raises:
ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: contents.searchable

For edge, I did this instead of specifying an AR version:

   gem "rails", github: "rails/rails"

Then I added

puts "AR version: #{ActiveRecord.version}"

to the start of the failing test, just to make sure I got the right AR.

Is there a better way to get just the edge AR gem in situations like this, when multiple gems share a repo? (cc @olleolleolle who might know)

[rails-bot[bot]]

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[olleolleolle]

@henrik Does still still fail, on 6.1?

[henrik]

I've now run the repro script again on both 6.1.3.1 and edge Rails.

Can still reproduce the issue in both.

[rails-bot[bot]]

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[rails-bot[bot]]

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[henrik]

I've confirmed the issue still exists on Rails 7.0.2.3 as well as edge Rails.

[henrik]

@kamipo Could you please reopen? I confirmed above the issue still exists.

[olleolleolle]

This is still:

AR version: 7.0.3.1
...
  1) Error:
BugTest#test_association_stuff_that_raises:
ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: contents.searchable

[rails-bot[bot]]

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[henrik]

I've verified this issue still happens on AR 7.0.4. Please reopen.

[rails-bot[bot]]

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[skipkayhil]

https://github.com/rails/rails/issues/40109#issuecomment-680799482 Reproduction still fails on 7.0.4.2 and main

[joshuay03]

Hey @henrik I came across this issue and thought I'd have a look. I've managed to reproduce the scenario in https://github.com/rails/rails/issues/34613 with a failing test, however, having an ambiguous column reference seems to be cause in that case? and can be easily fixed (see https://github.com/rails/rails/pull/48757#discussion_r1266687230).

I might need some help updating the test to reflect your scenario though, which seems a bit different?

[henrik]

Hi @joshuay03. Thanks so much for looking into this! I won't be in a position to look at this for several weeks but I will let you know.

[Alexander-Senko]

Just another steps to reproduce the issue:

require 'bundler/inline'

gemfile true do
  source 'https://rubygems.org/'

  gem 'minitest',     require: 'minitest/autorun'
  gem 'activerecord', require: 'active_record'
  gem 'sqlite3'
end

ActiveRecord::Base.establish_connection adapter: 'sqlite3', database: ':memory:'
ActiveRecord::Base.logger = Logger.new STDOUT

ActiveRecord::Schema.define do
  create_table :users
  create_table :posts

  create_table :relationships do
    _1.belongs_to :source, polymorphic: true
    _1.belongs_to :target, polymorphic: true
  end
end

class User < ActiveRecord::Base
  has_many :relationships, as: :source

  has_many :posts, -> { ordered }, through: :relationships,
      source:      :source,
      source_type: 'Post',
      class_name:  'Post'
end

class Post < ActiveRecord::Base
  scope :ordered, -> { order id: :desc }
end

class Relationship < ActiveRecord::Base
  belongs_to :source, polymorphic: true
  belongs_to :target, polymorphic: true
end

class AssociationsTest < Minitest::Test
  def test_scoped_polymorphic_association
    assert_equal <<~SQL.squish, User.joins(:posts).to_sql
        SELECT "users".* FROM "users" 
            INNER JOIN "relationships" ON "relationships"."source_type" = 'Post' AND "relationships"."source_id" = "users"."id"
            INNER JOIN "posts" ON "posts"."id" = "relationships"."source_id"
    SQL
  end
end

And yes, the issue is still present in edge Rails (7.2 for now).

[Alexander-Senko]

In case one needs that, the quick fix I use is moving the scope to a named one and calling it with try, e.g. has_many :posts, -> { try :ordered } for the above case.

[joshuay03]

@Alexander-Senko I don't think yours is the same issue?

Your association scope needs to be: has_many :posts, -> { merge(Post.ordered) }. I also think there's a typo in your assertion, shouldn't it be assert_equal <<~SQL.squish, User.joins(:posts).to_sql? There's no :related_posts association.

If you make both these changes your test passes.

[Alexander-Senko]

@Alexander-Senko I don't think yours is the same issue?

I'm sure it is.

Your association scope needs to be: has_many :posts, -> { merge(Post.ordered) }.

Or, it could be order id: :desc directly, or try :ordered as I've mentioned above. I know how to hack it, but I'd like the original issue to be fixed.

And the issue is — running the scope in the wrong context. Yes, we could hack our scopes every time it to be runnable from anywhere, but it doesn't fix the original issue.

I also think there's a typo in your assertion, shouldn't it be assert_equal <<~SQL.squish, User.joins(:posts).to_sql? There's no :related_posts association.

Thanks! I've missed it while reducing the code and removing unnecessary stuff. Fixed.

If you make both these changes your test passes.

The goal was to highlight the issue, not to make some artificial code run.

[joshuay03]

And the issue is — running the scope in the wrong context.

Ah yes you're right, I definitely misunderstood the issue after coming back to this after so long, my apologies. It's exactly the same as what I was able replicate in #48757. Thanks for the script!

[joshuay03]

@henrik @Alexander-Senko

I think both your scripts pass against #51507.