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

Queries generated incorrectly in class methods #51852

Closed
danielwaterworth opened this issue May 17, 2024 · 2 comments
Closed

Queries generated incorrectly in class methods #51852

danielwaterworth opened this issue May 17, 2024 · 2 comments

Comments

@danielwaterworth
Copy link

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

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

  gem "rails"
  gem "sqlite3", "~> 1.4"
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 :categories do |t|
    t.integer :parent_category_id
  end
end

class Category < ActiveRecord::Base
  def self.parents
    Category.where(id: select(:parent_category_id))
  end
end

class BugTest < Minitest::Test
  def test_association_stuff
    parent = Category.create!(parent_category_id: nil)
    category = Category.create!(parent_category_id: parent.id)

    assert_equal([parent.id], Category.where(id: Category.where(id: category.id).select(:parent_category_id)).pluck(:id))
    assert_equal([parent.id], Category.where(id: category.id).parents.pluck(:id))
  end
end

Expected behavior

These two assertions should be equivalent. The only difference between them is that, in the second case, some of the logic is contained in a class method.

Actual behavior

The second query is generated incorrectly. The part that filters for a specific id is put into the top-level query when it shouldn't have been.

System configuration

Rails version: 7.1.3.3
Ruby version: 3.3.0

@Jay0921
Copy link
Contributor

Jay0921 commented May 18, 2024

One of the differences between the first and second assertions is that the second assertion chains the parents method to the Category.where(id: category.id) query. In other words, it calls the parents method on the result of the Category.where(id: category.id) query. Since the parent record is not included in the result of the Category.where(id: category.id) query, calling the parents method on this result will not return the parent record.

To prove this, you can call Category.parents.pluck(:id), which should pass the test. In this case, all Category records will be available for the parent chain method.

assert_equal([parent.id], Category.parents.pluck(:id))

Trivial: Instead of using a class method, you can use a scope for the same purpose.

@vipulnsward
Copy link
Member

Hi @danielwaterworth , thanks for the report. The two queries are not the same. If you were to expand the self.parents method on the base scope in the second query, it would look something like this:

Category.where(id: category.id).parents.pluck(:id) #expands to

Category.where(id: category.id).where(id: Category.where(id: category.id).select(:parent_category_id)).pluck(:id))

Your base base scope still will have .where(id: category.id) on it.

The first one is this:

Category.where(id: Category.where(id: category.id).select(:parent_category_id)).pluck(:id))

hence the difference.

Let me know if you still some issue with this. Thanks!

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