This project is archived and is in readonly mode.

#3225 ✓stale
Paul Zupan

AR.count - Using symbol in :group with :join fails to return proper column

Reported by Paul Zupan | September 17th, 2009 @ 05:42 PM

I put together an SQL statement in count and found it didn't respond as expected.

Foo.count(:f2_id,

:joins => 'LEFT JOIN (foo AS table1) ON (foo.f1_id = table1.f2_id )' ,
:conditions => 'table1.f1_id = ' + current_user.t_id.to_s,
:group => :f2_id,
:having => 'count(*) > 3',
:order => 'count(*) DESC')

Result:

SELECT count(foo.f2_id) AS count_f2_id, f2_id AS f2_id

FROM `foo`
LEFT JOIN (foo AS table1) ON (foo.f1_id = table1.f2_id )
WHERE (table1.f1_id = 4599) 
GROUP BY f2_id
HAVING count(*) > 5
ORDER BY count(*) DESC

Which fails because the second field name f2_id is ambiguous as it lacks a table definition.

After tracing the parsing of the statement into AR/calculations.rb, I found that it fails to add a table name to group_field if you use only a symbol, even though it properly adds a table name as a prefix to the count column. The easy solution is to pass an explicit group column name when calling count (e.g.; :group => 'foo.f2_id'). However, I'm wondering if this is simply an unexpected oversight when using :group with :join and that the table name should always prefix the column when using :group. Anyone else think it should behave differently?

Here's the method from calculations.rb where group_field is created...

def execute_grouped_calculation(operation, column_name, column, options) #:nodoc:

  group_attr = options[:group].to_s
  association = reflect_on_association(group_attr.to_sym)
  associated = association && association.macro == :belongs_to # only count belongs_to associations
  group_field = associated ? association.primary_key_name : group_attr
  group_alias = column_alias_for(group_field)
  group_column = column_for group_field
  sql = construct_calculation_sql(operation, column_name, options.merge(:group_field => group_field, :group_alias => group_alias))
  calculated_data = connection.select_all(sql)
  aggregate_alias = column_alias_for(operation, column_name)

....

end

Suggested change:

...
group_field = associated ? association.primary_key_name : "#{connection.quote_table_name(table_name)}.#{group_attr}"
...

Comments and changes to this ticket

  • CancelProfileIsBroken

    CancelProfileIsBroken September 25th, 2009 @ 12:02 PM

    • Tag changed from “active_record, count, group, group_field, join, table_name” to “active_record, bugmash, count, group, group_field, join, table_name”
  • dira

    dira October 6th, 2009 @ 11:17 PM

    • Tag changed from “active_record, bugmash, count, group, group_field, join, table_name” to “active_record, bugmash, count, group, group_field, join, patch, table_name”

    I have attached a patch with tests and implementation.

  • Christopher Hlubek

    Christopher Hlubek October 9th, 2009 @ 12:14 PM

    Dira: I tested your patch today and found that the SQL in the new tests doesn't work with PostgreSQL and SQLite. The test did work with MySQL though.

    Another thing I found was the redefinition of the test method "test_should_sum_field" that seems to be useless.

    The errors in PostgreSQL and SQlite were the following:

    PostgreSQL
    1) Error: test_should_calculate_grouped_association_with_self_association_and_count_on_field_name(CalculationsTest):
    ActiveRecord::StatementInvalid: PGError: ERROR: syntax error at or near ")"
    LINE 1: ...ource_id FROM "edges" INNER JOIN (edges AS second) ON (edge...

    {mkd-extraction-2cdae367fc573288e8154f028fda606c}

    : SELECT count("edges".source_id) AS count_source_id, "edges".source_id AS edges_source_id FROM "edges" INNER JOIN (edges AS second) ON (edges.sink_id = second.source_id ) GROUP BY "edges".source_id

    {mkd-extraction-8a16f59ae20e40ff40fa2c1f8adb5d6c}

    2) Error: test_should_calculate_grouped_association_with_self_association_and_count_on_full_field_name(CalculationsTest):
    ActiveRecord::StatementInvalid: PGError: ERROR: syntax error at or near ")"
    LINE 1: ...ource_id FROM "edges" INNER JOIN (edges AS second) ON (edge...

    {mkd-extraction-2cdae367fc573288e8154f028fda606c}

    : SELECT count(second.source_id) AS count_second_source_id, second.source_id AS second_source_id FROM "edges" INNER JOIN (edges AS second) ON (edges.sink_id = second.source_id ) GROUP BY second.source_id

    {mkd-extraction-512a34c6016ffa7918514c390b738fb2}

    SQLite3
    1) Error: test_should_calculate_grouped_association_with_self_association_and_count_on_field_name(CalculationsTest):
    ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: second.source_id: SELECT count("edges".source_id) AS count_source_id, "edges".source_id AS edges_source_id FROM "edges" INNER JOIN (edges AS second) ON (edges.sink_id = second.source_id ) GROUP BY "edges".source_id

    {mkd-extraction-a916f19abb37361a398d56b0954d724a}

    2) Error: test_should_calculate_grouped_association_with_self_association_and_count_on_full_field_name(CalculationsTest):
    ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: second.source_id: SELECT count(second.source_id) AS count_second_source_id, second.source_id AS second_source_id FROM "edges" INNER JOIN (edges AS second) ON (edges.sink_id = second.source_id ) GROUP BY second.source_id

    {mkd-extraction-1f8406fc6a3a5eaadb6c748538d51476}

    So it seems that your INNER JOIN alias is not understood in these DBs (and is a syntax error in PostgreSQL).

    I'm not a PostgreSQL or SQLite3 expert, but it seems you example is very limited to MySQL.

  • dira

    dira October 12th, 2009 @ 12:56 PM

    Hi Christopher,

    thanks for your feedback. Indeed I tested only on MySQL as I did not have the other databases installed :|

    My Postgres installation is still in progress - but I fixed the tests to be compatible with generic SQL syntax and they pass on sqlite.

    Could you please verify the patch again? Thank you.

  • Christopher Hlubek

    Christopher Hlubek October 12th, 2009 @ 01:18 PM

    +1 from me, as it works in MySQL, PostgreSQL and SQLite3 now on Rails master

    BTW, I used the RailsBridge Bugmash VM (http://greg.nokes.name/2009/10/05/railsbridge-bugmash-vm/) that is preconfigured with the databases (except the grants were wrong for MySQL and PostgreSQL) for testing

  • dira

    dira October 12th, 2009 @ 02:10 PM

    Cool, thanks for the link!

  • Rizwan Reza

    Rizwan Reza February 12th, 2010 @ 12:46 PM

    • Tag changed from “active_record, bugmash, count, group, group_field, join, patch, table_name” to “active_record, count, group, group_field, join, patch, table_name”
  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:55 PM

    • State changed from “new” to “open”
    • Importance changed from “” to “”

    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 core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:55 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Tags

Pages