resolving includes() is severly broken: #count works, #sum is broken

[eiked]

I have a query, aka ActiveRelation.

• applying #count to it works as expect
• applying #sum(:somefield) is broken: it builds a query with left outer joins of the includes

I have a base table (disks) with a lot of other tables attached to it (i.e. tracks)

I now build a query based only on the Disk table:

• query = Disk.where(available:true)
• query.count
• query.sum(:price)

But when I use includes():

• query = Disk.where(available:true).includes(:tracks)
• query.count
  -- sql: select count(*) from disk where(available is true)
• query.sum(:price)
  -- sql: select sum(price) from disk LEFT OUTER JOIN tracks ON ... where(available is true)
  -- which results in sum([disk x tracks] price)

I believe this is a severly wrong behaviour.

I suspect there must be some place in ActiveRecord that tries to optimize fetches in a bad way.
I looks to me, like sometimes the engine tries to optimize includes()
by folding in the includes into a single query by using left outer joins.

This leads to a lot of really unpredictable behaviour thorughout the framework.
(you stumble around that whenever you use includes() .. can't find my previous bug report on that)

I'd like to suggest:

• identify where this "optimization" takes place
• recherche when and why it was invented
• remove it

To my understanding, #includes is only a hint to improve the query strategy to avoid nxm fetches, but it should never alter the semantics of the query.

But currently (as of activerecord 4.2.5) some weird optimization really breaks the semantics of the query in a really bad way.

I'd like to suggest that #includes MUST NOT change the query, but MAY be used to trigger additional queries to prefetch related tables.

---

While not related to this bug, I could think of an even better optimization.
I believe that the includes() should only be fetched on demand.
I'll make up a new issue for this

[maclover7]

Can you please create a standard reproduction script using one of the templates available here?

[aidanharan]

I've added the reproduction script for the issue above in https://gist.github.com/aidanharan/a4003a10d7cdada717b5

The issue is present in at least versions 5.0.0.beta3, 4.2.5.2 and 4.1.14.2.

[al2o3cr]

It may not be the intended use, but I've personally used things like includes(:foos).references(:foos) to deliberately create LEFT OUTER JOIN clauses in a query. If this is going to be changed, it will need to be messaged clearly to avoid subtly breaking existing apps that depend on the current behavior.

[rails-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 5-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.

[rails-bot]

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 5-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.

[hovancik]

Hi,

this is still an issue in Rails 6.

To repro:

# 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.0"
  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 :expenses, force: true do |t|
    t.integer :amount
  end

  create_table :users_expenses, force: true do |t|
    t.integer :expense_id
    t.integer :amount
  end
end

class Expense < ActiveRecord::Base
  has_many :users_expenses, dependent: :destroy
end

class UsersExpense < ActiveRecord::Base
  belongs_to :expense
end

class BugTest < Minitest::Test

  def setup
    expense = Expense.create!(amount: 10)
    expense.users_expenses << UsersExpense.create!(amount: 5)
    expense.users_expenses << UsersExpense.create!(amount: 5)

    another_expense = Expense.create!(amount: 10)
    another_expense.users_expenses << UsersExpense.create!(amount: 5)
    another_expense.users_expenses << UsersExpense.create!(amount: 5)
  end

  def teardown
    Expense.destroy_all
  end

  def test_sum_with_includes
    assert_equal 20, Expense.all.includes(:users_expenses).sum(:amount) # NOK
  end

  def test_sum_without_includes
    assert_equal 20, Expense.all.sum(:amount) # OK
  end

  def test_ampersand_sum_with_includes
    assert_equal 20, Expense.all.includes(:users_expenses).sum(&:amount) # OK
  end

  def test_ampersand_sum_without_includes
    assert_equal 20, Expense.all.sum(&:amount) # OK
  end


end

Finished in 0.209405s, 19.1017 runs/s, 19.1017 assertions/s.

  1) Failure:
BugTest#test_sum_with_includes [active_record_gem.rb:59]:
Expected: 20
  Actual: 40

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

Should I open new issue?