using uniq with a block on an ActiveRecord::Relation

[burnt43]

I have an issue with the uniq method.

I have a database with a table as below.

MariaDB [hpbx_development]> select * from asterisk_commands;
+-------+-----------------+---------------------+---------------------+-----------+----------+-------------------+
| id    | command         | created_at          | updated_at          | completed | override | virtual_server_id |
+-------+-----------------+---------------------+---------------------+-----------+----------+-------------------+
| 87914 | dialplan reload | 2015-05-18 14:27:01 | 2015-05-18 14:27:01 |         0 |        0 |                 6 |
| 87916 | dialplan reload | 2015-05-18 15:05:52 | 2015-05-18 15:05:52 |         0 |        0 |                 6 |
+-------+-----------------+---------------------+---------------------+-----------+----------+-------------------+
2 rows in set (0.00 sec)

I had some code that worked as I intended in Rails 3.X, but since upgrading to Rails 4.X I found some odd behavior.

I have a line in my application below:
AsteriskCommand.all.uniq {|x| "#{x.command}:#{x.virtual_server_id}"}

Basically, I consider 2 AsteriskCommands the same if they have the same string for command and same virtual_server_id the other fields don't matter. However, now in Rails 4 it looks like calling .uniq on a Relation does a SQL query behind the scenes

AsteriskCommand.all.uniq {|x| "#{x.command}:#{x.virtual_server_id}"}.to_sql
returns

"SELECT DISTINCT `asterisk_commands`.* FROM `asterisk_commands`"

This query is meaningless because each asterisk_command has a unique id in the table and as such won't actually do anything. I was able to correct this problem by first converting the Relation to an Array. AsteriskCommand.all.to_a.uniq {|x| "#{x.command}:#{x.virtual_server_id}"}.

To summarize, because of the behind the scenes query the block I give uniq is ignored. As you can see from below the difference between using to_a and not using to_a. I think that maybe uniq should behave differently if you give it a block.

irb(main):010:0> AsteriskCommand.all.uniq {|x| "#{x.command}:#{x.virtual_server_id}"}.length
=> 2
irb(main):011:0> AsteriskCommand.all.to_a.uniq {|x| "#{x.command}:#{x.virtual_server_id}"}.length
=> 1

[rafaelfranca]

Could you create an reproduction script with this template https://github.com/rails/rails/blob/master/guides/bug_report_templates/active_record_master.rb?

[burnt43]

@rafaelfranca, sure I will try to do this

[burnt43]

@rafaelfranca, I wrote my repro program.
https://gist.github.com/burnt43/43efda266d2855044768

It will fail my test on line 51

[rafaelfranca]

cool! Thank you, we will take a look

[hwhelchel]

@rafaelfranca if rails core is interested in this api change I'd like to contribute it, that being:

If uniq is passed a block then it uses ruby Array#uniq semantics.

Thanks!

[rafaelfranca]

@hwhelchel 👍. Please go ahead.

[sb8244]

@rafaelfranca This was marked as deprecated behavior in the relevant PR that @hwhelchel did. It looks like a "will not fix" unless I'm misunderstanding.

I am in the camp that passing a block to uniq or distinct seems like a reasonable use case.