Ecto `get!` then `delete!` Anti-pattern?

We cannot do Repo.delete!(queryable, …) is because there is no way for Ecto to guarantee that there will be only 1 deleted entry.

I prefer Repo.delete_all because in many cases it is what user really wants - they do not care whether the row exist at all, if it do not exists, then you already deleted it

If such a function existed, it would have to be very selective about what it supported in queryable - DELETE (at least in Postgres) doesn’t support everything SELECT does.

If there are two simultaneous requests to delete a record (we know users never double-click buttons, but they happen somehow ), one of them is either going to get a 409 (if the delete fails) or a 404 (if the get fails). In either case, the record’s state is consistent, so not sure if it’s problem.

IMO this is a good stepping-stone to the pattern:

user = get_visible_user(id, current_user)
{:ok, _user} = MyRepo.delete(user)

where get_visible_user handles verifying that current_user has access to the specified user.

What if I have code similar to this:

def delete_post(%Post{} = post) do
  if has_comments?(post) do
    {:error, :has_comments}
  else
    Repo.delete(post)
  end
end

defp has_comments?(%Post{} = post) do
  post = Repo.preload(post, :comments, force: true)

  Enum.count(post.comments) > 0
end

Should the code inside the delete_post/1 function be wrapped in Repo.transaction to prevent the situation that there was a comment added between calling the has_comments? and the Repo.delete functions?

(that example is intentionally simplified, but you get the idea… )

Thanks a lot for any input.

The standard way to avoid check-then-act issues like that is to use a foreign key on comments.post_id with the ON DELETE RESTRICT option.

Thanks, that makes sense. But there are situations when you don’t want that rule to be enforced always (by the DB). Would the Repo.transaction help in that case?