Skip to content

Commit

Permalink
tests: enhanced integration tests to cover ? placeholders (#32)
Browse files Browse the repository at this point in the history
also removed some unreachable code which wrongly lowered coverage
  • Loading branch information
murfffi authored Jan 14, 2025
1 parent 37a4011 commit 657aa1d
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 17 deletions.
4 changes: 2 additions & 2 deletions internal/isql/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,11 @@ func testInsert(t *testing.T, conn *sql.DB) {
require.NoError(t, err)
_, err = conn.Exec("CREATE TABLE test(a int)")
require.NoError(t, err)
insertRes, err := conn.Exec("INSERT INTO test (a) VALUES (1)")
insertRes, err := conn.Exec("INSERT INTO test (a) VALUES (?)", 1)
require.NoError(t, err)
_, err = insertRes.RowsAffected()
require.Error(t, err) // not supported yet, see todo in statement.go/exec
selectRes, err := conn.Query("SELECT * FROM test WHERE a = 1 LIMIT 1")
selectRes, err := conn.Query("SELECT * FROM test WHERE a = ? LIMIT 1", 1)
require.NoError(t, err)
defer fi.NoErrorF(selectRes.Close, t)
require.True(t, selectRes.Next())
Expand Down
23 changes: 8 additions & 15 deletions internal/isql/statement.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ func (s *Stmt) Close() error {

// NumInput returns number of inputs
func (s *Stmt) NumInput() int {
// -1 means the driver doesn't know how to count the number of
// placeholders, so (database/sql) won't sanity check input
// See https://cs.opensource.google/go/go/+/refs/tags/go1.23.4:src/database/sql/convert.go;l=109
// We could implement counting placeholders in the future.
return -1
}

Expand All @@ -43,17 +47,15 @@ func (s *Stmt) CheckNamedValue(val *driver.NamedValue) error {
// Exec executes a query that doesn't return rows
func (s *Stmt) Exec(args []driver.Value) (driver.Result, error) {
// This implementation is never used in recent versions of Go - ExecContext is used instead
// even when the user calls sql.Stmt.Exec(). We could implement this required interface method
// with panic("not implemented") but we keep a full implementation just in case.
nargs := toNamedValues(args)
return s.ExecContext(context.Background(), nargs)
// even when the user calls sql.Stmt.Exec(). Following the example in database/sql/fakedb_test.go
// we can implement this as:
panic("ExecContext was not called.")
}

// Query executes a query that may return rows
func (s *Stmt) Query(args []driver.Value) (driver.Rows, error) {
// Comment in Exec() above applies here as well.
nargs := toNamedValues(args)
return s.QueryContext(context.Background(), nargs)
panic("QueryContext was not called.")
}

// QueryContext executes a query that may return rows
Expand All @@ -76,15 +78,6 @@ func (s *Stmt) ExecContext(ctx context.Context, args []driver.NamedValue) (drive
return exec(ctx, session, stmt)
}

func toNamedValues(args []driver.Value) []driver.NamedValue {
// note that database/sql ensures Value never wraps a NamedValue so we don't need to check
nargs := make([]driver.NamedValue, len(args))
for i, arg := range args {
nargs[i] = driver.NamedValue{Ordinal: i, Value: arg}
}
return nargs
}

func template(query string) string {
ordinal := 1
for {
Expand Down

0 comments on commit 657aa1d

Please sign in to comment.