nakagami / firebirdsql

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

Got a "driver: bad connection" error on db.conn.Close() due to previos EOF error on stmt.Query() #169

Closed palevi67 closed 1 month ago

palevi67 commented 2 months ago

Is this a normal behaviour?

palevi67 commented 2 months ago

After further investigation, this seems related to an EOF error obtained on second execution of prepared stmt.Query()

func TestEOF(t *testing.T) {
    conn, err := sql.Open("firebirdsql", "sysdba:secreto@localhost:3050/users/pablo/tmp/hbde.fdb5")
    checkErr(t, err)
    stmt, err := conn.Prepare("select * from T2 where entero_nn>?")
    checkErr(t, err)

    for k := 1; k < 5; k++ {
        fmt.Println("k:", k)
        rows, err := stmt.Query(k) // <- this returns an EOF error the second time is executed ...
        checkErr(t, err)
        rows.Next()
        err = rows.Close()
        checkErr(t, err)
    }

    err = stmt.Close()
    checkErr(t, err)
    err = conn.Close()
    checkErr(t, err)
}

func checkErr(t *testing.T, err error) {
    if err != nil {
        t.Fatal(err)
    }
}

BTW: I'm connecting to the latest FB5 ...

palevi67 commented 2 months ago

tested on FB 2.5.9 ... same EOF error :-(

nakagami commented 2 months ago

The first time stmt.Query() is executed, the driver processing is performed inside sql.Open() and conn.Prepare() for the first time.

Are there wrong connection parameters or SQL statements?

palevi67 commented 2 months ago

I think there is no error on parameters or statements, I have updated the test and I obtain this output:

k: 1
k: 2
--- FAIL: TestEOF (0.15s)
    hbde_test.go:222: EOF
FAIL

Here is the table DDL and DML, in case you want to run the test yourself:

CREATE TABLE T2 (
    ENTERO_NN INTEGER NOT NULL,
    ENTERO INTEGER,
    TEXTO_NN VARCHAR(30) NOT NULL,
    TEXTO VARCHAR(3000),
    FECHA_NN DATE NOT NULL,
    FECHA DATE,
    HORA_NN TIME NOT NULL,
    HORA TIME,
    MOMENTO_NN TIMESTAMP NOT NULL,
    MOMENTO TIMESTAMP,
    MEMO BLOB SUB_TYPE TEXT,
    BINARIO BLOB SUB_TYPE BINARY,
    SIMPLE_NN FLOAT NOT NULL,
    SIMPLE FLOAT,
    DOBLE_NN DOUBLE PRECISION NOT NULL,
    DOBLE DOUBLE PRECISION,
    LETRAS_NN CHAR(30) NOT NULL,
    LETRAS CHAR(30),
    CONSTRAINT PK_T2 PRIMARY KEY (ENTERO_NN)
);

INSERT INTO T2
(ENTERO_NN, ENTERO, TEXTO_NN, TEXTO, FECHA_NN, FECHA, HORA_NN, HORA, MOMENTO_NN, MOMENTO, MEMO, BINARIO, SIMPLE_NN, SIMPLE, DOBLE_NN, DOBLE, LETRAS_NN, LETRAS)
VALUES(1, 1, 'uno', 'uno', '2024-06-04', '2024-06-04', '12:50:00', '12:50:00', '2024-06-04 12:50:00.000', '2024-06-04 12:50:00.000', 'memo', NULL, 1234.0, 1234.0, 12345678, 12345678, 'HOLA                          ', 'ESCAROLA                      ');
INSERT INTO T2
(ENTERO_NN, ENTERO, TEXTO_NN, TEXTO, FECHA_NN, FECHA, HORA_NN, HORA, MOMENTO_NN, MOMENTO, MEMO, BINARIO, SIMPLE_NN, SIMPLE, DOBLE_NN, DOBLE, LETRAS_NN, LETRAS)
VALUES(2, NULL, 'dos', NULL, '2024-06-04', NULL, '12:50:00', NULL, '2024-06-04 12:50:00.000', NULL, NULL, NULL, 1234.0, NULL, 12345678, NULL, 'HOLA                          ', NULL);
palevi67 commented 2 months ago

I have isolated the firebird testing in a sigle file so you can run them easily ...

firebird_test.zip

nakagami commented 2 months ago

Thank you. I could confirm the same result.

For example, the following modification is not able to gracefull clean up the error and transaction is roll backed automaically.

diff --git a/statement.go b/statement.go
index 16b7932..97bd036 100644
--- a/statement.go
+++ b/statement.go
@@ -152,7 +152,7 @@ func (stmt *firebirdsqlStmt) query(ctx context.Context, args []driver.Value) (dr
                _, _, _, err = stmt.wp.opResponse()
                done <- struct{}{}

-               if err != nil {
+               if err != nil && err.Error() != "EOF"{
                        return nil, err
                }

So I cannot find a good solution. I assume that this behaviour is a normal behaviour.

nakagami commented 1 month ago

When I have time, I will find out if there is a solution.

palevi67 commented 1 month ago

I have just found a workaround/clue for this error: you can do a second call to stmt.Query() after rows.Close() just to clear the error ... here is the modified (passing) test:

func TestEOF(t *testing.T) {

    expectedNumberOfRows := []int{2, 2, 1, 0, 0}

    conn, err := sql.Open("firebirdsql", "sysdba:secreto@localhost:3050/users/pablo/tmp/hbde.fdb5")
    require.NoError(t, err)
    stmt, err := conn.Prepare("select entero_nn from T2 where entero_nn>=?")
    require.NoError(t, err)

    for k := 0; k < 5; k++ {
        rows, err := stmt.Query(k) // <-- this returns an EOF error the second time is executed ...
        require.NoError(t, err)
        n := 0
        for rows.Next() {
            err = rows.Err()
            require.NoError(t, err)
            n++
        }
        require.Equal(t, expectedNumberOfRows[k], n)
        err = rows.Close()
        require.NoError(t, err)
        stmt.Query() // <-- workaround: this is just to clear the EOF error
    }

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

thanks

nakagami commented 1 month ago

Internally, the modification was such that the statement was reworked.

https://github.com/nakagami/firebirdsql/pull/173

This is unfortunate, because it would make the use of Prepare() meaningless, but I prioritized fixing the problem without breaking the current behavior.

If anyone has a fix that uses transactions and statements more efficiently, I'd be glad to hear it.