Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• 

  Neeraj Singh August 24th, 2010 @ 03:26 PM

  • Importance changed from “” to “Low”

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

• You flagged this item as spam.
  

  oleg dashevskii August 24th, 2010 @ 04:29 PM

  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)
  

• 

  Santiago Pastorino August 24th, 2010 @ 06:10 PM

  • Milestone cleared.
  • State changed from “new” to “open”
  • Assigned user set to “Aaron Patterson”

• You flagged this item as spam.
  

  oleg dashevskii August 25th, 2010 @ 09:53 AM

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

  • 0001-tests-proving-5441.patch 1.5 KB
• 

  Neeraj Singh August 25th, 2010 @ 03:30 PM

  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)
  

• You flagged this item as spam.
  

  oleg dashevskii August 25th, 2010 @ 05:19 PM

  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.

• 

  Jeremy Kemper August 30th, 2010 @ 04:10 AM

  • Milestone cleared.

  [bulk edit]

• You flagged this item as spam.
  

  oleg dashevskii August 31st, 2010 @ 06:12 PM

  • 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.

  • 0001-a-brute-force-fix-for-5441.patch 2.5 KB
• 

  Neeraj Singh August 31st, 2010 @ 09:14 PM

  Nice work Oleg. Your patch fixes the problem.

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

• You flagged this item as spam.
  

  oleg dashevskii September 1st, 2010 @ 04:40 AM

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

• 

  Repository September 30th, 2010 @ 06:17 PM

  • State changed from “open” to “resolved”

  (from [fc1bd2bba49a8c3f424b9d6f4c13f430f5db3d5c]) [#5441 state:resolved] refactoring code to determine aggregate column http://github.com/rails/rails/commit/fc1bd2bba49a8c3f424b9d6f4c13f4...

• 

  Repository September 30th, 2010 @ 06:17 PM

  (from [a8a62f87f6777b1bad8f38cc4b0239b74a4f8a7a]) [#5441 state:resolved] refactoring code to determine aggregate column http://github.com/rails/rails/commit/a8a62f87f6777b1bad8f38cc4b0239...

• 

  Jeremy Kemper October 15th, 2010 @ 11:02 PM

  • Milestone set to “3.0.2”

  [bulk edit]