Incorrect parameter to blr conversion

[ctengiz]

I think in there is a bug in paramsToBlr function, because with queries with more than 3 or 4 params, params are incorrectly bound. Please see the attached code. The code generates conversion error from string "" although there is no "" (empty string) value.

package main

import (
    "bufio"
    "crypto/rand"
    "encoding/hex"
    "fmt"
    "log"
    "os"
    "path/filepath"

    "database/sql"

    _ "github.com/nakagami/firebirdsql"
)

func readLine(input string) string {
    reader := bufio.NewReader(os.Stdin)
    fmt.Print(input)
    text, _ := reader.ReadString('\n')
    return text
}

func tempFileName(prefix string) string {
    randBytes := make([]byte, 16)
    rand.Read(randBytes)
    return filepath.Join(os.TempDir(), prefix+hex.EncodeToString(randBytes)+".fdb")
}

func handleError(err error) {
    if err != nil {
        log.Fatal(err)
    }
}

func newConnection(createDB bool, tempPath string) *sql.DB {
    var err error
    var conn *sql.DB
    var sqlTxt string

    connectionString := "sysdba:masterkey@localhost:3050" + tempPath

    if createDB {
        conn, err = sql.Open("firebirdsql_createdb", connectionString)
        handleError(err)

        sqlTxt = `
        create table t1 (
        id bigint generated by default as identity primary key,
            code varchar(50) unique,
            upass VARCHAR(64),
            is_active boolean default true not null,
            full_name varchar(200),
            email varchar(200) unique,
            mobile varchar(30),

            ficomp_id bigint,
            ogunit_id bigint,
            dfrole_id bigint,

            grp_id bigint,
            cls_id bigint,
            spe_id bigint)
        `
        _, err = conn.Exec(sqlTxt)
        handleError(err)

        fmt.Println("db created")
    } else {
        conn, err = sql.Open("firebirdsql", connectionString)
        handleError(err)
    }

    return conn
}

func main() {
    var tx *sql.Tx
    var err error
    var conn *sql.DB
    var sqlTxt string

    tempPath := tempFileName("test_params")

    conn = newConnection(true, tempPath)

    tx, err = conn.Begin()
    handleError(err)

    sqlTxt = "insert into t1(code, upass, is_active, full_name, email, mobile, ficomp_id, ogunit_id, dfrole_id, grp_id, cls_id, spe_id) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"
    _, err = tx.Exec(sqlTxt, "test", nil, true, nil, nil, nil, nil, nil, nil, nil, nil, nil)
    handleError(err)
    err = tx.Commit()
    handleError(err)

    conn.Close()
}

Or am I missing something?

TIA

[jacobalberty]

I believe I am running into this too, it seems to be treating the nil's as empty strings, if i do one with just a few parameters however it works fine

[jacobalberty]

Ok I wrote up a little test, it happens after 8 parameters, so the 9th parameter fails. If you add this to driver_test.go you can see it fails reliably.

func TestGoIssue80(t *testing.T) {
    temppath := TempFileName("test_issue80_")
    conn, err := sql.Open("firebirdsql_createdb", "sysdba:masterkey@localhost:3050"+temppath)
    if err != nil {
        t.Fatalf("Error occured at sql.Open()")
    }
    defer conn.Close()

    query := `
        CREATE TABLE foo (
            a INT,
            b INT,
            c INT,
            d INT,
            e INT,
            f INT,
            g INT,
            h INT,
            i INT
        )
    `

    _, err = conn.Exec(query)                                                                                                                                                 if err != nil {
        t.Error(err)
    }

    _, err = conn.Exec(
        "insert into foo(a, b, c, d, e, f, g, h, i) values (?, ?, ? ,?, ?, ?, ?, ?, ?)",                                                                                          nil, nil, nil, nil, nil, nil, nil, nil, nil)

    if err != nil {
        t.Error(err)
    }

}

Some thoughts on the test, this test is hard coded to just what it takes to fail. It might be a good idea to modify the test to allow a dynamic number of parameters to make it easier to test that the fix doesn't just correct it for 9 parameters but for any arbitrarily large number, like 20-30 parameters.

[nakagami]

I merged @jacobalberty 's patch, thanks. Does this issue fixed ? @ctengiz

[jacobalberty]

@nakagami I pulled the new one with my last patch into my real world usage aaand things broke. I found a new test case. I think some improvement on the test case could potentially be done. I am working on a new patch that i'm manually verifying against the binary from the resulting blr. the ultimate issue seems to be big.Int.Mod() changes the object you're calling it from to be the modulus so you're losing everything past the modulus when its called.

func TestGoIssue80(t *testing.T) {
    temppath := TempFileName("test_issue80_")
    conn, err := sql.Open("firebirdsql_createdb", "sysdba:masterkey@localhost:3050"+temppath)
    if err != nil {                                                                                                                                                               t.Fatalf("Error occured at sql.Open()")                                                                                                                               }                                                                                                                                                                         defer conn.Close()
                                                                                                                                                                              query := `                                                                                                                                                                    CREATE TABLE foo (                                                                                                                                                            a VARCHAR(10),
            b VARCHAR(10),
            c BIGINT,                                                                                                                                                                 d INT,                                                                                                                                                                    e INT,
            f INT,
            g INT,
            h INT,
            i INT,
            j INT,                                                                                                                                                                    k INT,                                                                                                                                                                    l INT,                                                                                                                                                                    m INT,                                                                                                                                                                    n INT
        )                                                                                                                                                                     `
                                                                                                                                                                              _, err = conn.Exec(query)                                                                                                                                                 if err != nil {
        t.Error(err)
    }  

    _, err = conn.Exec(
        "insert into foo(a, b, c, d, e, f, g, h, i, j, k, l, m, n) values (?, ?, ? ,?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
        " ", nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)

    if err != nil {
        t.Error(err)
    }
                                                                                                                                                                          }