Why does Repo.get and Repo.get_by return the resource or nil instead of an ok or error tuple?

Hi,

Repo.insert, Repo.update and Repo.delete all return {:ok, resource} or {:error, changeset}.

But Repo.get and Repo.get_by return resource or nil.

I also noticed a decent amount of custom code (such as the Programming Phoenix Rumbl example app) prefers to use the tuple style, such as the RumblWeb.Auth.login_by_email_and_pass function returning {:ok, conn} or {:error, _reason, conn}.

That made me very curious. Why doesn’t Repo.get return {:ok, resource} or {:error, :resource_not_found}?

If it’s an API oversight and too late to change, what do you think about adding this custom function to your lib/myapp/repo.ex and always using this instead of get_by?

  def fetch_by(queryable, clauses, opts \\ []) do
    case get_by(queryable, clauses, opts) do
      nil ->
        {:error, :resource_not_found}

      resource ->
        {:ok, resource}
    end
  end

The fetch function (not included here) would do the same thing, except wrap get.

Edit: For clarity, it would be useful to have this because when pattern matching on else when using with it’s a lot more obvious to see what went wrong when you match on something like {:error, :resource_not_found} vs matching on nil.

4 Likes

There is get_by! which will raise Ecto.NoResultsError if no record is found.

https://hexdocs.pm/ecto/Ecto.Repo.html#c:get_by!/3

1 Like

You’re really answering your own question here.

There seems to be a convention of get functions directly returning a value or nil (or some supplied default) while the fetch functions return result tuples:

2 Likes

Yup. I add this to all my apps repo modules usually:

  def fetch(query) do
    case all(query) do
      [] -> {:error, query}
      [obj] -> {:ok, obj}
      _ -> raise "Expected one or no items, got many items #{inspect(query)}"
    end
  end

Super handy when you use with as well.

11 Likes

Why do you think this hasn’t been added to Ecto, along with the variants to lookup 1 record?

Yes, but often times I don’t want to raise. I just want to be able to pattern match on a record existing or not, and using nil inside of a with else can get very weird because you don’t know what function returned nil.

you can use a simple trick to be able to match on multiple nil cases

with {:foo, result} <- {:foo, get_item()} do
  {:ok, result}
else
  {:foo, nil} -> {:error, :not_found}
end
2 Likes

That is really cool, and definitely makes it easier to see what was nil.

But if you’re going to go that far, why not just wrap it up into a custom fetch_by function and auto-set the tuple behind the scenes so you don’t have to do it in every with statement?

Yeah, you can definitely do this as well, I was just showing that it is possible to distinguish different nils or other results.

1 Like

Wait…how does this work? if get_item returns nil you’ll end up getting {:ok, nil} which doesn’t seem like what you want.

Yeah there’s a reason my fetch uses all internally because you can also run into issues if your query selects just one column which can be nil.

value = User |> select([u], u.banned_at) |> Repo.get(id)

If value is nil, was the user never banned? or do they not exist? The fetch function I wrote will return {:ok, nil} for a user that exists but the column is nil, and {:error, query} if they don’t exist at all.

I proposed it to the ecto team a while ago, you can find the discussion here: https://github.com/elixir-ecto/ecto/issues/1225

5 Likes

yeah, seems like I forgot a guard…

with {:foo, result} when not is_nil(result) <- {:foo, get_item()} do
  {:ok, result}
else
  {:foo, nil} -> {:error, :not_found}
end
2 Likes

Ah, I never thought about it from the POV of wanting to return just a specific column. Normally I deal with returning the whole row.

But in your fetch case, you lose the ability to do this right? Repo.fetch(User, 4) or Repo.fetch(User, email: "foo@bar.com"). Basically the convenience of using get and get_by.

1 Like

Ah well I actually have three functions:

  def fetch_by(query, args) do
    query
    |> where(^args)
    |> fetch
  end

  def fetch(query, id) do
    query
    |> where(id: ^id)
    |> fetch
  end

  def fetch(query) do
    case all(query) do
      [] -> {:error, query}
      [obj] -> {:ok, obj}
      _ -> raise "Expected one or no items, got many items #{inspect(query)}"
    end
  end
6 Likes

Excellent, thanks. Is there any reason you’re using the long form syntax in your first 2 functions instead of calling out to Repo.get and Repo.get_by? Is it mainly to leverage the third function’s tuple response so you don’t need to duplicate it?

Also, in the case of an error pattern in fetch, how would you pattern match on a query object? What made you choose that instead of something like {:error, :resource_not_found}?

get and get_by suffer from the column ambiguity issue. Only fetch disambiguates that.

The reason for returning the query is that it lets you disambiguate which thing wasn’t found.

with {:ok, thing1} <- Repo.fetch(thing1_query),
{:ok, thing2} <- Repo.fetch(thing2_query),
{:ok, thing3} <- Repo.fetch(thing3_query) do
  foo(thing1, thing2, thing3)
else
  {:error, query} -> {:error, not_found_msg(query)}
end

This lets you have a generic not_found_msg/1 that can tell you what wasn’t found without any particular labeling on the part of the developer.

And that generic not_found_msg(query) function would look at the query and potentially build up an atom like :#{resource}_not_found where resource is a variable you created by parsing it from query (I’m guessing that info would be available in the query)?

A query that does not return data is not really an error.

The query is :ok but the data is not there (and maybe that’s just what you wanna know)

2 Likes

Right, and this was the objection that was raised by the Ecto team. Perhaps my fetch would be better if it returned {:not_found, query}?

Fundamentally, I still think there is a meaningful need here that isn’t addressed by the get API.

  1. Ambiguity with queries that select a single column
  2. Related to #1, if you start selecting individual columns or sets of columns you have to start pattern matching for whatever that specific column type is.
  3. Hard to use with with. The “not found” value for the get api is nil, which doesn’t tell you what wasn’t found. You have to annotate this yourself.

Obviously none of these are deal breakers, but since you’re passing an ecto query to fetch, it can handle all of this for you.

1 Like

:not_found is an error, nil is a value, a perfectly legit value when dealing with SQL.

For example

select * from users where login='not_existent'

should return :not_found, while

select optional_field_that_could_be_null from users where ...

could return NULL (nil converted to Elixir) or :not_found

now you have to handle 3 cases: :ok, :not_found and nil