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

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?

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.

StackExchange Code Review Q#52312, answer score: 3