go-sqlite3 icon indicating copy to clipboard operation
go-sqlite3 copied to clipboard

SQLiteStmt.execSync should reset the statement on the normal path

Open leventov opened this issue 4 years ago • 1 comments

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.

leventov avatar Feb 03 '21 10:02 leventov

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.

ljluestc avatar Jul 03 '23 19:07 ljluestc