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.

This project is archived and is in readonly mode.

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

#3225

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}"
...

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

State: stale
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from active_record, count, group, group_field, join, table_name to active_record, bugmash, count, group, group_field, join, table_name

    September 25th, 2009 @ 12:02 PM

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

    October 6th, 2009 @ 11:17 PM

  3. Christopher Hlubek
    Christopher Hlubek

    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.

    October 9th, 2009 @ 12:14 PM

  4. dira
    dira

    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.

    October 12th, 2009 @ 12:56 PM

  5. Christopher Hlubek
    Christopher Hlubek

    +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

    October 12th, 2009 @ 01:18 PM

  6. dira
    dira

    Cool, thanks for the link!

    October 12th, 2009 @ 02:10 PM

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

    February 12th, 2010 @ 12:46 PM

  8. Santiago Pastorino
    Santiago Pastorino
    • 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.

    February 2nd, 2011 @ 04:55 PM

  9. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 2nd, 2011 @ 04:55 PM