jmoiron / modl

golang database modelling library
MIT License
479 stars 48 forks source link

Export (Transaction).tx *sqlx.Tx field #28

Closed sqs closed 9 years ago

sqs commented 9 years ago

This PR exports the (Transaction).tx *sqlx.Tx field.

We use an internal fork of modl at @sourcegraph with this field exported. It makes it easier to add additional functionality to SqlExecutor that makes use of sqlx and database/sql methods/helpers. Here 2 examples from our codebase where exporting this field has been necessary/helpful. I'm just mentioning these because this is essentially an API design decision and real world use cases are often helpful to me in these situations.

  1. We have a Copy function that takes an sqlx.Tx (and calls .Prepare on it), but internally all of our DB handles are modl.SqlExecutor:
// Copy executes a PostgreSQL COPY statement. All of the rows must have the same
// data type, which must be mapped to a table using modl.
func Copy(tx *sqlx.Tx, tableName string, rows interface{}, omitCols ...int) error {
   ...
}

In order to be able to call Copy on a modl.SqlExecutor or *modl.Transaction, the tx field must be exported. Then we can call, e.g., db.Copy(dbh.(*modl.Transaction).Tx, "mytable", data).

  1. We have helper functions for selecting a single SQL result into a variable, which accepts a modl.SqlExecutor as an argument and wants to be able to call QueryRow on the DBMap or Transaction:
func selectVal(e modl.SqlExecutor, v interface{}, query string, args ...interface{}) error {
    e = getUnderlyingSQLExecutor(e)

    var row *sql.Row
    switch e := e.(type) {
    case *modl.DbMap:
        row = e.Dbx.QueryRow(query, args...)
    case *modl.Transaction:
        row = e.Tx.QueryRow(query, args...)
    default:
        panic("selectVal: unknown type: " + reflect.TypeOf(e).String())
    }

    if row != nil {
        err := row.Scan(v)
        if err != nil {
            return err
        }
    }

    return nil
}

The tx field must be exported in order to be able to use sqlx's QueryRow on a *modl.Transaction. Note that the analogous Dbx field on DbMap is exported, so it would seem consistent for Tx to also be exported.

jmoiron commented 9 years ago

This makes the tracing layer a little easier to defeat, but I agree that it mirrors the exported Db/Dbx field on the DbMap.