Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database/sql: Inconsistent Behavior with Custom Null Types #66621

Closed
t1mqa opened this issue Mar 31, 2024 · 2 comments
Closed

database/sql: Inconsistent Behavior with Custom Null Types #66621

t1mqa opened this issue Mar 31, 2024 · 2 comments

Comments

@t1mqa
Copy link

t1mqa commented Mar 31, 2024

Go version

1.22

Output of go env in your module/workspace:

set GO111MODULE=on
set GOARCH=amd64                                 
set GOBIN=                                       
set GOCACHE=C:\Users\T1mQa\AppData\Local\go-build
set GOENV=C:\Users\T1mQa\AppData\Roaming\go\env  
set GOEXE=.exe                                   
set GOEXPERIMENT=                                
set GOFLAGS=                                     
set GOHOSTARCH=amd64                             
set GOHOSTOS=windows                             
set GOINSECURE=                                  
set GOMODCACHE=C:\Users\T1mQa\go\pkg\mod         
set GONOPROXY=                                   
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\T1mQa\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:/Program Files/Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.1
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\T1mQa\GolandProjects\MAFIA\regBot\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\Temp\go-build2127611667=/tmp/go-build -gno-record-gcc-switches

What did you do?

The Go 1.22 release introduced support for custom Null Types within the database/sql package. While implementing my own custom Null Types, I've encountered a consistent issue across various database/sql functions, such as ExecContext, QueryRowContext, and others, where custom Null Types do not behave as expected.

Code Sample:

// Custom Null Type definition
type SQLNickname struct {
    String string
    Valid  bool
}

func (n *SQLNickname) Scan(value any) error {
    // Implementation of Scan method
}

func (n *SQLNickname) Value() (driver.Value, error) {
    // Implementation of Value method
}

// Using the custom type as an argument directly results in an error
_, err = tx.ExecContext(ctx, query, sqlUser.TelegramID, sqlUser.Nickname, sqlUser.State, sqlUser.IsAdmin)
// Error encountered: sql: converting argument $2 type: unsupported type sqlModels.SQLNickname, a struct

Suggestion for Improvement:
Adjust the handling of custom Null Types in the database/sql package functions to ensure that they are treated on par with built-in Null Types, providing a consistent interface for developers.

What did you see happen?

Custom Null Types trigger an unsupported type error when used directly as arguments in database/sql functions. However, manually invoking the Value() method on the custom type and passing the returned value does not result in an error, suggesting that the interface is implemented correctly. Despite this, such an approach does not align semantically with how built-in Null Types are handled, leading to inconsistent and inconvenient code practices.

What did you expect to see?

It is expected that custom Null Types will seamlessly integrate with database/sql operations, just like built-in types such as sql.NullInt64, given that they implement the Valuer interface.

UPDATE (resolve):
Hey, if you see this issue, ure probably thinking "whats wrong with Valuer". All good..)
Check if your Value method is non-pointer.
Thats how it SHOULD looks in my code:

func (n *SQLNickname) Scan(value any) error {
	if value == nil {
		n.String, n.Valid = "", false
		return nil
	}

	switch v := value.(type) {
	case string:
		n.String, n.Valid = v, true
	case []byte:
		n.String, n.Valid = string(v), true
	default:
		return errors.New("incompatible type for Nickname")
	}
	return nil

}

func (n SQLNickname) Value() (driver.Value, error) {
	if !n.Valid || n.String == "" {
		return nil, nil
	}
	return n.String, nil
}
@t1mqa
Copy link
Author

t1mqa commented Mar 31, 2024

For now I'm using:

telegramId, _ := sqlUser.TelegramID.Value()
nickName, _ := sqlUser.Nickname.Value()
state, _ := sqlUser.State.Value()
_, err = tx.ExecContext(ctx, query, telegramId, nickName, state, sqlUser.IsAdmin)

And I don't really like it, especially since I can use standard types without invoking Value().

@seankhliao
Copy link
Member

database/sql specifically introduced the database/sql.Null[T[ type to wrap your own type, not for you to create a custom nullable type.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants