Suggestion: txdb.Close(name string) to close txdb.txDriver.db

[sfllaw]

I’ve run into a small problem with txdb because it automatically opens a new connection here: https://github.com/DATA-DOG/go-txdb/blob/6ca798a10f4408c9d8c74527ff93da373e521ebf/db.go#L122-L128

However, there is no way to call txdb.txDriver.db.Close(), which would be nice because I am spawning temporary test databases that are dropped after the tests are run. Unfortunately, because PostgreSQL doesn’t let you destroy an active database, having something hold the sql.DB connection makes the following SQL fail:

db.Exec(`DROP DATABASE test_database`) pq: database "test_database" is being accessed by other users

My suggestion is to create a new function called txdb.Reset:

var drivers struct {
    sync.Mutex
    drvs map[string]*txDriver
}

func Reset(name string) error {
    drivers.Lock()
    defer drivers.Unlock()

    d, ok := drivers.drv[name]
    if !ok {
        return ErrNotRegistered
    }

    if d.db == nil {
        return nil
    }

    var err error
    for _, c := range d.conns {
        if cerr := c.Close(); cerr != nil && err == nil {
            err = cerr
        }
    }
    if err != nil {
        return err
    }

    if err := d.db.Close(); err != nil {
        return err
    }

    d.db = nil
    return nil
}

This would also require changes to txdb.Register:

func Register(name, drv, dsn string, options ...func(*conn) error) {
    drv := &txDriver{
        dsn:     dsn,
        drv:     drv,
        conns:   make(map[string]*conn),
        options: options,
    }
    sql.Register(name, drv)

    drivers.Lock()
    defer drivers.Unlock()
    drivers[name] = drv
}

Sorry I have not tested or documented this code.

[l3pp4rd]

Hi, the whole purpose of txdb is to improve performance and isolation for a test database. if you are recreating database on every test, it does not seem to be worth it. Postgres specifically, supports table related operations within transaction, so each of your test could create tables and have them rolled back after test is done.

But I do not see an issue in adding this method. you could open the pull request. but maybe instead of Reset it could be called, Close

[sfllaw]

@l3pp4rd My use case is to create a temporary database in TestMain when setting up the tests and dropping the database after the tests have run. With txdb holding on to a database connection permanently, there is no way to cleanly drop the test database after the tests finish running.

I could definitely submit a pull request for this, with under the name of txdb.Close Initially, I thought that this isn't really synonymous with other Closers, but since this function calls txDriver.db.Close(), I think you are right.

[l3pp4rd]

yes, I think the pull request can be accepted it makes sense

[sfllaw]

I have now revised my opinion: we should not require the developer to call database/sql.DB.Driver, instead we should make database/sql.DB.Close just Do The Right Thing.