This is a great point, and in light of that multi seems more reasonable. On the other hand, we can also see how these limitations of DBs spill over to affect the Elixir code. We end up with this more declarative design, and clumsy value threading via the assigns-like map.
In the mentioned issue (trying to do something useful after a failed db operation), I assume that any subsequent attempt to do a db operation, or to commit a transaction, would raise, right?
If I remember correctly it raises with a database specific error (that may not be clear at all). Although my comment was mostly for completeness. It is a caveat one can mention in the docs (and I believe Repo.transaction already does).
FWIW, that article (which I’ve read before and now have read two more times) doesn’t answer my question. That is not a comment on the article, I just don’t see the correlation between it and returning errors tuples from functions that don’t error. This is off topic, though.
Point taken. I got too excited about trying to override the functions in Repo and ended up making it inconsistent. I understand what y’all mean about it causing confusion.
Thanks for catching that! I probably would’ve found out about it eventually, but not without frustration.
It doesn’t say anything about changing the return shape of those functions, but rather adding new functions. Overriding just came to mind since the article was talking about adding functions to the repo module. Usually, I just look at the source or documentation to see how the return value should look, so it wouldn’t confuse me to see that a get returns {:ok, term}. Take a library like Cachex - notice anything about the return type? Not that it’s a justification, but the get vs fetch distinction hasn’t been top of mind for me.
Ok, this is my last off-topic post in the thread. Thanks for all the pointers everyone.
I should preface that anyone is of course allowed to do whatever they want! I’m just genuinely interested if my ideas that I’ve held about return tuples, at least as implemented by Erlang/Elixir and libraries in general, are off or not.
The Cachex example is actually in line with that I’m talking about, though. My point was not about get returning a tuple (the fact that it’s inline with get from the language is something that had never occurred to me until José pointed it out here) it’s that an empty database result is not an “error.” And this is exactly how Cachex operates: it returns {:ok, nil} if the key doesn’t exist. It’s using tuples because there is a third error state when the cache itself doesn’t exist which seems reasonable to me. This isn’t something Ecto would have because we aren’t going to code around a whole table suddenly going missing! That is when it’s time for us humans to panic and freak out
When retrieving exactly one row, a missing row could be considered an error. It’s no different than e.g. fetching from a map (Map.fetch), or the OS env (System.fetch_env).
There already are bang versions, such as get! which raise if the row is missing. Hence, it makes sense to add functions which explicitly return an error if the desired row is missing. I don’t know why Ecto doesn’t provide this out of the box, but it’s easily done by extending the repo module, and I do this in pretty much every project. I’ve also seen other teams doing the same.
Having fetch, fetch_one, and fetch_by play nice with the with expression, and with the general convention to use ok/error to distinguish between found/not found.
You need to double and triple check. IIRC, the semantics of aborted transactions inside savepoints were a bit surprising. When we went from Ecto 1 → Ecto 2, I believe one of the major changes were transaction handling, because we got a couple things wrong.
Is there any place I can read about these surprising semantics? Or could you give a gist of it? AshPostgres is leveraging save points to define atomic validations and changes, and while I did my research and I think we should be fine, maybe there are some gotchas we haven’t detected yet.
Ah yes. I yet again failed to correlate the hypothetical Ecto.fetch with Elixir’s Keyword/Map.fetch. When you put it that way it’s hard to argue with!
I’ve mostly just never come across this. I certainly use Repo.get! when a record must be in the database by design and if it’s not then I have bigger problems (like some config data or something). Otherwise if it’s fetching a record form a “show” page where I’d want to show the user a non-404 message, for example, I just use an if here. I guess I’ve never found myself wanting to fetch a single record being part of a with so is hard for me to imaging. Of course you can always pattern match on the struct, but ya, I see the point now.
They all come from the database, so you would have to try it out. Anyway, the goal of the refactor we did in Ecto was to closely reflect what happens in the database (by effectively making it dumber), so the hope is that whatever you are doing is right by design.
Understood, not semantics of ecto + db savepoints, just semantics of savepoints. The way we use save points is that we have a special error that we raise during an operation, and if one of those errors is raised only, do we restore a save point. And we release the save point immediately afterwards. So I think we should be alright Thanks for the info
Is there any chance that at some point it will support returning :ok from the callback? I’d like to adopt transact instead of the one I rolled out long time ago, but we have a lot of function that do some writes and just return ok.