Closing prepared statement after error in nested transaction prevents transaction rollback in v0.1.6

[ahobson]

First of all, thank you for go-txdb! It's been a huge help for us testing our project.

v0.1.6 introduced some new behavior that I believe to be a bug. The following test passes on v0.1.5 but not v0.1.6. When using postgresql, it fails. It seems closing a prepared statement automatically rolls back the nested transaction?

I took a look at the changes in v0.1.6 and wasn't able to untangle the behavior, but I was hoping this test case might help someone else figure it out.

Please let me know if I can provide any more information.

func TestShouldAllowClosingPreparedStatementAndRollback(t *testing.T) {
	for _, driver := range drivers() {
		db, err := sql.Open(driver, "rollback")
		if err != nil {
			t.Fatalf(driver+": failed to open a connection, have you run 'make test'? err: %s", err)
		}
		defer db.Close()

		// do a query prior to starting a nested transaction to
		// reproduce the error
		var count int
		err = db.QueryRow("SELECT COUNT(id) FROM users").Scan(&count)
		if err != nil {
			t.Fatalf(driver+": prepared statement count err %v", err)
		}
		if count != 3 {
			t.Logf("Count not 3: %d", count)
			t.FailNow()
		}

		// start a nested transaction
		tx, err := db.Begin()
		if err != nil {
			t.Fatalf(driver+": failed to start transaction: %s", err)
		}
		// need a prepared statement to reproduce the error
		insertSQL := "INSERT INTO users (username, email) VALUES(?, ?)"
		if strings.Index(driver, "psql_") == 0 {
			insertSQL = "INSERT INTO users (username, email) VALUES($1, $2)"
		}
		stmt, err := tx.Prepare(insertSQL)
		if err != nil {
			t.Fatalf(driver+": failed to prepare named statement: %s", err)
		}

		// try to insert already existing username/email
		_, err = stmt.Exec("gopher", "gopher@go.com")
		if err == nil {
			t.Fatalf(driver + ": double insert?")
		}
		// The insert failed, so we need to close the prepared statement
		err = stmt.Close()
		if err != nil {
			t.Fatalf(driver+": error closing prepared statement: %s", err)
		}
		// rollback the transaction now that it has failed
		err = tx.Rollback()
		if err != nil {
			t.Logf(driver+": failed rollback of failed transaction: %s", err)
			t.FailNow()
		}
	}
}

[arthurspa]

The problem seems to be that when we open a Tx, go will reuse an existing connection (https://github.com/golang/go/blob/release-branch.go1.20/src/database/sql/sql.go#L1302) and never release it, since there was no commit nor rollback in this scenario.

Then, when we call Close() method, db.freeConn will be empty here (https://github.com/golang/go/blob/release-branch.go1.20/src/database/sql/sql.go#L887) and it will not call dc.closeDBLocked()(https://github.com/golang/go/blob/release-branch.go1.20/src/database/sql/sql.go#L888). Therefore go-txdb Close() method will never be called.

I'm not sure how to solve this problem though.

Another way to reproduce it:

1. open a DB connection
2. interact with any table, insert or select, etc
3. open a transaction
4. close the connection

After step 4, the transaction will still be alive.

[mdestagnol]

Same issue here with v0.1.6, we have to rollback to v0.1.5

[l3pp4rd]

Hi, not using go for over few years now, would be really helpful if someone could resolve the issue and raise a pull request with a test to prevent regression.

[Yiling-J]

@stefafafan Any luck on this issue? If you are too busy I can also take a look

[stefafafan]

@Yiling-J I took a look at this just a little, but got a bit busy with other stuff. It will help if you would be able to take a look at it too, thanks!

[Yiling-J]

@ahobson I've merge PR to the main branch that I think might fix the bug. Could you please run your tests against it and let me know if you still see the issue?

[ahobson]

@Yiling-J Thank you! I tried it with our application and the tests that used to fail with v0.1.6 pass with v0.1.7-0.20230731141835-240c3c2c17cd. This is great!

[ahobson]

@Yiling-J If you don't mind, let me know when you get a chance to make a release with this fix. Thanks again!

[Yiling-J]

@ahobson v0.1.7 released