6 ms·

> I've definitely seen What did they say was the thinking behind it? defer tx.Rollback() would make sense, but defer tx.Commit() is nonsensical, regardless of

> I've definitely seen 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.

TheDong 2y ago
"defer tx.Commit() is nonsensical" 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.
- bryancoxwell 2y ago
  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.
  - randomdata 2y ago
    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. If you are only reading, then sure, who cares?
- randomdata 2y ago
  > 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.
  - owl57 2y ago
    > You always want to rollback 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.
    - deleted 2y ago
      [deleted]
    - randomdata 2y ago
      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.
      - owl57 2y ago
        > The Go database/sql package actually executes ROLLBACK in the SQL engine. No: https://github.com/golang/go/blob/beaf7f3282c2548267d3c894417cc4ecacc5d575/src/database/sql/sql.go#L2319-L2321 https://github.com/golang/go/blob/beaf7f3282c2548267d3c89441...
        randomdata 2y ago
        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.

TheDong 2y ago

"defer tx.Commit() is nonsensical" 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.

bryancoxwell 2y ago
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.
- randomdata 2y ago
  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. If you are only reading, then sure, who cares?
randomdata 2y ago
> 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.
- owl57 2y ago
  > You always want to rollback 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.
  - deleted 2y ago
    [deleted]
  - randomdata 2y ago
    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.
    - owl57 2y ago
      > The Go database/sql package actually executes ROLLBACK in the SQL engine. No: https://github.com/golang/go/blob/beaf7f3282c2548267d3c894417cc4ecacc5d575/src/database/sql/sql.go#L2319-L2321 https://github.com/golang/go/blob/beaf7f3282c2548267d3c89441...
      - randomdata 2y ago
        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.