marcboeker / go-duckdb

go-duckdb provides a database/sql driver for the DuckDB database engine.
MIT License
648 stars 97 forks source link

Appender creation generates segmentation violation #109

Closed a-agmon closed 1 year ago

a-agmon commented 1 year ago

The following code - which is supposed to replicate the test in the repo, generates a fault. Would be grateful for advise if i'm doing something wrong here. Using GO 1.20.6 on MacOS (M1) ARM.

package data

import (
    "context"
    "database/sql"
    "testing"

    "github.com/marcboeker/go-duckdb"
)

func TestAppender(t *testing.T) {
    c, err := duckdb.NewConnector("", nil)
    if err != nil {
        t.Errorf("Error creating connector: %v", err)
    }
    db := sql.OpenDB(c)
    defer db.Close()
    println("Opened DB: %v", db)
    conn, err := c.Connect(context.Background())
    if err != nil {
        t.Errorf("Error creating connection: %v", err)
    }
    defer conn.Close()
    appender, err := duckdb.NewAppenderFromConn(conn, "", "test")
    if err != nil {
        t.Errorf("Error creating appender: %v", err)
    }
    appender.Close()
}
pened DB: %v 0x1400007d040
--- FAIL: TestAppender (0.01s)
    ddbAppender_test.go:27: Error creating appender: can't create appender
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x30 pc=0x1026578f8]

goroutine 6 [running]:
testing.tRunner.func1.2({0x1039c9280, 0x103aa9fe0})
        /Users/alonagmon/sdk/go1.20.6/src/testing/testing.go:1526 +0x1c8
testing.tRunner.func1()
        /Users/alonagmon/sdk/go1.20.6/src/testing/testing.go:1529 +0x384
panic({0x1039c9280, 0x103aa9fe0})
        /Users/alonagmon/sdk/go1.20.6/src/runtime/panic.go:884 +0x204
github.com/marcboeker/go-duckdb.(*Appender).Close(0x140001324e0?)
        /Users/alonagmon/go/pkg/mod/github.com/marcboeker/go-duckdb@v1.4.1/appender.go:70 +0x18
command-line-arguments.TestAppender(0x140001324e0)
        /Users/alonagmon/MyData/work/golang-projects/data-warp/foundation/data/ddbAppender_test.go:29 +0x1e4
testing.tRunner(0x140001324e0, 0x1039ea600)
        /Users/alonagmon/sdk/go1.20.6/src/testing/testing.go:1576 +0x10c
created by testing.(*T).Run
marcboeker commented 1 year ago

Thanks for bringing this up. The problem is that you are checking for an error in

if err != nil {
    t.Errorf("Error creating appender: %v", err)
}

but then you do not return and instead continue with the execution. Then in the next line

appender.Close()

you're trying to close the appender, but it isn't initialized because of the error that has been returned above.

Error creating appender: can't create appender

So the behaviour is intended. Please cancel the execution when receiving an error.

a-agmon commented 1 year ago

Thanks for the detailed reply @marcboeker . Apologies for not stating the issue correctly, Im trying to understand why there is a failure in creating the appender. I'm trying to figure out why this code would fail under my setup go1.21.0 darwin/arm64

package main

import (
    "context"
    "database/sql"
    "log"

    "github.com/marcboeker/go-duckdb"
)

func main() {
    log.Println("Starting the application...")
    c, err := duckdb.NewConnector("", nil)
    if err != nil {
        log.Fatalf("Error creating connector: %v", err)
    }
    db := sql.OpenDB(c)
    defer db.Close()
    conn, err := c.Connect(context.Background())
    if err != nil {
        log.Fatalf("Error creating connection: %v", err)
    }
    defer conn.Close()
    appender, err := duckdb.NewAppenderFromConn(conn, "", "test")
    if err != nil {
        log.Fatalf("Error creating appender: %v", err)
    }
    appender.Close()
}
marcboeker commented 1 year ago

The appender you are creating requires a table called test. This table does not exist in the example yet. Adding it via

res, err := db.Exec(`CREATE TABLE test (a INTEGER, b INTEGER)`)

fixes the problem.

a-agmon commented 1 year ago

Thanks @marcboeker - a small suggestion https://github.com/marcboeker/go-duckdb/pull/110