From 336392b5379d515280036c8f813e0c2f605708d1 Mon Sep 17 00:00:00 2001 From: Marco de Abreu <18629099+marcoabreu@users.noreply.github.com> Date: Fri, 3 Apr 2026 01:40:18 +0200 Subject: [PATCH 1/2] fix(queries): check rows.Err() after Next() loop in bind() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bind() did not check rows.Err() after the rows.Next() loop. When rows.Next() returned false due to a connection error (not an empty result set), bind() returned sql.ErrNoRows — silently swallowing the real error. This caused callers that distinguish "not found" from "transient failure" to make wrong decisions. In production, intermittent connection errors were misclassified as "row not found", causing permanent data loss in retry-based message processing systems. The fix adds a rows.Err() check after the loop, before the foundOne/ErrNoRows check. This covers all bind kinds (struct, slice, ptr slice) uniformly and is backwards-compatible. --- queries/reflect.go | 4 ++ queries/reflect_test.go | 130 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/queries/reflect.go b/queries/reflect.go index 855c5f90..0846b2e5 100644 --- a/queries/reflect.go +++ b/queries/reflect.go @@ -270,6 +270,10 @@ Rows: } } + if err := rows.Err(); err != nil { + return errors.Wrap(err, "failed to iterate rows") + } + if bkind == kindStruct && !foundOne { return sql.ErrNoRows } diff --git a/queries/reflect_test.go b/queries/reflect_test.go index a58bc535..fe0b6451 100644 --- a/queries/reflect_test.go +++ b/queries/reflect_test.go @@ -935,4 +935,134 @@ func TestUnTitleCase(t *testing.T) { t.Errorf("[%d] (%s) Out was wrong: %q, want: %q", i, test.In, out, test.Out) } } +} + +// TestBindStructRowsError tests that bind returns the underlying connection +// error instead of sql.ErrNoRows when rows.Next() returns false due to an I/O +// failure. RowError(0, ...) causes the first Next() call to return false with +// the error stored in rows.Err(), simulating a connection drop before any row +// is read. Without checking rows.Err(), bind sees foundOne==false and +// incorrectly returns sql.ErrNoRows. +func TestBindStructRowsError(t *testing.T) { + t.Parallel() + + testResults := struct { + ID int + Name string `boil:"test"` + }{} + + query := &Query{ + from: []string{"fun"}, + dialect: &drivers.Dialect{LQ: '"', RQ: '"', UseIndexPlaceholders: true}, + } + + db, mock, err := sqlmock.New() + if err != nil { + t.Error(err) + } + + connErr := fmt.Errorf("connection reset by peer") + ret := sqlmock.NewRows([]string{"id", "test"}). + AddRow(driver.Value(int64(35)), driver.Value("pat")). + RowError(0, connErr) + mock.ExpectQuery(`SELECT \* FROM "fun";`).WillReturnRows(ret) + + err = query.Bind(context.Background(), db, &testResults) + if err == nil { + t.Error("expected error, got nil") + } + if err == sql.ErrNoRows { + t.Error("got sql.ErrNoRows, want the underlying connection error") + } + if err != nil && !strings.Contains(err.Error(), "connection reset by peer") { + t.Error("error did not contain expected message, got:", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} + +// TestBindSliceRowsError tests that bind returns an error instead of silently +// returning a partial result set when a connection error occurs mid-iteration. +// Two rows are added but RowError(1, ...) causes the second Next() call to +// fail. Without checking rows.Err() after the loop, bind returns nil with only +// one row in the slice — the caller has no way to know the result is incomplete. +func TestBindSliceRowsError(t *testing.T) { + t.Parallel() + + testResults := []struct { + ID int + Name string `boil:"test"` + }{} + + query := &Query{ + from: []string{"fun"}, + dialect: &drivers.Dialect{LQ: '"', RQ: '"', UseIndexPlaceholders: true}, + } + + db, mock, err := sqlmock.New() + if err != nil { + t.Error(err) + } + + connErr := fmt.Errorf("connection reset by peer") + ret := sqlmock.NewRows([]string{"id", "test"}). + AddRow(driver.Value(int64(35)), driver.Value("pat")). + AddRow(driver.Value(int64(12)), driver.Value("cat")). + RowError(1, connErr) + mock.ExpectQuery(`SELECT \* FROM "fun";`).WillReturnRows(ret) + + err = query.Bind(context.Background(), db, &testResults) + if err == nil { + t.Error("expected error from connection failure during iteration, got nil") + } + if err != nil && !strings.Contains(err.Error(), "connection reset by peer") { + t.Error("error did not contain expected message, got:", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} + +// TestBindPtrSliceRowsError tests the same rows.Err() behavior for pointer +// slice binding. RowError(0, ...) causes the first Next() to fail, so the +// slice stays empty. Without checking rows.Err(), bind returns nil and the +// caller receives an empty slice with no indication that an error occurred. +func TestBindPtrSliceRowsError(t *testing.T) { + t.Parallel() + + testResults := []*struct { + ID int + Name string `boil:"test"` + }{} + + query := &Query{ + from: []string{"fun"}, + dialect: &drivers.Dialect{LQ: '"', RQ: '"', UseIndexPlaceholders: true}, + } + + db, mock, err := sqlmock.New() + if err != nil { + t.Error(err) + } + + connErr := fmt.Errorf("connection reset by peer") + ret := sqlmock.NewRows([]string{"id", "test"}). + AddRow(driver.Value(int64(35)), driver.Value("pat")). + RowError(0, connErr) + mock.ExpectQuery(`SELECT \* FROM "fun";`).WillReturnRows(ret) + + err = query.Bind(context.Background(), db, &testResults) + if err == nil { + t.Error("expected error from connection failure, got nil") + } + if err != nil && !strings.Contains(err.Error(), "connection reset by peer") { + t.Error("error did not contain expected message, got:", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } } \ No newline at end of file From 5477c208406f86770b736530fe3480d1a2b8f01d Mon Sep 17 00:00:00 2001 From: Marco de Abreu <18629099+marcoabreu@users.noreply.github.com> Date: Fri, 3 Apr 2026 02:24:28 +0200 Subject: [PATCH 2/2] fix(drivers): check rows.Err() after Next() loops in all drivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same bug pattern as the bind() fix: rows.Next() loops in driver schema introspection functions did not check rows.Err() afterward. Per the database/sql contract, Next() returns false both when the result set is exhausted and when an error occurs — Err() must be consulted to distinguish the two cases. Without this check, a connection error during code generation would silently produce incomplete schema information (missing tables, columns, or indexes), leading to incorrect generated code. Affected functions across all four drivers: - PostgreSQL: TableNames, ViewNames, Columns, loadUniqueColumns - MySQL: TableNames, ViewNames, Columns - MSSQL: TableNames, ViewNames, Columns - SQLite3: TableNames, ViewNames, tableInfo, indexes (nested loops) --- drivers/sqlboiler-mssql/driver/mssql.go | 12 ++ drivers/sqlboiler-mssql/driver/mssql_test.go | 107 ++++++++++++ drivers/sqlboiler-mysql/driver/mysql.go | 9 + drivers/sqlboiler-mysql/driver/mysql_test.go | 82 +++++++++ drivers/sqlboiler-psql/driver/psql.go | 17 ++ drivers/sqlboiler-psql/driver/psql_test.go | 155 ++++++++++++++++++ drivers/sqlboiler-sqlite3/driver/sqlite3.go | 16 ++ .../sqlboiler-sqlite3/driver/sqlite3_test.go | 131 ++++++++++++++- 8 files changed, 528 insertions(+), 1 deletion(-) diff --git a/drivers/sqlboiler-mssql/driver/mssql.go b/drivers/sqlboiler-mssql/driver/mssql.go index e0059cb1..6b6c1adf 100644 --- a/drivers/sqlboiler-mssql/driver/mssql.go +++ b/drivers/sqlboiler-mssql/driver/mssql.go @@ -193,6 +193,10 @@ func (m *MSSQLDriver) TableNames(schema string, whitelist, blacklist []string) ( names = append(names, name) } + if err := rows.Err(); err != nil { + return nil, err + } + return names, nil } @@ -240,6 +244,10 @@ func (m *MSSQLDriver) ViewNames(schema string, whitelist, blacklist []string) ([ names = append(names, name) } + if err := rows.Err(); err != nil { + return nil, err + } + return names, nil } @@ -359,6 +367,10 @@ func (m *MSSQLDriver) Columns(schema, tableName string, whitelist, blacklist []s columns = append(columns, column) } + if err := rows.Err(); err != nil { + return nil, err + } + return columns, nil } diff --git a/drivers/sqlboiler-mssql/driver/mssql_test.go b/drivers/sqlboiler-mssql/driver/mssql_test.go index 0d986529..dc2a9821 100644 --- a/drivers/sqlboiler-mssql/driver/mssql_test.go +++ b/drivers/sqlboiler-mssql/driver/mssql_test.go @@ -26,11 +26,13 @@ import ( "bytes" "encoding/json" "flag" + "fmt" "os" "os/exec" "regexp" "testing" + "github.com/DATA-DOG/go-sqlmock" "github.com/aarondl/sqlboiler/v4/drivers" ) @@ -139,3 +141,108 @@ func TestDriver(t *testing.T) { } }) } + +// TestTableNames_RowsErr verifies that TableNames checks rows.Err() after +// iterating and propagates any error encountered during row iteration back +// to the caller rather than silently returning partial results. +func TestTableNames_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + + simulatedErr := fmt.Errorf("rows iteration error") + + rows := sqlmock.NewRows([]string{"table_name"}). + AddRow("table1"). + RowError(0, simulatedErr) + + mock.ExpectQuery(`SELECT table_name`). + WithArgs("dbo"). + WillReturnRows(rows) + + driver := &MSSQLDriver{conn: db} + _, err = driver.TableNames("dbo", nil, nil) + if err == nil { + t.Fatal("expected error from rows.Err(), got nil") + } + if err.Error() != simulatedErr.Error() { + t.Errorf("expected error %q, got %q", simulatedErr, err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unfulfilled expectations: %s", err) + } +} + +// TestViewNames_RowsErr verifies that ViewNames checks rows.Err() after +// iterating and propagates any error encountered during row iteration back +// to the caller rather than silently returning partial results. +func TestViewNames_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + + simulatedErr := fmt.Errorf("rows iteration error") + + rows := sqlmock.NewRows([]string{"table_name"}). + AddRow("view1"). + RowError(0, simulatedErr) + + mock.ExpectQuery(`select table_name`). + WithArgs("dbo"). + WillReturnRows(rows) + + driver := &MSSQLDriver{conn: db} + _, err = driver.ViewNames("dbo", nil, nil) + if err == nil { + t.Fatal("expected error from rows.Err(), got nil") + } + if err.Error() != simulatedErr.Error() { + t.Errorf("expected error %q, got %q", simulatedErr, err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unfulfilled expectations: %s", err) + } +} + +// TestColumns_RowsErr verifies that Columns checks rows.Err() after iterating +// and propagates any error encountered during row iteration back to the caller +// rather than silently returning partial results. +func TestColumns_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + + simulatedErr := fmt.Errorf("rows iteration error") + + rows := sqlmock.NewRows([]string{ + "column_name", "full_type", "data_type", "column_default", + "is_nullable", "is_unique", "is_identity", "is_computed", + }). + AddRow("id", "int", "int", nil, false, true, true, false). + RowError(0, simulatedErr) + + mock.ExpectQuery(`SELECT column_name`). + WithArgs("dbo", "test_table"). + WillReturnRows(rows) + + driver := &MSSQLDriver{conn: db} + _, err = driver.Columns("dbo", "test_table", nil, nil) + if err == nil { + t.Fatal("expected error from rows.Err(), got nil") + } + if err.Error() != simulatedErr.Error() { + t.Errorf("expected error %q, got %q", simulatedErr, err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unfulfilled expectations: %s", err) + } +} diff --git a/drivers/sqlboiler-mysql/driver/mysql.go b/drivers/sqlboiler-mysql/driver/mysql.go index 66371a69..f1299c0e 100644 --- a/drivers/sqlboiler-mysql/driver/mysql.go +++ b/drivers/sqlboiler-mysql/driver/mysql.go @@ -222,6 +222,9 @@ func (m *MySQLDriver) TableNames(schema string, whitelist, blacklist []string) ( } names = append(names, name) } + if err := rows.Err(); err != nil { + return nil, err + } return names, nil } @@ -269,6 +272,9 @@ func (m *MySQLDriver) ViewNames(schema string, whitelist, blacklist []string) ([ names = append(names, name) } + if err := rows.Err(); err != nil { + return nil, err + } return names, nil } @@ -381,6 +387,9 @@ func (m *MySQLDriver) Columns(schema, tableName string, whitelist, blacklist []s columns = append(columns, column) } + if err := rows.Err(); err != nil { + return nil, err + } return columns, nil } diff --git a/drivers/sqlboiler-mysql/driver/mysql_test.go b/drivers/sqlboiler-mysql/driver/mysql_test.go index 813c8b16..1fb7d4ad 100644 --- a/drivers/sqlboiler-mysql/driver/mysql_test.go +++ b/drivers/sqlboiler-mysql/driver/mysql_test.go @@ -18,6 +18,7 @@ import ( "os/exec" "testing" + "github.com/DATA-DOG/go-sqlmock" "github.com/stretchr/testify/require" "github.com/aarondl/sqlboiler/v4/drivers" ) @@ -142,3 +143,84 @@ func TestDriver(t *testing.T) { require.False(t, found, "blacklisted column 'string_three' should not be present in table 'magic'") }) } + +// TestTableNames_RowsErr verifies that TableNames propagates errors returned +// by rows.Err() after the row-iteration loop. A RowError on the first row +// simulates a mid-iteration failure (e.g. a dropped connection) and the test +// asserts that the caller receives the underlying error instead of a nil. +func TestTableNames_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + simulatedErr := fmt.Errorf("simulated row iteration error") + + rows := sqlmock.NewRows([]string{"table_name"}). + AddRow("table1"). + RowError(0, simulatedErr) + + mock.ExpectQuery(`select table_name from information_schema\.tables`). + WithArgs("testschema"). + WillReturnRows(rows) + + m := &MySQLDriver{conn: db} + _, err = m.TableNames("testschema", nil, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "simulated row iteration error") + require.NoError(t, mock.ExpectationsWereMet()) +} + +// TestViewNames_RowsErr verifies that ViewNames propagates errors returned +// by rows.Err() after the row-iteration loop. A RowError on the first row +// simulates a mid-iteration failure and the test asserts that the caller +// receives the underlying error instead of a nil. +func TestViewNames_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + simulatedErr := fmt.Errorf("simulated row iteration error") + + rows := sqlmock.NewRows([]string{"table_name"}). + AddRow("view1"). + RowError(0, simulatedErr) + + mock.ExpectQuery(`select table_name from information_schema\.views`). + WithArgs("testschema"). + WillReturnRows(rows) + + m := &MySQLDriver{conn: db} + _, err = m.ViewNames("testschema", nil, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "simulated row iteration error") + require.NoError(t, mock.ExpectationsWereMet()) +} + +// TestColumns_RowsErr verifies that Columns propagates errors returned by +// rows.Err() after the row-iteration loop. A RowError on the first row +// simulates a mid-iteration failure and the test asserts that the caller +// receives the underlying error instead of a nil. +func TestColumns_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + simulatedErr := fmt.Errorf("simulated row iteration error") + + rows := sqlmock.NewRows([]string{ + "column_name", "column_type", "column_comment", "data_type", + "column_default", "is_nullable", "is_generated", "is_unique", + }). + AddRow("id", "int", "", "int", nil, false, false, true). + RowError(0, simulatedErr) + + mock.ExpectQuery(`select\s+c\.column_name`). + WithArgs("test_table", "test_table", "testschema", "testschema", "testschema", "testschema", "test_table", "test_table", "testschema"). + WillReturnRows(rows) + + m := &MySQLDriver{conn: db} + _, err = m.Columns("testschema", "test_table", nil, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "simulated row iteration error") + require.NoError(t, mock.ExpectationsWereMet()) +} diff --git a/drivers/sqlboiler-psql/driver/psql.go b/drivers/sqlboiler-psql/driver/psql.go index 8b93cfc1..c356399a 100644 --- a/drivers/sqlboiler-psql/driver/psql.go +++ b/drivers/sqlboiler-psql/driver/psql.go @@ -223,6 +223,10 @@ func (p *PostgresDriver) TableNames(schema string, whitelist, blacklist []string names = append(names, name) } + if err := rows.Err(); err != nil { + return nil, err + } + return names, nil } @@ -281,6 +285,10 @@ func (p *PostgresDriver) ViewNames(schema string, whitelist, blacklist []string) names = append(names, name) } + if err := rows.Err(); err != nil { + return nil, err + } + return names, nil } @@ -387,6 +395,11 @@ select * from results; } p.uniqueColumns.Store(c, struct{}{}) } + + if err := rows.Err(); err != nil { + return err + } + return nil } @@ -661,6 +674,10 @@ func (p *PostgresDriver) Columns(schema, tableName string, whitelist, blacklist columns = append(columns, column) } + if err := rows.Err(); err != nil { + return nil, err + } + return columns, nil } diff --git a/drivers/sqlboiler-psql/driver/psql_test.go b/drivers/sqlboiler-psql/driver/psql_test.go index 23445788..5e611047 100644 --- a/drivers/sqlboiler-psql/driver/psql_test.go +++ b/drivers/sqlboiler-psql/driver/psql_test.go @@ -15,8 +15,11 @@ import ( "fmt" "os" "os/exec" + "strings" + "sync" "testing" + "github.com/DATA-DOG/go-sqlmock" "github.com/stretchr/testify/require" "github.com/aarondl/sqlboiler/v4/drivers" @@ -143,3 +146,155 @@ func TestAssemble(t *testing.T) { require.False(t, found, "blacklisted column 'string_three' should not be present in table 'magic'") }) } + +// TestTableNames_RowsErr verifies that TableNames propagates errors surfaced by +// rows.Err() after the iteration loop. Without the rows.Err() check the +// function would silently return a partial (or empty) result set when the +// database connection fails mid-iteration, violating the database/sql contract. +func TestTableNames_RowsErr(t *testing.T) { + t.Parallel() + + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{"table_name"}). + AddRow("table1"). + RowError(0, rowErr) + mock.ExpectQuery(`select table_name from information_schema\.tables`).WillReturnRows(rows) + + p := &PostgresDriver{conn: db} + names, err := p.TableNames("public", nil, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if names != nil { + t.Errorf("expected nil result, got: %v", names) + } + if !strings.Contains(err.Error(), "connection reset by peer") { + t.Errorf("error did not contain expected message, got: %v", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} + +// TestViewNames_RowsErr verifies that ViewNames propagates errors surfaced by +// rows.Err() after the iteration loop. A mid-iteration failure (e.g. a +// connection reset) must be returned to the caller so it does not act on an +// incomplete list of view names. +func TestViewNames_RowsErr(t *testing.T) { + t.Parallel() + + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{"table_name"}). + AddRow("view1"). + RowError(0, rowErr) + mock.ExpectQuery(`select`).WillReturnRows(rows) + + p := &PostgresDriver{conn: db} + names, err := p.ViewNames("public", nil, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if names != nil { + t.Errorf("expected nil result, got: %v", names) + } + if !strings.Contains(err.Error(), "connection reset by peer") { + t.Errorf("error did not contain expected message, got: %v", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} + +// TestLoadUniqueColumns_RowsErr verifies that loadUniqueColumns propagates +// errors surfaced by rows.Err() after the iteration loop. This function +// populates the driver's uniqueColumns cache; a silent mid-iteration failure +// would leave the cache incomplete, causing downstream Columns() calls to +// produce incorrect uniqueness metadata. +func TestLoadUniqueColumns_RowsErr(t *testing.T) { + t.Parallel() + + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{"schema_name", "table_name", "column_name"}). + AddRow("public", "table1", "id"). + RowError(0, rowErr) + mock.ExpectQuery(`with`).WillReturnRows(rows) + + p := &PostgresDriver{conn: db} + err = p.loadUniqueColumns() + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "connection reset by peer") { + t.Errorf("error did not contain expected message, got: %v", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} + +// TestColumns_RowsErr verifies that Columns propagates errors surfaced by +// rows.Err() after the iteration loop. Column metadata drives code generation, +// so a silently truncated result set could produce generated code that is +// missing fields or has wrong types. The test pre-initializes uniqueColumns so +// only the Columns query itself is exercised. +func TestColumns_RowsErr(t *testing.T) { + t.Parallel() + + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{ + "column_name", "column_type", "column_full_type", "udt_name", + "array_type", "domain_name", "column_default", "column_comment", + "is_nullable", "is_generated", "is_identity", + }). + AddRow("id", "integer", "integer", "int4", + nil, nil, nil, "", + false, false, false). + RowError(0, rowErr) + mock.ExpectQuery(`SELECT`).WillReturnRows(rows) + + p := &PostgresDriver{ + conn: db, + uniqueColumns: &sync.Map{}, + } + cols, err := p.Columns("public", "test_table", nil, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if cols != nil { + t.Errorf("expected nil result, got: %v", cols) + } + if !strings.Contains(err.Error(), "connection reset by peer") { + t.Errorf("error did not contain expected message, got: %v", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} diff --git a/drivers/sqlboiler-sqlite3/driver/sqlite3.go b/drivers/sqlboiler-sqlite3/driver/sqlite3.go index 275f5613..56260eee 100644 --- a/drivers/sqlboiler-sqlite3/driver/sqlite3.go +++ b/drivers/sqlboiler-sqlite3/driver/sqlite3.go @@ -172,6 +172,9 @@ func (s SQLiteDriver) TableNames(schema string, whitelist, blacklist []string) ( names = append(names, name) } } + if err := rows.Err(); err != nil { + return nil, err + } return names, nil } @@ -219,6 +222,9 @@ func (s SQLiteDriver) ViewNames(schema string, whitelist, blacklist []string) ([ names = append(names, name) } } + if err := rows.Err(); err != nil { + return nil, err + } return names, nil } @@ -276,6 +282,9 @@ func (s SQLiteDriver) tableInfo(tableName string) ([]*sqliteTableInfo, error) { ret = append(ret, tinfo) } + if err := rows.Err(); err != nil { + return nil, err + } return ret, nil } @@ -306,10 +315,17 @@ func (s SQLiteDriver) indexes(tableName string) ([]*sqliteIndex, error) { } columns = append(columns, colName) } + if err := rowsColumns.Err(); err != nil { + rowsColumns.Close() + return nil, err + } rowsColumns.Close() idx.Columns = columns ret = append(ret, idx) } + if err := rows.Err(); err != nil { + return nil, err + } return ret, nil } diff --git a/drivers/sqlboiler-sqlite3/driver/sqlite3_test.go b/drivers/sqlboiler-sqlite3/driver/sqlite3_test.go index 831da5fc..1161e29a 100644 --- a/drivers/sqlboiler-sqlite3/driver/sqlite3_test.go +++ b/drivers/sqlboiler-sqlite3/driver/sqlite3_test.go @@ -2,6 +2,7 @@ package driver import ( "bytes" + "database/sql/driver" "encoding/json" "flag" "fmt" @@ -12,8 +13,9 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" + "github.com/DATA-DOG/go-sqlmock" "github.com/aarondl/sqlboiler/v4/drivers" + "github.com/stretchr/testify/require" _ "modernc.org/sqlite" ) @@ -88,3 +90,130 @@ func TestDriver(t *testing.T) { } } + +// TestTableNames_RowsErr verifies that TableNames propagates errors surfaced +// by rows.Err() after iterating the result set from sqlite_master. Without the +// rows.Err() check the function would silently return partial results when the +// row iteration is interrupted by a transient error (e.g. a dropped connection). +func TestTableNames_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{"name"}). + AddRow(driver.Value("users")). + RowError(0, rowErr) + mock.ExpectQuery(`SELECT name FROM sqlite_master`).WillReturnRows(rows) + + s := SQLiteDriver{dbConn: db} + _, err = s.TableNames("", nil, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "connection reset by peer") + require.NoError(t, mock.ExpectationsWereMet()) +} + +// TestViewNames_RowsErr verifies that ViewNames propagates errors surfaced +// by rows.Err() after iterating the result set from sqlite_master. This is the +// same class of bug as TableNames: without the check, a mid-iteration error +// would be swallowed and the caller would receive a truncated list of views. +func TestViewNames_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{"name"}). + AddRow(driver.Value("user_view")). + RowError(0, rowErr) + mock.ExpectQuery(`SELECT name FROM sqlite_master`).WillReturnRows(rows) + + s := SQLiteDriver{dbConn: db} + _, err = s.ViewNames("", nil, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "connection reset by peer") + require.NoError(t, mock.ExpectationsWereMet()) +} + +// TestTableInfo_RowsErr verifies that tableInfo propagates errors surfaced by +// rows.Err() after iterating the PRAGMA table_xinfo result set. A missing +// check here would cause the driver to silently return incomplete column +// metadata for a table, leading to incorrect code generation downstream. +func TestTableInfo_RowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{"cid", "name", "type", "notnull", "dflt_value", "pk", "hidden"}). + AddRow(driver.Value(int64(0)), driver.Value("id"), driver.Value("INTEGER"), driver.Value(true), nil, driver.Value(int64(1)), driver.Value(int64(0))). + RowError(0, rowErr) + mock.ExpectQuery(`PRAGMA table_xinfo`).WillReturnRows(rows) + + s := SQLiteDriver{dbConn: db} + _, err = s.tableInfo("users") + require.Error(t, err) + require.Contains(t, err.Error(), "connection reset by peer") + require.NoError(t, mock.ExpectationsWereMet()) +} + +// TestIndexes_OuterRowsErr verifies that the indexes function propagates +// errors surfaced by rows.Err() on the outer PRAGMA index_list loop. The outer +// loop enumerates all indexes for a table; an undetected iteration error would +// cause the driver to return an incomplete set of indexes, which could result +// in incorrect uniqueness metadata on generated columns. +// +// We add two rows and set RowError on row 1 so the first iteration completes +// (triggering the inner PRAGMA index_info query) while the second iteration +// fails, making the error available via rows.Err(). +func TestIndexes_OuterRowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + rowErr := fmt.Errorf("connection reset by peer") + rows := sqlmock.NewRows([]string{"seq", "name", "unique", "origin", "partial"}). + AddRow(driver.Value(int64(0)), driver.Value("idx_users_email"), driver.Value(int64(1)), driver.Value("c"), driver.Value(int64(0))). + AddRow(driver.Value(int64(1)), driver.Value("idx_users_name"), driver.Value(int64(0)), driver.Value("c"), driver.Value(int64(0))). + RowError(1, rowErr) + mock.ExpectQuery(`PRAGMA index_list`).WillReturnRows(rows) + + // The first outer iteration succeeds, so the inner query for index_info + // is issued for the first index. + innerRows := sqlmock.NewRows([]string{"seqno", "cid", "name"}). + AddRow(driver.Value(int64(0)), driver.Value(int64(1)), driver.Value("email")) + mock.ExpectQuery(`PRAGMA index_info`).WillReturnRows(innerRows) + + s := SQLiteDriver{dbConn: db} + _, err = s.indexes("users") + require.Error(t, err) + require.Contains(t, err.Error(), "connection reset by peer") + require.NoError(t, mock.ExpectationsWereMet()) +} + +// TestIndexes_InnerRowsErr verifies that the indexes function propagates +// errors surfaced by rowsColumns.Err() on the inner PRAGMA index_info loop. +// The inner loop retrieves the columns belonging to a specific index; a +// swallowed error here would silently produce an index with missing columns, +// corrupting the uniqueness analysis that depends on column counts. +func TestIndexes_InnerRowsErr(t *testing.T) { + db, mock, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + outerRows := sqlmock.NewRows([]string{"seq", "name", "unique", "origin", "partial"}). + AddRow(driver.Value(int64(0)), driver.Value("idx_users_email"), driver.Value(int64(1)), driver.Value("c"), driver.Value(int64(0))) + mock.ExpectQuery(`PRAGMA index_list`).WillReturnRows(outerRows) + + rowErr := fmt.Errorf("connection reset by peer") + innerRows := sqlmock.NewRows([]string{"seqno", "cid", "name"}). + AddRow(driver.Value(int64(0)), driver.Value(int64(1)), driver.Value("email")). + RowError(0, rowErr) + mock.ExpectQuery(`PRAGMA index_info`).WillReturnRows(innerRows) + + s := SQLiteDriver{dbConn: db} + _, err = s.indexes("users") + require.Error(t, err) + require.Contains(t, err.Error(), "connection reset by peer") + require.NoError(t, mock.ExpectationsWereMet()) +}