Remove ActiveRecord::Relation#& alias for 'merge' ? (or at least only reference and not use in AR)

310 views
Skip to first unread message

Brian Cardarella

unread,
Feb 8, 2011, 3:54:40 PM2/8/11
to Ruby on Rails: Core
Would the core team be open to the removal of the '&' alias for
ActiveRecord::Relation#merge?

The reason is that in Ruby the '&' operator refers to an intersection
of two sets of data, not a merge. (within the context of the Array
class)

Recently Arel added support for SQL Set Operators:

2-0-stable: https://github.com/rails/arel/commit/9e816c406399139d9ca76d2089df7f2d94d4fb8b
master: https://github.com/rails/arel/commit/74caeaad157e79853b9c6804f561d3c70eea2346
and https://github.com/rails/arel/commit/d532b7ee430c5d0c412ab9f1a5e0dd3ebc47f86b

So this allows for a higher level abstraction to use the Array class'
convention:

'|' => Union
'+' => UnionALL
'&' => Intersect
'-' => Except (Minus for Oracle)

to operate on two ActiveRecord::Relation instances.

This is sitting as a pending pull request on MetaWhere:
https://github.com/ernie/meta_where/pull/12

Ernie Miller and I have been going back and forth. Ernie points out
that ActiveRecord is using the alias '&' a few times in the code base
instead of 'merge' and overriding this method in MetaWhere would cause
some major issues.

Thoughts?

Aaron Patterson

unread,
Feb 10, 2011, 1:21:27 PM2/10/11
to rubyonra...@googlegroups.com
On Tue, Feb 08, 2011 at 12:54:40PM -0800, Brian Cardarella wrote:
> Would the core team be open to the removal of the '&' alias for
> ActiveRecord::Relation#merge?

Yes, this seems fine to me. We should add a deprecation warning in
3.0.x, then remove the method in master.

At the very least, file a ticket for me so I don't forget. Patches
would be nice too, but aren't necessary. ;-)

--
Aaron Patterson
http://tenderlovemaking.com/

Ernie Miller

unread,
Feb 10, 2011, 1:27:42 PM2/10/11
to rubyonra...@googlegroups.com
On Feb 10, 2011, at 1:21 PM, Aaron Patterson wrote:

On Tue, Feb 08, 2011 at 12:54:40PM -0800, Brian Cardarella wrote:
Would the core team be open to the removal of the '&' alias for
ActiveRecord::Relation#merge?

Yes, this seems fine to me.  We should add a deprecation warning in
3.0.x, then remove the method in master.

[...]

At the very least, file a ticket for me so I don't forget.  Patches
would be nice too, but aren't necessary.  ;-)


Thanks, Aaron! I'll get a patch for master ready shortly.

-- 
Ernie Miller

Ernie Miller

unread,
Feb 10, 2011, 2:42:29 PM2/10/11
to Ruby on Rails: Core

Brian Cardarella

unread,
Feb 26, 2011, 10:00:00 PM2/26/11
to Ruby on Rails: Core
Sweet! I totally missed out on the follow ups to this thread. Thanks!

- Brian
> Patches for 3-0-stable and master are both athttps://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6...
Reply all
Reply to author
Forward
0 new messages