Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

There was a problem

You must be a member of this account.

This project is archived and is in readonly mode.

[PATCH] ActiveRecord::Calculations#count doesn't allow table names in column_name argument

#5441

I have the following code that works in 2.3.8

@dishes.count('order_items.id',
              :group => 'dishes.id',
              :joins => {:multi_dishes => { :menu_item => :order_items }})

@dishes is some Dish scope. This code counts how many times each dish appeared in orders.

When I try to run it under 3.0.0.rc2, it fails (@dishes = Dish):

ActiveRecord::StatementInvalid: Mysql::Error: Unknown column 'dishes.order_items.id' in 'field list': SELECT COUNT(dishes.order_items.id)
AS count_order_items_id, dishes.id AS dishes_id FROM dishes
INNER JOIN multi_dishes ON multi_dishes.dish_id = dishes.id INNER JOIN menu_items ON menu_items.id = multi_dishes.menu_item_id
INNER JOIN order_items ON order_items.menu_item_id = menu_items.id GROUP BY dishes.id

The problem is apparent: unneeded 'dishes.' prepended to the column name.

I also tried to specify :select option, but it made no change.

Reported by oleg dashevskii · August 24th, 2010 @ 02:19 PM

State: resolved
Milestone: 3.0.2
Assigned to: Aaron Patterson Aaron Patterson
Importance: Low

Activity

  1. Neeraj Singh
    Neeraj Singh
    • Importance changed from to Low

    Can you post your Dish model with scope information? Also post schema info.

    August 24th, 2010 @ 03:26 PM

  2. oleg dashevskii
    oleg dashevskii

    Well, let's simplify the example:

    User.count('orders.id', :joins => :orders, :group => 'users.id')
    

    Schema: User has many orders.

    The count() call without :group works:

    User.count('orders.id', :joins => :orders)
    

    August 24th, 2010 @ 04:29 PM

  3. Santiago Pastorino
    Santiago Pastorino
    • Milestone cleared.
    • State changed from new to open
    • Assigned user set to Aaron Patterson

    August 24th, 2010 @ 06:10 PM

  4. oleg dashevskii
    oleg dashevskii

    I made up a pair of tests which prove the bug.

    August 25th, 2010 @ 09:53 AM

  5. Neeraj Singh
    Neeraj Singh

    This would require changes in Arel because currently arel has following code.

    def attribute(attribute)
      "#{quote_table_name(name_for(attribute.original_relation))}.#{quote_column_name(attribute.name)}" +
      (attribute.alias ? " AS #{quote(attribute.alias.to_s)}" : "")
    end
    

    That needs to be changed to something like this

    def attribute(attribute, prefix = true)
      table_name = "#{quote_table_name(name_for(attribute.original_relation))}"
      column_name = "#{quote_column_name(attribute.name)}"
      qualified_name = prefix ? "#{table_name}.#{column_name}" : column_name
      qualified_name + (attribute.alias ? " AS #{quote(attribute.alias.to_s)}" : "")
    end
    

    Method execute_grouped_calculation in calculations.rb is executing

      Arel::Attribute.new(r, column_name).send(operation).as(aggregate_alias).to_sql
    

    That needs to be changed to pass additional parameter to to_sql to tell it not to add prefix. Something like

      Arel::Attribute.new(r, column_name).send(operation).as(aggregate_alias).to_sql(:table_name => false)
    

    August 25th, 2010 @ 03:30 PM

  6. oleg dashevskii
    oleg dashevskii

    Since simple count (not grouped) works, I looked into its implementation:

        def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
          column = if @klass.column_names.include?(column_name.to_s)
            Arel::Attribute.new(@klass.unscoped, column_name)
          else
            Arel::SqlLiteral.new(column_name == :all ? "*" : column_name.to_s)
          end
    
          # Postgresql doesn't like ORDER BY when there are no GROUP BY
          relation = except(:order).select(operation == 'count' ? column.count(distinct) : column.send(operation))
          type_cast_calculated_value(@klass.connection.select_value(relation.to_sql), column_for(column_name), operation)
        end
    

    Could this Arel::SqlLiteral thing help here? I tried to copy this if-statement to execute_grouped_calculation, but got very strange error messages somewhere from Arel.

    August 25th, 2010 @ 05:19 PM

  7. Jeremy Kemper
  8. oleg dashevskii
    oleg dashevskii
    • Title changed from ActiveRecord::Calculations#count doesn't allow table names in column_name argument to [PATCH] ActiveRecord::Calculations#count doesn't allow table names in column_name argument
    • Tag changed from 3.0.0.rc2, :select, calculations to activerecord 3.0, 3.0.0.rc2, :select, calculations, patch

    I fixed the bug with a rather brute-force style. This @klass.send(:relation) thing apparently is a mess.

    Patch is attached.

    August 31st, 2010 @ 06:12 PM

  9. Neeraj Singh
    Neeraj Singh

    Nice work Oleg. Your patch fixes the problem.

    For some reason when I tried to apply your patch it said email is invalid.

    August 31st, 2010 @ 09:14 PM

  10. oleg dashevskii
    oleg dashevskii

    Neeraj, it works for me (git am < file).

    September 1st, 2010 @ 04:40 AM

  11. Repository
  12. Repository
  13. Jeremy Kemper