Search for User by first and/or last name, efficiently

Question

I am refactoring a user search that just feels dirty. (Users#search)

I need to allow a blank param to search on partial or only a first or last, but don't want to return all the records in the database (which is what seems to be happening when I search with one value nil).

That said, what's the best way to handle one blank param / nil value? What's the most succinct, efficient way to do this?

Here is where I started:

    def user_search ## this is my before_action
      if params[:fname] || params[:lname]
        @users = User.where("first_name LIKE ? OR last_name LIKE ?", "%#{params[:fname]}%", "%#{params[:lname]}%")
      elsif params[:email_search]
        @users = User.where("email LIKE ?", "%#{params[:email_search]}%")
      end
      head 404 if @users.blank?
    end

Here is where I am:

    def user_search
      if params[:fname].presence && params[:lname].presence
        @users = User.where("first_name LIKE ? AND last_name LIKE ?", "%#{params[:fname]}%", "%#{params[:lname]}%")
      elsif query = (params[:fname] || params[:lname])
        @users = User.where("first_name LIKE ? OR last_name LIKE ?", "%#{query}%", "%#{query}%")
      elsif params[:email_search]
        @users = User.where("email LIKE ?", "%#{params[:email_search]}%")
      end
      head 404 if @users.blank?
    end

The significance behind the SQL of OR versus AND seems to be a convenient way to query and return matches on one of the names, but is it really efficient? Looking for a first_name in the last_name column sounds kind of silly hackery. Unless I'm looking at this the wrong way.

Is there a more performant way to do this?

Answer 1

These are just some ideas based on some assumptions, so I may be way off.

Looking for a first_name in the last_name column sounds kind of silly hackery. Unless I'm looking at this the wrong way.

The "hacky" feeling is maybe not so much because of the code itself, but elsewhere in the design. It sounds like a "hacky" user-facing UI or client-server API, rather than hacky code, is to blame.

I assume the user interface (if there is one) always shows both a first name and a last name input field of some kind. If that's the case, isn't it valid to only search for the value(s) the user provides?

I certainly wouldn't expect to get results for last name OR first name matches if I only provide a "last name" search string. Similar, I wouldn't expect to find both "Daniel Craig" and "Timothy Dalton" if I type in "Da" in the first name field.

Even if the client has no obvious or direct user input, I'd still say the API should be clear so that an fname param searches for first name, and only first name matches, while lname is last-name-only.

Overloading a user interface or API with non-obvious, special functionality is no different than overloading a method with strange behavior that triggers when pass it certain arguments.

In other words, I'm not sure the OR case makes sense, given input that's so specific and the (assumed to be) equally specific UI.

So, if we can eliminate the OR query (and I admit it's all assumptions here), I'd might do something like:

# get the search parameters, but only
# keep those that that are actually present
search_params = {
  first_name: params[:fname],
  last_name:  params[:lname]
}.keep_if { |_, value| value.present? }

# now do the search
@users = if search_params.any?
  # chaining `where` calls will implicitly add the `AND` in between
  search_params.inject(nil) do |memo, pair|
    column, string = pair
    (memo || User).where("#{column} LIKE ?", "%#{string}%")
  end
elsif params[:email_search].present?
  User.where("email LIKE ?", "%#{params[:email_search]}%")
end

head :not_found if @users.blank?

Haven't tested it, but it should work.

Alternatively, you can leave the "search for name" vs "search for email" decision to the client, and simply say the the API will return for whatever matches all the criteria you send it; if you don't want something matched, don't send it. In that case, the elsif isn't even necessary:

search_params = {
  first_name: params[:fname],
  last_name:  params[:lname],
  email:      params[:email_search]
}.keep_if { |_, value| value.present? }

@users = search_params.inject(nil) do |memo, pair|
  column, string = pair
  (memo || User).where("#{column} LIKE ?", "%#{string}%")
end

head :not_found if @users.blank?

The client can then implement its UI/code however it sees fit, and send whatever params it needs to. Yes, it's just moving the problem, but your API is a lot clearer.

Of course, another solution still would be to index your data in some fashion, so you could just search on any field in one go by just passing a q param or something. But that's another kettle of fish entirely.

Again, just ideas based on assumptions.

Answer 2

Note that user_search as a controller action name implies that your controller isn't RESTfully designed, which is perhaps not the best idea in the world. Also, do you have any control over what the API looks like? fname, lname, and email_search seem like less good parameter names than first_name, last_name, and email. But moving on from that...

This is a lot of logic to put into the controller, so I'd put it in a model method instead (either on the User model, or possibly creating a UserSearch model).

What would that method look like? Well, you're building up a query bit by bit, so this is a great use case for the chaining property of ARel. You can build up a query tree in ARel one condition at a time, and it won't fire the query until you actually request records from the query. Therefore, I'd try something like this:

class User
  def self.search(first_name: nil, last_name: nil, email: nil)
    raise ArgumentError, 'at least one of :first_name, :last_name, and :email is required' if (first_name.nil? && last_name.nil? && email.nil?)
    query = self # the User class, just so we can start the query chain
    query = query.where('first_name like ?', "%#{first_name}%") if first_name.present?
    query = query.where('last_name like ?', "%#{last_name}%") if last_name.present?
    query = query.where('email like ?', "%#{email}%") if email.present?
  end
end

class UsersController < ApplicationController # or whatever the controller is
  def user_search
    @users = User.search first_name: params[:fname], last_name: params[:lname], email: params[:email_search]
    head 404 if @users.empty?
  end
end

Does this make sense? If not, please ask questions and I'll explain further.

Also note that you could DRY up the search method further with some clever metaprogramming.

Answer 3

you can make OR query using AREL, something along the lines:

search_params = { first_name: 'ro', last_name: 'uy'}

search_conditions = search_params.reject {|_,v| v.blank?}.map {|attribute, value| User.arel_table[attribute].matches("#{value}%")}.inject(:or)

User.where(search_conditions).to_sql
#=> SELECT "users".* FROM "users"  WHERE (("users"."first_name" ILIKE 'ro%' OR "users"."last_name" ILIKE 'uy%'))