From 5a84b4799e4d13b211233b761c0228d2bc51651e Mon Sep 17 00:00:00 2001 From: Lz Date: Wed, 11 Dec 2024 11:14:33 +0800 Subject: [PATCH 1/8] fix(rows): doesn't close rows on Scan --- rows.go | 1 - rows_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/rows.go b/rows.go index 9f4dd0f..123b80b 100644 --- a/rows.go +++ b/rows.go @@ -24,7 +24,6 @@ func (r *Rows) Close() error { } func (r *Rows) Scan(dest ...any) error { - defer r.Close() return r.Rows.Scan(dest...) } diff --git a/rows_test.go b/rows_test.go index 1b3ff56..bcf4eb3 100644 --- a/rows_test.go +++ b/rows_test.go @@ -36,6 +36,40 @@ func TestRowsBind(t *testing.T) { name string run func(t *testing.T) }{ + { + name: "scan_on_rows_should_work", + run: func(t *testing.T) { + type user struct { + ID int + Status int + Email string + Passwd string + Salt string + Created *time.Time + } + rows, err := db.Query("SELECT id,email FROM rows WHERE id<4") + require.NoError(t, err) + + var id int + var email string + + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 1, id) + require.Equal(t, "test1@mail.com", email) + + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 2, id) + require.Equal(t, "test2@mail.com", email) + + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 3, id) + require.Equal(t, "test3@mail.com", email) + + }, + }, { name: "bind_slice_of_struct_should_work", run: func(t *testing.T) { From 326662bb5d0c8b3720f5a7b06d2ed8736d1237a2 Mon Sep 17 00:00:00 2001 From: Lz Date: Wed, 11 Dec 2024 11:23:20 +0800 Subject: [PATCH 2/8] fix(rows): updated test code for rows.Scan --- rows_test.go | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/rows_test.go b/rows_test.go index bcf4eb3..fb9ad7d 100644 --- a/rows_test.go +++ b/rows_test.go @@ -39,35 +39,31 @@ func TestRowsBind(t *testing.T) { { name: "scan_on_rows_should_work", run: func(t *testing.T) { - type user struct { - ID int - Status int - Email string - Passwd string - Salt string - Created *time.Time - } + rows, err := db.Query("SELECT id,email FROM rows WHERE id<4") require.NoError(t, err) + defer rows.Close() + var id int var email string - err = rows.Scan(&id, &email) - require.NoError(t, err) - require.Equal(t, 1, id) - require.Equal(t, "test1@mail.com", email) - - err = rows.Scan(&id, &email) - require.NoError(t, err) - require.Equal(t, 2, id) - require.Equal(t, "test2@mail.com", email) - - err = rows.Scan(&id, &email) - require.NoError(t, err) - require.Equal(t, 3, id) - require.Equal(t, "test3@mail.com", email) - + for rows.Next() { + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 1, id) + require.Equal(t, "test1@mail.com", email) + + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 2, id) + require.Equal(t, "test2@mail.com", email) + + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 3, id) + require.Equal(t, "test3@mail.com", email) + } }, }, { From 61f1b4474aa38b34d06b957ef12d8c3525b12e10 Mon Sep 17 00:00:00 2001 From: Lz Date: Wed, 11 Dec 2024 11:31:57 +0800 Subject: [PATCH 3/8] fix(rows): upgrade golangci-lint and call rows.Next in each scan --- .github/workflows/tests.yml | 4 ++-- rows_test.go | 31 +++++++++++++++---------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 833e78f..4c9113c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -25,9 +25,9 @@ jobs: with: go-version: ^1.21 - name: golangci-lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v6 with: - version: v1.54 + version: v1.61 unit-tests: name: Unit Tests runs-on: ubuntu-latest diff --git a/rows_test.go b/rows_test.go index fb9ad7d..aade77a 100644 --- a/rows_test.go +++ b/rows_test.go @@ -48,22 +48,21 @@ func TestRowsBind(t *testing.T) { var id int var email string - for rows.Next() { - err = rows.Scan(&id, &email) - require.NoError(t, err) - require.Equal(t, 1, id) - require.Equal(t, "test1@mail.com", email) - - err = rows.Scan(&id, &email) - require.NoError(t, err) - require.Equal(t, 2, id) - require.Equal(t, "test2@mail.com", email) - - err = rows.Scan(&id, &email) - require.NoError(t, err) - require.Equal(t, 3, id) - require.Equal(t, "test3@mail.com", email) - } + rows.Next() + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 1, id) + require.Equal(t, "test1@mail.com", email) + rows.Next() + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 2, id) + require.Equal(t, "test2@mail.com", email) + rows.Next() + err = rows.Scan(&id, &email) + require.NoError(t, err) + require.Equal(t, 3, id) + require.Equal(t, "test3@mail.com", email) }, }, { From 810680309648be871bc8a2bff93dd83678cf6579 Mon Sep 17 00:00:00 2001 From: Lz Date: Thu, 12 Dec 2024 10:50:03 +0800 Subject: [PATCH 4/8] fix(rows): fixed rows colse issue in stmt_reuse_should_work_in_rows_scan test --- client_stmt_test.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/client_stmt_test.go b/client_stmt_test.go index 4c51e6c..b32fd51 100644 --- a/client_stmt_test.go +++ b/client_stmt_test.go @@ -12,20 +12,20 @@ func TestStmt(t *testing.T) { d, err := sql.Open("sqlite3", "file::memory:") require.NoError(t, err) - _, err = d.Exec("CREATE TABLE `rows` (`id` int , `status` tinyint,`email` varchar(50),`passwd` varchar(120), `salt` varchar(45), `created` DATETIME, PRIMARY KEY (`id`))") + _, err = d.Exec("CREATE TABLE `rows_stmt` (`id` int , `status` tinyint,`email` varchar(50),`passwd` varchar(120), `salt` varchar(45), `created` DATETIME, PRIMARY KEY (`id`))") require.NoError(t, err) now := time.Now() - _, err = d.Exec("INSERT INTO `rows`(`id`,`status`,`email`,`passwd`,`salt`,`created`) VALUES(1, 1,'test1@mail.com','1xxxx','1zzzz', ?)", now) + _, err = d.Exec("INSERT INTO `rows_stmt`(`id`,`status`,`email`,`passwd`,`salt`,`created`) VALUES(1, 1,'test1@mail.com','1xxxx','1zzzz', ?)", now) require.NoError(t, err) - _, err = d.Exec("INSERT INTO `rows`(`id`,`status`,`email`,`passwd`,`salt`) VALUES(2, 2,'test2@mail.com','2xxxx','2zzzz')") + _, err = d.Exec("INSERT INTO `rows_stmt`(`id`,`status`,`email`,`passwd`,`salt`) VALUES(2, 2,'test2@mail.com','2xxxx','2zzzz')") require.NoError(t, err) - _, err = d.Exec("INSERT INTO `rows`(`id`,`status`,`email`,`passwd`,`salt`) VALUES(3, 3,'test3@mail.com','3xxxx','3zzzz')") + _, err = d.Exec("INSERT INTO `rows_stmt`(`id`,`status`,`email`,`passwd`,`salt`) VALUES(3, 3,'test3@mail.com','3xxxx','3zzzz')") require.NoError(t, err) - _, err = d.Exec("INSERT INTO `rows`(`id`) VALUES(4)") + _, err = d.Exec("INSERT INTO `rows_stmt`(`id`) VALUES(4)") require.NoError(t, err) stmtMaxIdleTime := StmtMaxIdleTime @@ -51,7 +51,7 @@ func TestStmt(t *testing.T) { } for i := 0; i < 100; i++ { - rows, err := db.Query("SELECT * FROM rows WHERE id=100 order by id") + rows, err := db.Query("SELECT id FROM rows_stmt WHERE id>=100 order by id") require.NoError(t, err) var list [][]int err = rows.Bind(&list) @@ -117,7 +117,7 @@ func TestStmt(t *testing.T) { { name: "stmt_reuse_should_work_in_exec", run: func(t *testing.T) { - q := "INSERT INTO `rows`(`id`,`status`) VALUES(?, ?)" + q := "INSERT INTO `rows_stmt`(`id`,`status`) VALUES(?, ?)" result, err := db.Exec(q, 200, 0) require.NoError(t, err) @@ -147,7 +147,7 @@ func TestStmt(t *testing.T) { name: "stmt_reuse_should_work_in_rows_scan", run: func(t *testing.T) { var id int - q := "SELECT id, 'rows_scan' as reuse FROM rows WHERE id = ?" + q := "SELECT id, 'rows_scan' as reuse FROM rows_stmt WHERE id = ?" rows, err := db.Query(q, 200) require.NoError(t, err) @@ -164,6 +164,8 @@ func TestStmt(t *testing.T) { require.True(t, s.isUsing) rows.Scan(&id) // nolint: errcheck + require.True(t, s.isUsing) + rows.Close() require.False(t, s.isUsing) db.closeStaleStmt() @@ -181,7 +183,7 @@ func TestStmt(t *testing.T) { ID int } - q := "SELECT id, 'rows_bind' as reuse FROM rows WHERE id = ?" + q := "SELECT id, 'rows_bind' as reuse FROM rows_stmt WHERE id = ?" rows, err := db.Query(q, 200) require.NoError(t, err) @@ -212,7 +214,7 @@ func TestStmt(t *testing.T) { name: "stmt_reuse_should_work_in_row_scan", run: func(t *testing.T) { var id int - q := "SELECT id, 'row_scan' as reuse FROM rows WHERE id = ?" + q := "SELECT id, 'row_scan' as reuse FROM rows_stmt WHERE id = ?" row := db.QueryRow(q, 200) require.NoError(t, err) @@ -245,7 +247,7 @@ func TestStmt(t *testing.T) { var r struct { ID int } - q := "SELECT id, 'row_bind' as reuse FROM rows WHERE id = ?" + q := "SELECT id, 'row_bind' as reuse FROM rows_stmt WHERE id = ?" row, err := db.Query(q, 200) require.NoError(t, err) From 73e8e3c0b6083998aeec9209b12569fadd41f0cd Mon Sep 17 00:00:00 2001 From: Lz Date: Thu, 12 Dec 2024 11:04:36 +0800 Subject: [PATCH 5/8] chore(test): added precondtion tests on Rows.Close and Rows.Bind --- rows_test.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/rows_test.go b/rows_test.go index aade77a..6c2114e 100644 --- a/rows_test.go +++ b/rows_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestRowsBind(t *testing.T) { +func TestRows(t *testing.T) { d, err := sql.Open("sqlite3", "file::memory:") require.NoError(t, err) @@ -36,6 +36,28 @@ func TestRowsBind(t *testing.T) { name string run func(t *testing.T) }{ + { + name: "close_should_always_work", + run: func(t *testing.T) { + + rows := &Rows{} + rows.Close() + }, + }, + { + name: "bind_only_work_with_non_nil_pointer", + run: func(t *testing.T) { + + rows := &Rows{} + var dest int + err := rows.Bind(dest) + require.ErrorIs(t, err, ErrMustPointer) + + var dest2 *int + err = rows.Bind(dest2) + require.ErrorIs(t, err, ErrMustNotNilPointer) + }, + }, { name: "scan_on_rows_should_work", run: func(t *testing.T) { From 6f59bc5291d18ce4c67942388360899f66bc7302 Mon Sep 17 00:00:00 2001 From: Lz Date: Thu, 12 Dec 2024 11:11:06 +0800 Subject: [PATCH 6/8] chore(test): added precondtion tests on Row.Close and Row.Bind --- row_test.go | 24 +++++++++++++++++++++++- rows_test.go | 2 +- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/row_test.go b/row_test.go index 7dfad5a..97c0509 100644 --- a/row_test.go +++ b/row_test.go @@ -64,7 +64,7 @@ func (cb *customBinder) Bind(_ reflect.Value, columns []string) []any { return values } -func TestRowBind(t *testing.T) { +func TestRow(t *testing.T) { d, err := sql.Open("sqlite3", "file::memory:?cache=shared") require.NoError(t, err) @@ -91,6 +91,28 @@ func TestRowBind(t *testing.T) { name string run func(t *testing.T) }{ + { + name: "close_should_always_work", + run: func(*testing.T) { + + rows := &Row{} + rows.Close() + }, + }, + { + name: "bind_only_work_with_non_nil_pointer", + run: func(t *testing.T) { + + rows := &Row{} + var dest int + err := rows.Bind(dest) + require.ErrorIs(t, err, ErrMustPointer) + + var dest2 *int + err = rows.Bind(dest2) + require.ErrorIs(t, err, ErrMustNotNilPointer) + }, + }, { name: "full_columns_should_work", run: func(t *testing.T) { diff --git a/rows_test.go b/rows_test.go index 6c2114e..41e4108 100644 --- a/rows_test.go +++ b/rows_test.go @@ -38,7 +38,7 @@ func TestRows(t *testing.T) { }{ { name: "close_should_always_work", - run: func(t *testing.T) { + run: func(*testing.T) { rows := &Rows{} rows.Close() From c260cbc25c5bb6da8cab4f1b81aedb8610a28353 Mon Sep 17 00:00:00 2001 From: Lz Date: Thu, 12 Dec 2024 11:30:46 +0800 Subject: [PATCH 7/8] chore(test): added precondtion tests on Row.Close and Row.Bind --- row.go | 14 +++++++------- row_test.go | 13 +++++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/row.go b/row.go index a511e14..7ab1c75 100644 --- a/row.go +++ b/row.go @@ -77,13 +77,6 @@ func (r *Row) Bind(dest any) error { return r.err } - if !r.rows.Next() { - if err := r.rows.Err(); err != nil { - return err - } - return sql.ErrNoRows - } - v := reflect.ValueOf(dest) if v.Kind() != reflect.Pointer { @@ -95,6 +88,13 @@ func (r *Row) Bind(dest any) error { } var err error + if !r.rows.Next() { + err = r.rows.Err() + if err != nil { + return err + } + return sql.ErrNoRows + } cols, err := getColumns(r.query, r.rows) if err != nil { diff --git a/row_test.go b/row_test.go index 97c0509..f558d5c 100644 --- a/row_test.go +++ b/row_test.go @@ -94,22 +94,23 @@ func TestRow(t *testing.T) { { name: "close_should_always_work", run: func(*testing.T) { - - rows := &Row{} - rows.Close() + var row *Row + row.Close() + row = &Row{} + row.Close() }, }, { name: "bind_only_work_with_non_nil_pointer", run: func(t *testing.T) { - rows := &Row{} + row := &Row{} var dest int - err := rows.Bind(dest) + err := row.Bind(dest) require.ErrorIs(t, err, ErrMustPointer) var dest2 *int - err = rows.Bind(dest2) + err = row.Bind(dest2) require.ErrorIs(t, err, ErrMustNotNilPointer) }, }, From 3f8d5d71dbd5506a8ba38fcc7f5ab1195852603f Mon Sep 17 00:00:00 2001 From: Lz Date: Thu, 12 Dec 2024 11:35:11 +0800 Subject: [PATCH 8/8] chore(docs): updated CHANGELOG.md --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af6d35b..3564139 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [1.5.1] +## [1.5.2] - 2024-12-12 +- fix(rows): don't close rows in rows.Scan (#49) + +## [1.5.1] - 2024-06-12 - !fix(orderby): use BuildOption instead of allowedColumns (#46) - feat(string): added nullable String/Null for sql/json (#47)