nakagami / firebirdsql

Firebird RDBMS sql driver for Go (golang)
MIT License
220 stars 60 forks source link

error "invalid transaction handle" on concurrently prepared queries #174

Closed palevi67 closed 1 month ago

palevi67 commented 1 month ago

When preparing several queries concurrently I get this error on fourth call to stmt.Query() method:

invalid transaction handle (expecting explicit transaction start)

I have prepared a test to reveal this:

func TestConcurrently(t *testing.T) {

    conn, err := sql.Open("firebirdsql", dsn)
    require.NoError(t, err)

    stmt1, err := conn.Prepare("select entero_nn from T2 where entero_nn>=?")
    require.NoError(t, err)

    stmt2, err := conn.Prepare("select texto_nn from T2 where texto_nn=?")
    require.NoError(t, err)

    for k := 0; k < 3; k++ {

        fmt.Println("k:", k)

        rows1, err := stmt1.Query(k)
        require.NoError(t, err)

        rows2, err := stmt2.Query("uno") // Fails here on second pass
        require.NoError(t, err)  

        err = rows1.Close()
        require.NoError(t, err)

        err = rows2.Close()
        require.NoError(t, err)
    }

    err = stmt1.Close()
    require.NoError(t, err)

    err = stmt2.Close()
    require.NoError(t, err)

    err = conn.Close()
    require.NoError(t, err)
}

Dataset is the same of https://github.com/nakagami/firebirdsql/issues/169#issuecomment-2110467837 and I'm using latest code from master branch.

nakagami commented 1 month ago

Thanks, I wrote new pull reqeust. https://github.com/nakagami/firebirdsql/pull/175 Will this patch solve the problem?

palevi67 commented 1 month ago

Though the original error is gone, now I'm getting an "sql: statement is closed" error on some calls to stmt.Query(). For now I can't identify the exact sequence that leads to this error ... when I do, I'll upload a test case.

palevi67 commented 1 month ago

Finally I could create the test for the "sql: statement is closed" error, which happens when a statement prepared in a transaction is used in other transaction. Though this might sound a bit awkward, I have checked this sequence works with other implementation (old BDE).

func TestClosed(t *testing.T) {
    conn, err := sql.Open(`firebirdsql`, dsn)
    require.NoError(t, err)

    // STARTS TX1
    tx, err := conn.Begin()
    require.NoError(t, err)
    stmt, err := conn.Prepare(`select * from T2 where entero_nn = ?`) // Prepare the statement in TX1
    require.NoError(t, err)
    err = tx.Commit()
    require.NoError(t, err)
    // ENDS TX1

    // STARTS TX2
    tx, err = conn.Begin()
    require.NoError(t, err)
    _, err = tx.Stmt(stmt).Query(1) // Use the prepared statement in TX2
    require.NoError(t, err)
    err = tx.Commit()
    require.NoError(t, err)
    // ENDS TX2

    err = conn.Close()
    require.NoError(t, err)
}

Note: I have corrected the test so now it passes ...

palevi67 commented 1 month ago

Another related test, this time I get a "invalid transaction handle (expecting explicit transaction start)" error:

func TestPreparedTX(t *testing.T) {
    conn, err := sql.Open(`firebirdsql`, dsn)
    require.NoError(t, err)

    stmt, err := conn.Prepare(`select * from T2 where entero_nn = ?`) // Prepares statement
    require.NoError(t, err)

    // START TRANSACTION
    tx, err := conn.Begin()
    require.NoError(t, err)
    err = tx.Commit()
    require.NoError(t, err)
    // END TRANSACTION

    rows, err := stmt.Query(1) // <- invalid transaction handle (expecting explicit transaction start)
    require.NoError(t, err)

    rows.Close()
    require.NoError(t, err)
    err = conn.Close()
    require.NoError(t, err)
}
palevi67 commented 1 month ago

Forget about the TestClosed error: I had a misunderstanding on how transactions and prepared statements mix in golang. I have run it against a postgresql v13 and fails with the exact same error. I will correct the previous post so the test passes.

However IMHO the TestPreparedTX test should pass, as it does against the PG database.

nakagami commented 1 month ago

Thanks!

I have made additional modificatons. How about this? https://github.com/nakagami/firebirdsql/pull/175

palevi67 commented 1 month ago

Yes, now TestPreparedTX is passing.

But I have another transaction/prepared statement sequence failing with an EOF error:

func TestEOF2(t *testing.T) {
    conn, err := sql.Open(`firebirdsql`, dsn)
    require.NoError(t, err)

    // START TX1
    tx, err := conn.Begin()
    require.NoError(t, err)

    stmt1, err := conn.Prepare(`SELECT * FROM T2 WHERE entero_nn = ?`)
    require.NoError(t, err)
    rows1, err := tx.Stmt(stmt1).Query(1)
    require.NoError(t, err)
    err = rows1.Close()
    require.NoError(t, err)
    err = stmt1.Close()
    require.NoError(t, err)

    stmt2, err := conn.Prepare(`UPDATE T2 SET TEXTO=NULL WHERE entero_nn = ?`)
    require.NoError(t, err)
    _, err = tx.Stmt(stmt2).Exec(333)
    require.NoError(t, err)

    err = tx.Commit()
    require.NoError(t, err)
    // END TX1

    // START TX2
    tx, err = conn.Begin()
    require.NoError(t, err)
    _, err = tx.Stmt(stmt2).Exec(333) // <- error: "EOF"
    require.NoError(t, err)
    err = tx.Rollback() // <- error: "read tcp 192.168.246.100:49206->192.168.246.100:3050: wsarecv: An established connection was aborted by the software in your host machine."
    require.NoError(t, err)
    // END TX2

    err = conn.Close()
    require.NoError(t, err)
}

Update: translate error message into English.

nakagami commented 1 month ago

Thank you again and again

I guess I should not have repeatedly closed the statement that was closed. How about this?

palevi67 commented 1 month ago

Yes! Now I can't find any issues related to prepared statements! Thanks for your responsiveness!

nakagami commented 1 month ago

I appreciate your report.

Merged into master and set tag v0.9.10