1
$ ruby -v
ruby 2.3.3p222 (2016-11-21 revision 56859) [x86_64-linux]
$ rails -v
Rails 4.2.0
$ grep DESCR /etc/lsb-release 
DISTRIB_DESCRIPTION="Ubuntu 16.04.1 LTS"

I'm trying to build and expose a subset of records to my view(s) from my controller like this:

@assigned_roles = @user.user_roles.select{|x| x.is_required }.map{ |ur| ur.role }

if @assigned_roles.blank?
  @optional_roles = Role.all
else
  @optional_roles = Role.where('id NOT IN (?)',@assigned_roles.map{ |r| r.id})
end

It works but I'm looking for something more elegant. the .where NOT IN doesn't work as I expect when @assigned_roles is empty.

I have models for Role and User which have many of eachother through user_roles which bears the t.boolean "is_required", default: true, null: false attribute.

Sorry I'm a total n00b so I don't know what else to include. Much Thanks!

mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • What were you expecting the `'NOT IN'` to do? Also, if `@assigned_roles` is empty, your code shouldn't even be getting to the `'NOT IN'` call -- you should be falling into `Role.all`. Is that not happening? – oolong Feb 01 '17 at 20:40
  • @oolong The problem is that `not in (?)` produces [nonsense](http://stackoverflow.com/a/12946338/479863) when the `?` is a placeholder for an empty array. That's why the `if @assigned_roles.blank?` is needed and the OP would prefer to not have to worry about whether or not `@assigned_roles` is empty. This is more of "how do I make this code less ugly" question than a "why doesn't this work" question. – mu is too short Feb 01 '17 at 22:18

1 Answers1

0

Your problem is that ActiveRecord doesn't know what the string 'id NOT IN (?)' means so its replacement of the ? placeholder is simpleminded. AR doesn't parse the SQL snippet to see how it should handle the placeholder, it just knows that there's a placeholder that it should replace with your empty array so it produces the silly looking SQL:

where id not in (null)

There's some more discussion on this over in Issue when retrieving records with empty array.

However, if you let ActiveRecord generate the SQL, then it can do something sensible because it understands what it is doing rather than blinding slinging strings around. This used to be ugly with a NOT IN but now it is easy because you can say .where.not(...):

Role.where.not(:id => @assigned_roles.map(&:id))

That will produce something like this if there are @assigned_roles:

where id not in (6, 11, 23, 42)

and something like this if there aren't any @assigned_roles:

where 1 = 1

The 1 = 1 is just AR's way of building a WHERE clause that is always true (i.e. it is equivalent to Role.all).

Furthermore, if you can arrange for @assigned_roles to be a relation (i.e. @assigned_roles = SomeModel.where(...).where(...)) then you can get ActiveRecord to use a subquery by saying things like:

Role.where.not(:id => @assigned_roles.select(:role_id))

which will result in SQL like this:

where id not in (
  select role_id
  from some_models
  where ...
)

Keeping the query logic inside the database tends to be faster than having a bunch of chatter between the database and your Ruby code.

Community
  • 1
  • 1
mu is too short
  • 426,620
  • 70
  • 833
  • 800