Skip to content

Commit 5477c20

Browse files
committed
fix(drivers): check rows.Err() after Next() loops in all drivers
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)
1 parent 336392b commit 5477c20

8 files changed

Lines changed: 528 additions & 1 deletion

File tree

drivers/sqlboiler-mssql/driver/mssql.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,10 @@ func (m *MSSQLDriver) TableNames(schema string, whitelist, blacklist []string) (
193193
names = append(names, name)
194194
}
195195

196+
if err := rows.Err(); err != nil {
197+
return nil, err
198+
}
199+
196200
return names, nil
197201
}
198202

@@ -240,6 +244,10 @@ func (m *MSSQLDriver) ViewNames(schema string, whitelist, blacklist []string) ([
240244
names = append(names, name)
241245
}
242246

247+
if err := rows.Err(); err != nil {
248+
return nil, err
249+
}
250+
243251
return names, nil
244252
}
245253

@@ -359,6 +367,10 @@ func (m *MSSQLDriver) Columns(schema, tableName string, whitelist, blacklist []s
359367
columns = append(columns, column)
360368
}
361369

370+
if err := rows.Err(); err != nil {
371+
return nil, err
372+
}
373+
362374
return columns, nil
363375
}
364376

drivers/sqlboiler-mssql/driver/mssql_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ import (
2626
"bytes"
2727
"encoding/json"
2828
"flag"
29+
"fmt"
2930
"os"
3031
"os/exec"
3132
"regexp"
3233
"testing"
3334

35+
"github.com/DATA-DOG/go-sqlmock"
3436
"github.com/aarondl/sqlboiler/v4/drivers"
3537
)
3638

@@ -139,3 +141,108 @@ func TestDriver(t *testing.T) {
139141
}
140142
})
141143
}
144+
145+
// TestTableNames_RowsErr verifies that TableNames checks rows.Err() after
146+
// iterating and propagates any error encountered during row iteration back
147+
// to the caller rather than silently returning partial results.
148+
func TestTableNames_RowsErr(t *testing.T) {
149+
db, mock, err := sqlmock.New()
150+
if err != nil {
151+
t.Fatal(err)
152+
}
153+
defer db.Close()
154+
155+
simulatedErr := fmt.Errorf("rows iteration error")
156+
157+
rows := sqlmock.NewRows([]string{"table_name"}).
158+
AddRow("table1").
159+
RowError(0, simulatedErr)
160+
161+
mock.ExpectQuery(`SELECT table_name`).
162+
WithArgs("dbo").
163+
WillReturnRows(rows)
164+
165+
driver := &MSSQLDriver{conn: db}
166+
_, err = driver.TableNames("dbo", nil, nil)
167+
if err == nil {
168+
t.Fatal("expected error from rows.Err(), got nil")
169+
}
170+
if err.Error() != simulatedErr.Error() {
171+
t.Errorf("expected error %q, got %q", simulatedErr, err)
172+
}
173+
174+
if err := mock.ExpectationsWereMet(); err != nil {
175+
t.Errorf("unfulfilled expectations: %s", err)
176+
}
177+
}
178+
179+
// TestViewNames_RowsErr verifies that ViewNames checks rows.Err() after
180+
// iterating and propagates any error encountered during row iteration back
181+
// to the caller rather than silently returning partial results.
182+
func TestViewNames_RowsErr(t *testing.T) {
183+
db, mock, err := sqlmock.New()
184+
if err != nil {
185+
t.Fatal(err)
186+
}
187+
defer db.Close()
188+
189+
simulatedErr := fmt.Errorf("rows iteration error")
190+
191+
rows := sqlmock.NewRows([]string{"table_name"}).
192+
AddRow("view1").
193+
RowError(0, simulatedErr)
194+
195+
mock.ExpectQuery(`select table_name`).
196+
WithArgs("dbo").
197+
WillReturnRows(rows)
198+
199+
driver := &MSSQLDriver{conn: db}
200+
_, err = driver.ViewNames("dbo", nil, nil)
201+
if err == nil {
202+
t.Fatal("expected error from rows.Err(), got nil")
203+
}
204+
if err.Error() != simulatedErr.Error() {
205+
t.Errorf("expected error %q, got %q", simulatedErr, err)
206+
}
207+
208+
if err := mock.ExpectationsWereMet(); err != nil {
209+
t.Errorf("unfulfilled expectations: %s", err)
210+
}
211+
}
212+
213+
// TestColumns_RowsErr verifies that Columns checks rows.Err() after iterating
214+
// and propagates any error encountered during row iteration back to the caller
215+
// rather than silently returning partial results.
216+
func TestColumns_RowsErr(t *testing.T) {
217+
db, mock, err := sqlmock.New()
218+
if err != nil {
219+
t.Fatal(err)
220+
}
221+
defer db.Close()
222+
223+
simulatedErr := fmt.Errorf("rows iteration error")
224+
225+
rows := sqlmock.NewRows([]string{
226+
"column_name", "full_type", "data_type", "column_default",
227+
"is_nullable", "is_unique", "is_identity", "is_computed",
228+
}).
229+
AddRow("id", "int", "int", nil, false, true, true, false).
230+
RowError(0, simulatedErr)
231+
232+
mock.ExpectQuery(`SELECT column_name`).
233+
WithArgs("dbo", "test_table").
234+
WillReturnRows(rows)
235+
236+
driver := &MSSQLDriver{conn: db}
237+
_, err = driver.Columns("dbo", "test_table", nil, nil)
238+
if err == nil {
239+
t.Fatal("expected error from rows.Err(), got nil")
240+
}
241+
if err.Error() != simulatedErr.Error() {
242+
t.Errorf("expected error %q, got %q", simulatedErr, err)
243+
}
244+
245+
if err := mock.ExpectationsWereMet(); err != nil {
246+
t.Errorf("unfulfilled expectations: %s", err)
247+
}
248+
}

