mattn / go-sqlite3

sqlite3 driver for go using database/sql
http://mattn.github.io/go-sqlite3
MIT License
8k stars 1.11k forks source link

SQLiteStmt.execSync should reset the statement on the normal path #912

Open leventov opened 3 years ago

leventov commented 3 years ago

https://github.com/mattn/go-sqlite3/blob/3cbdae750e52afa881060732446298f98131e834/sqlite3.go#L1991-L2008

Should call C.sqlite3_reset(s.s) on the normal return path as well as abnormal. This is because implicit transactions are committed when the statement is reset: https://sqlite.org/lang_transaction.html#implicit_versus_explicit_transactions. If we don't reset the statement, then if the user executes the code like the following:

stmt, _ := conn.PrepareContext(ctx, "INSERT ...")

for ... {
  stmt.Exec()
  time.Sleep(time.Hour)
}

the INSERT statement may remain uncommitted for one hour, which violates the principle of least astonishment.

ljluestc commented 1 year ago
func (s *SQLiteStmt) execSync(args []namedValue) (driver.Result, error) {
    if err := s.bind(args); err != nil {
        C.sqlite3_reset(s.s)
        C.sqlite3_clear_bindings(s.s)
        return nil, err
    }

    var rowid, changes C.longlong
    rv := C._sqlite3_step_row_internal(s.s, &rowid, &changes)
    if rv != C.SQLITE_ROW && rv != C.SQLITE_OK && rv != C.SQLITE_DONE {
        err := s.c.lastError()
        C.sqlite3_reset(s.s)
        C.sqlite3_clear_bindings(s.s)
        return nil, err
    }

    // Add the following line to reset the statement on the normal return path
    C.sqlite3_reset(s.s)

    return &SQLiteResult{id: int64(rowid), changes: int64(changes)}, nil
}

By adding the C.sqlite3_reset(s.s) line on the normal return path, the statement will be reset even when there are no errors. This ensures that implicit transactions are committed and any uncommitted changes are discarded, aligning with the principle of least astonishment.