From 9263a71b92e84aa34d0e35887d5c23f5a0a21537 Mon Sep 17 00:00:00 2001 From: Ravi Sastry Kadali Date: Sun, 25 Jan 2026 18:29:30 -0800 Subject: database/sql: ensure Null* types have Valid=false when Scan returns error The Scan methods for NullString, NullInt64, NullInt32, NullFloat64, NullBool, and NullTime set Valid=true before calling convertAssign. If convertAssign returns an error, Valid remains true, which creates an inconsistent state where Valid=true but err!=nil. Fix by setting Valid only after successful conversion. Fixes #45662 Change-Id: I855a20abbe517ed017f7c9b8f5603b17bd9d487d Reviewed-on: https://go-review.googlesource.com/c/go/+/739160 Auto-Submit: Sean Liao LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov Reviewed-by: Sean Liao Reviewed-by: Michael Pratt --- src/database/sql/sql_test.go | 149 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) (limited to 'src/database/sql/sql_test.go') diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index e8a6560097..5f093a2d6d 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -5086,3 +5086,152 @@ type unknownInputsValueConverter struct{} func (unknownInputsValueConverter) ConvertValue(v any) (driver.Value, error) { return "string", nil } + +func TestNullTypeScanErrorConsistency(t *testing.T) { + // Issue #45662: Null* types should have Valid=false when Scan returns an error. + // Previously, Valid was set to true before convertAssign was called, + // so if conversion failed, Valid would still be true despite the error. + + tests := []struct { + name string + scanner Scanner + input any + wantErr bool + }{ + { + name: "NullInt32 with invalid input", + scanner: &NullInt32{}, + input: []byte("not_a_number"), + wantErr: true, + }, + { + name: "NullInt64 with invalid input", + scanner: &NullInt64{}, + input: []byte("not_a_number"), + wantErr: true, + }, + { + name: "NullFloat64 with invalid input", + scanner: &NullFloat64{}, + input: []byte("not_a_float"), + wantErr: true, + }, + { + name: "NullBool with invalid input", + scanner: &NullBool{}, + input: []byte("not_a_bool"), + wantErr: true, + }, + // Valid cases should still work + { + name: "NullInt32 with valid input", + scanner: &NullInt32{}, + input: int64(42), + wantErr: false, + }, + { + name: "NullInt64 with valid input", + scanner: &NullInt64{}, + input: int64(42), + wantErr: false, + }, + { + name: "NullFloat64 with valid input", + scanner: &NullFloat64{}, + input: float64(3.14), + wantErr: false, + }, + { + name: "NullBool with valid input", + scanner: &NullBool{}, + input: true, + wantErr: false, + }, + { + name: "NullString with valid input", + scanner: &NullString{}, + input: "hello", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.scanner.Scan(tt.input) + + // Check that error matches expectation + if (err != nil) != tt.wantErr { + t.Errorf("Scan() error = %v, wantErr %v", err, tt.wantErr) + } + + // The key invariant: Valid should be the opposite of whether we got an error + // (assuming non-nil input) + var valid bool + switch s := tt.scanner.(type) { + case *NullInt32: + valid = s.Valid + case *NullInt64: + valid = s.Valid + case *NullFloat64: + valid = s.Valid + case *NullBool: + valid = s.Valid + case *NullString: + valid = s.Valid + case *NullTime: + valid = s.Valid + } + + if err != nil && valid { + t.Errorf("Scan() returned error but Valid=true; want Valid=false when err!=nil") + } + if err == nil && !valid { + t.Errorf("Scan() returned nil error but Valid=false; want Valid=true when err==nil") + } + }) + } +} + +// TestNullTypeScanNil verifies that scanning nil sets Valid=false without error. +func TestNullTypeScanNil(t *testing.T) { + tests := []struct { + name string + scanner Scanner + }{ + {"NullString", &NullString{String: "preset", Valid: true}}, + {"NullInt64", &NullInt64{Int64: 42, Valid: true}}, + {"NullInt32", &NullInt32{Int32: 42, Valid: true}}, + {"NullFloat64", &NullFloat64{Float64: 3.14, Valid: true}}, + {"NullBool", &NullBool{Bool: true, Valid: true}}, + {"NullTime", &NullTime{Time: time.Now(), Valid: true}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.scanner.Scan(nil) + if err != nil { + t.Errorf("Scan(nil) error = %v; want nil", err) + } + + var valid bool + switch s := tt.scanner.(type) { + case *NullString: + valid = s.Valid + case *NullInt64: + valid = s.Valid + case *NullInt32: + valid = s.Valid + case *NullFloat64: + valid = s.Valid + case *NullBool: + valid = s.Valid + case *NullTime: + valid = s.Valid + } + + if valid { + t.Errorf("Scan(nil) left Valid=true; want Valid=false") + } + }) + } +} -- cgit v1.3