proposal: Implement Scanner & Valuer interfaces

[ccakes]

trafficstars

I'm using PostgreSQL to store IP address information and would like to be able to use these types with database/sql.

Would you be open to implementing the Scanner and Valuer interfaces for IP and IPPrefix types? Alternatively, would you be open to PR which did this?

[bradfitz]

UnmarshalText and UnmarshalBinary isn't enough?

[ccakes]

Unfortunately not, unless there is another way I should be doing this?

package main

import (
	"database/sql"
	"fmt"

	_ "github.com/jackc/pgx/v4/stdlib"
	"inet.af/netaddr"
)

func main() {
	db, err := sql.Open("pgx", "host=localhost user=postgres")
	if err != nil {
		panic("error connecting to database")
	}

	var inet netaddr.IPPrefix
	if err := db.QueryRow("SELECT '192.0.2.14/26'::inet").Scan(&inet); err != nil {
		panic(fmt.Sprintf("error scanning value: %v", err))
	}

	fmt.Println("Got value", inet.String())
}

results in

panic: error scanning value: sql: Scan error on column index 0, name "inet": unsupported Scan, storing driver.Value type string into type *netaddr.IPPrefix

goroutine 1 [running]:
main.main()
	inet.go:19 +0x166
exit status 2

[dsnet]

Given that MarshalText and MarshalBinary are fairly standardized methods, perhaps the sql package should be extended to understand them?

[ccakes]

I'd imagine an implementation of Scanner and Valuer inside this package would probably just call those under the hood

[dsnet]

I don't think it's reasonable for the types for every package to implement magic methods for the interfaces from most other packages that exist. That would lead to untenable code bloat. In contrast, I believe that packages should aim to understand a small set of common magic methods.

[jxsl13]

create a type alias for IP/IPPrefix and implement the Valuer interface by creating a method that returns an interface{}, error where interface{} may be of e.g. type string. This can be achieved by calling MarshalText inside of the Value() method, I guess. You may also use MarshalBinary and return a byte slice as that seems to be also allowed to be returned by the Valuer interface.

https://pkg.go.dev/database/sql/driver#Valuer

https://pkg.go.dev/database/sql/driver#Value

[ccakes]

I don't think it's reasonable for the types for every package to implement magic methods for the interfaces from most other packages that exist.

Agree with that, however with database/sql being part of the stdlib I'd argue that the onus is on third-party libraries such as this to support it, and not the other way around.

create a type alias for IP/IPPrefix

👍 Exactly what I've done, it'd be a bit nicer if this library compatible with database/sql so users don't need to do that. I'm happy to send a PR, just want to check the maintainers are receptive first