What did they say was the thinking behind it? defer tx.Rollback() would make sense, but defer tx.Commit() is nonsensical, regardless of whether or not the error is handled. It seems apparent that the problem there isn't forgetting to check an error, but that someone got their logic all mixed up, confusing rollback with commit.
It's not pure nonsense, it works in the happy path, and it matches the pattern of how people often handle file IO in go.
f, err := os.OpenFile(...)
defer f.Close()
... which is another place most gophers ignore errors incorrectly. Just like the "defer tx.Commit()" example, it's collocating the idea of setup and cleanup together.
Those two patterns are so similar, python handles them in the same way:
with db.begin() as conn: # implicit transaction, gets automatically committed
with open(...) as f: # implicit file open + close pair, automatically closed
You're of course right that go requires more boiler-plate to do the right thing, but the wrong code has no compiler errors and works in the happy path, and fits how people think about the problem in other languages with sane RAII constructs, so of course people will write it.
> it's collocating the idea of setup and cleanup together.
Okay, sure, but Rollback is the cleanup function. You always want to rollback – you only sometimes want to commit. I suppose this confirms that someone got their logic mixed up.
This phrase highlights the confusion. If you learned SQL before Go, you want to rollback only on error.
Every time I write `defer tx.Rollback()`, I cringe and have to remind myself that yes, it's actually ok to call a method called `Rollback` after succesfully writing data.
If you learned SQL before Go, you'd know the pattern works just fine and is arguably a good sanity check.
BEGIN;
INSERT INTO ...
COMMIT;
ROLLBACK;
It is not some kind of Go-ism. The Go database/sql package actually executes ROLLBACK in the SQL engine. Check out the error returned by it.
Perhaps you mean learned SQL in the context of languages that consider a failed rollback an exception? In that case one needs to be careful to not rollback, else be stricken to handling the exception, which programmers seem to hate doing.
Fair enough, although it wouldn't matter if it did. Nothing would change other than some unnecessary computation would occur. SQL supports it just fine. Anyone who learned SQL first would know that quite well.
Fair as well. It's just sending a command which you know will fail, and even relying on the fact it fails, feels wrong, whether it sends a request over network or not.
BTW I checked and it's not an error in Postgres, only a warning. Still not something I would want in my database logs for the happy path.
You know it will fail if you didn't screw up anything else.
But it's a good sanity check/safety measure to call it anyway incase you made a mistake elsewhere. An errant rollback is more likely to be caught in testing than a dangling transaction.
Unrelated but I’ve never really understood what to do with an error that gets returned by Close() aside from logging it. I’ve also never experienced that error being returned and don’t really understand what could even cause that.
A write is not guaranteed to be written if close fails. You almost certainly want to do something with the error state to ensure that the data gets safely stored in that case.
What did they say was the thinking behind it? defer tx.Rollback() would make sense, but defer tx.Commit() is nonsensical, regardless of whether or not the error is handled. It seems apparent that the problem there isn't forgetting to check an error, but that someone got their logic all mixed up, confusing rollback with commit.