drivers/sqlboiler-mysql/driver/mysql.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ func (m *MySQLDriver) TableNames(schema string, whitelist, blacklist []string) (
222222
}
223223
names = append(names, name)
224224
}
225+
if err := rows.Err(); err != nil {
226+
return nil, err
227+
}
225228

226229
return names, nil
227230
}
@@ -269,6 +272,9 @@ func (m *MySQLDriver) ViewNames(schema string, whitelist, blacklist []string) ([
269272

270273
names = append(names, name)
271274
}
275+
if err := rows.Err(); err != nil {
276+
return nil, err
277+
}
272278

273279
return names, nil
274280
}
@@ -381,6 +387,9 @@ func (m *MySQLDriver) Columns(schema, tableName string, whitelist, blacklist []s
381387

382388
columns = append(columns, column)
383389
}
390+
if err := rows.Err(); err != nil {
391+
return nil, err
392+
}
384393

385394
return columns, nil
386395
}

drivers/sqlboiler-mysql/driver/mysql_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"os/exec"
1919
"testing"
2020

21+
"github.com/DATA-DOG/go-sqlmock"
2122
"github.com/stretchr/testify/require"
2223
"github.com/aarondl/sqlboiler/v4/drivers"
2324
)
@@ -142,3 +143,84 @@ func TestDriver(t *testing.T) {
142143
require.False(t, found, "blacklisted column 'string_three' should not be present in table 'magic'")
143144
})
144145
}
146+
147+
// TestTableNames_RowsErr verifies that TableNames propagates errors returned
148+
// by rows.Err() after the row-iteration loop. A RowError on the first row
149+
// simulates a mid-iteration failure (e.g. a dropped connection) and the test
150+
// asserts that the caller receives the underlying error instead of a nil.
151+
func TestTableNames_RowsErr(t *testing.T) {
152+
db, mock, err := sqlmock.New()
153+
require.NoError(t, err)
154+
defer db.Close()
155+
156+
simulatedErr := fmt.Errorf("simulated row iteration error")
157+
158+
rows := sqlmock.NewRows([]string{"table_name"}).
159+
AddRow("table1").
160+
RowError(0, simulatedErr)
161+
162+
mock.ExpectQuery(`select table_name from information_schema\.tables`).
163+
WithArgs("testschema").
164+
WillReturnRows(rows)
165+
166+
m := &MySQLDriver{conn: db}
167+
_, err = m.TableNames("testschema", nil, nil)
168+
require.Error(t, err)
169+
require.Contains(t, err.Error(), "simulated row iteration error")
170+
require.NoError(t, mock.ExpectationsWereMet())
171+
}
172+
173+
// TestViewNames_RowsErr verifies that ViewNames propagates errors returned
174+
// by rows.Err() after the row-iteration loop. A RowError on the first row
175+
// simulates a mid-iteration failure and the test asserts that the caller
176+
// receives the underlying error instead of a nil.
177+
func TestViewNames_RowsErr(t *testing.T) {
178+
db, mock, err := sqlmock.New()
179+
require.NoError(t, err)
180+
defer db.Close()
181+
182+
simulatedErr := fmt.Errorf("simulated row iteration error")
183+
184+
rows := sqlmock.NewRows([]string{"table_name"}).
185+
AddRow("view1").
186+
RowError(0, simulatedErr)
187+
188+
mock.ExpectQuery(`select table_name from information_schema\.views`).
189+
WithArgs("testschema").
190+
WillReturnRows(rows)
191+
192+
m := &MySQLDriver{conn: db}
193+
_, err = m.ViewNames("testschema", nil, nil)
194+
require.Error(t, err)
195+
require.Contains(t, err.Error(), "simulated row iteration error")
196+
require.NoError(t, mock.ExpectationsWereMet())
197+
}
198+
199+
// TestColumns_RowsErr verifies that Columns propagates errors returned by
200+
// rows.Err() after the row-iteration loop. A RowError on the first row
201+
// simulates a mid-iteration failure and the test asserts that the caller
202+
// receives the underlying error instead of a nil.
203+
func TestColumns_RowsErr(t *testing.T) {
204+
db, mock, err := sqlmock.New()
205+
require.NoError(t, err)
206+
defer db.Close()
207+
208+
simulatedErr := fmt.Errorf("simulated row iteration error")
209+
210+
rows := sqlmock.NewRows([]string{
211+
"column_name", "column_type", "column_comment", "data_type",
212+
"column_default", "is_nullable", "is_generated", "is_unique",
213+
}).
214+
AddRow("id", "int", "", "int", nil, false, false, true).
215+
RowError(0, simulatedErr)
216+
217+
mock.ExpectQuery(`select\s+c\.column_name`).
218+
WithArgs("test_table", "test_table", "testschema", "testschema", "testschema", "testschema", "test_table", "test_table", "testschema").
219+
WillReturnRows(rows)
220+
221+
m := &MySQLDriver{conn: db}
222+
_, err = m.Columns("testschema", "test_table", nil, nil)
223+
require.Error(t, err)
224+
require.Contains(t, err.Error(), "simulated row iteration error")
225+
require.NoError(t, mock.ExpectationsWereMet())
226+
}

drivers/sqlboiler-psql/driver/psql.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,10 @@ func (p *PostgresDriver) TableNames(schema string, whitelist, blacklist []string
223223
names = append(names, name)
224224
}
225225

226+
if err := rows.Err(); err != nil {
227+
return nil, err
228+
}
229+
226230
return names, nil
227231
}
228232

@@ -281,6 +285,10 @@ func (p *PostgresDriver) ViewNames(schema string, whitelist, blacklist []string)
281285
names = append(names, name)
282286
}
283287

288+
if err := rows.Err(); err != nil {
289+
return nil, err
290+
}
291+
284292
return names, nil
285293
}
286294

@@ -387,6 +395,11 @@ select * from results;
387395
}
388396
p.uniqueColumns.Store(c, struct{}{})
389397
}
398+
399+
if err := rows.Err(); err != nil {
400+
return err
401+
}
402+
390403
return nil
391404
}
392405

@@ -661,6 +674,10 @@ func (p *PostgresDriver) Columns(schema, tableName string, whitelist, blacklist
661674
columns = append(columns, column)
662675
}
663676

677+
if err := rows.Err(); err != nil {
678+
return nil, err
679+
}
680+
664681
return columns, nil
665682
}
666683

0 commit comments

Comments
 (0)