Merge pull request #6 from Vinovest/mj/single-row

Validate single row count for Row.Scan

Author: mrj0

named_test.go

@@ -276,7 +276,7 @@ func TestNamedQueries(t *testing.T) {
 
 		for _, p := range sls {
 			dest := Person{}
-			err = db.Get(&dest, db.Rebind("SELECT * FROM person WHERE email=?"), p.Email)
+			err = db.Get(&dest, db.Rebind("SELECT * FROM person WHERE email=? limit 1"), p.Email)
 			test.Error(err)
 			if dest.Email != p.Email {
 				t.Errorf("expected %s, got %s", p.Email, dest.Email)

sqlx.go

@@ -15,6 +15,12 @@ import (
 	"github.com/vinovest/sqlx/reflectx"
 )
 
+// ErrMultiRows is returned by functions which are expected to work with result sets
+// that only contain a single row but multiple rows were returned.
+// This typically indicates an issue with the query such as a missing join criteria or
+// limit condition or the use of Get(...) when Select(...) was intended.
+var ErrMultiRows = errors.New("sql: multiple rows returned")
+
 // Although the NameMapper is convenient, in practice it should not
 // be relied on except for application code.  If you are writing a library
 // that uses sqlx, you should be aware that the name mappings you expect
@@ -175,6 +181,7 @@ type Row struct {
 
 // Scan is a fixed implementation of sql.Row.Scan, which does not discard the
 // underlying error from the internal rows object if it exists.
+// Returns ErrMultiRows if the result set contains more than one row.
 func (r *Row) Scan(dest ...interface{}) error {
 	if r.err != nil {
 		return r.err
@@ -206,10 +213,16 @@ func (r *Row) Scan(dest ...interface{}) error {
 		}
 		return sql.ErrNoRows
 	}
-	err := r.rows.Scan(dest...)
-	if err != nil {
+	if err := r.rows.Scan(dest...); err != nil {
+		return err
+	}
+
+	if r.rows.Next() {
+		return ErrMultiRows
+	} else if err := r.rows.Err(); err != nil {
 		return err
 	}
+
 	// Make sure the query can be processed to completion with no errors.
 	if err := r.rows.Close(); err != nil {
 		return err
@@ -352,7 +365,7 @@ func (db *DB) Select(dest interface{}, query string, args ...interface{}) error
 
 // Get using this DB.
 // Any placeholder parameters are replaced with supplied args.
-// An error is returned if the result set is empty.
+// An error is returned if the result set is empty or contains more than one row.
 func (db *DB) Get(dest interface{}, query string, args ...interface{}) error {
 	return Get(db, dest, query, args...)
 }
@@ -483,7 +496,7 @@ func (tx *Tx) QueryRowx(query string, args ...interface{}) *Row {
 
 // Get within a transaction.
 // Any placeholder parameters are replaced with supplied args.
-// An error is returned if the result set is empty.
+// An error is returned if the result set is empty or contains more than one row.
 func (tx *Tx) Get(dest interface{}, query string, args ...interface{}) error {
 	return Get(tx, dest, query, args...)
 }
@@ -551,7 +564,7 @@ func (s *Stmt) Select(dest interface{}, args ...interface{}) error {
 
 // Get using the prepared statement.
 // Any placeholder parameters are replaced with supplied args.
-// An error is returned if the result set is empty.
+// An error is returned if the result set is empty or contains more than one row.
 func (s *Stmt) Get(dest interface{}, args ...interface{}) error {
 	return Get(&qStmt{s}, dest, "", args...)
 }
@@ -727,7 +740,7 @@ func Select(q Queryer, dest interface{}, query string, args ...interface{}) erro
 // to dest.  If dest is scannable, the result must only have one column.  Otherwise,
 // StructScan is used.  Get will return sql.ErrNoRows like row.Scan would.
 // Any placeholder parameters are replaced with supplied args.
-// An error is returned if the result set is empty.
+// An error is returned if the result set is empty or contains more than one row.
 func Get(q Queryer, dest interface{}, query string, args ...interface{}) error {
 	r := q.QueryRowx(query, args...)
 	return r.scanAny(dest, false)

sqlx_test.go

@@ -74,7 +74,7 @@ func ConnectAll() {
 	if TestMysql {
 		mysqldb, err = Connect("mysql", mydsn)
 		if err != nil {
-			fmt.Printf("Disabling MySQL tests:\n    %v", err)
+			fmt.Printf("Disabling MySQL tests:\n    %v\n", err)
 			TestMysql = false
 		}
 	} else {
@@ -84,7 +84,7 @@ func ConnectAll() {
 	if TestSqlite {
 		sldb, err = Connect("sqlite3", sqdsn)
 		if err != nil {
-			fmt.Printf("Disabling SQLite:\n    %v", err)
+			fmt.Printf("Disabling SQLite:\n    %v\n", err)
 			TestSqlite = false
 		}
 	} else {
@@ -1728,6 +1728,46 @@ func TestEmbeddedLiterals(t *testing.T) {
 	})
 }
 
+// TestGet tests to ensure that Get behaves correctly for
+// single row and multi row results.
+func TestGet(t *testing.T) {
+	var schema = Schema{
+		create: `CREATE TABLE tst (v integer);`,
+		drop:   `drop table tst;`,
+	}
+
+	RunWithSchema(schema, t, func(db *DB, t *testing.T, now string) {
+		for _, v := range []int{1, 2} {
+			_, err := db.Exec(db.Rebind("INSERT INTO tst (v) VALUES (?)"), v)
+			if err != nil {
+				t.Error(err)
+			}
+		}
+
+		tests := []struct {
+			name string
+			val  int
+			err  bool
+		}{
+			{"multi-rows", 1, true},
+			{"single-row", 2, false},
+		}
+		for _, tc := range tests {
+			t.Run(tc.name, func(t *testing.T) {
+				var v int
+				err := db.Get(&v, db.Rebind("SELECT v FROM tst WHERE v >= ?"), tc.val)
+				if tc.err {
+					if err == nil {
+						t.Error("expected error but got nil")
+					}
+				} else if err != nil {
+					t.Error("unexpected error:", err)
+				}
+			})
+		}
+	})
+}
+
 func BenchmarkBindStruct(b *testing.B) {
 	b.StopTimer()
 	q1 := `INSERT INTO foo (a, b, c, d) VALUES (:name, :age, :first, :last)`