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

proposal: database/sql: support more nullable types #31231

Closed
mcandre opened this issue Apr 3, 2019 · 15 comments
Closed

proposal: database/sql: support more nullable types #31231

mcandre opened this issue Apr 3, 2019 · 15 comments

Comments

@mcandre
Copy link

mcandre commented Apr 3, 2019

I'm not entirely sure why the database/sql package requires using special types to represent nullable fields, as opposed to plain old pointer types. Why don't all of the built-in Go types just feature sql.Scanner implementations, including arbitrarily deep pointers?

In any case, the API here is limited to a paltry set of types, compared to the full set of built-in types actually available to Go. We are missing nullable:

  • Unsigned integers
  • Non-64 bit width integers
  • Timestamps
  • Runes
  • Structs (esp. for JSON SQL columns)

Sure, we can probably skip over complex numbers due to SQL generally not supporting complex numbers as standard types. But the aforementioned types are very common to use, so it would be better to round out the halfway marshaling and actually support these. As of now, users have to write a lot of custom code to manually marshal and truncate these types, with plenty of risk for error compared to built-in API's.

@andybons andybons changed the title database/sql missing most nullable types proposal: database/sql: support more nullable types Apr 3, 2019
@gopherbot gopherbot added this to the Proposal milestone Apr 3, 2019
@andybons
Copy link
Member

andybons commented Apr 3, 2019

@kardianos
Copy link
Contributor

  • Unsigned integers in SQL databases are rare / often not supported.
  • NullTime proposal is accepted I need to implement.
  • Runes.... I don't think we need to support this, I'd want a use case.
  • Structs... That kinda lands in the land of database specific implementations. Though I'd be happy to know of a specific use case.

I'd like to make a better API for passing values out and I've sketched some ideas out, but nothing final yet.

If you want to make a case for having a NullRune type or if you want to do a survey of databases that support complex structures like JSON, please do so.

I have other strategies I used to avoid these problems in general, so feedback on this area is great. Thanks!

@rsc
Copy link
Contributor

rsc commented Apr 10, 2019

NullTime is on its way. The others seem very specialized. I don't even know how we would handle all structs.

Note that for an arbitrary type T, the way to write sql.NullT is *T (pointer to T).

We can leave this on hold for generics but probably the answer is just use a pointer type. Those are nullable automatically.

@cespare
Copy link
Contributor

cespare commented Apr 10, 2019

@kardianos the main internally-defined sql.Scanner/driver.Value type we use all over the place is NullInt32. This is because we want to use a 32-bit integer in our code for IDs which are 32 bits in the DB. So (in postgres):

integer NOT NULL -> int32
bigint NOT NULL -> int64
integer -> NullInt32
bigint -> NullInt64

(Naturally we prefer NullInt32 to turning a bunch of small integers into pointers.)

@gopherbot
Copy link

Change https://golang.org/cl/174178 mentions this issue: database/sql: add NullInt32

@kardianos
Copy link
Contributor

I mailed a CL to support int32. I don't see any other nullable types being added as they are either specific to the database or uncommon. Int32 (INT or INTEGER) is commonly used in many databases and database schemas today.

gopherbot pushed a commit that referenced this issue Apr 26, 2019
It is common for database integers to be represented as int32
internally. Although NullInt64 is already defined,
this should remove some type casts and make working with those eaiser.

For #31231

Change-Id: Ia0c37ecef035fee0734c1d1fb6f58aef6905cf5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/174178
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@kardianos
Copy link
Contributor

I've closed this issue. I really can't see adding any other Nullable types in the Go std lib. If you think differently please open up a new issue showing how it will be useful on a type by type basis.

@mei-rune
Copy link

why not add Nullable?

type Nullable struct {
	Value interface{}
	Valid bool
}

func (s *Nullable) Scan(src interface{}) error {
	if src == nil {
	        s.Valid = false
		return nil
	}
	s.Valid = true
	return convert.ConvertAssign(s.Value, src)
}

@nicola-spb
Copy link

@kardianos Hello. Why Value() in NullInt32 returns int64????? It's really strange.

@kardianos
Copy link
Contributor

@nicola-spb As far as I can tell, it doesn't. Can you show your code or reproduce the issue? Or even link to the database/sql code that you are talking about?

@nicola-spb
Copy link

nicola-spb commented Sep 8, 2019

@kardianos golang 1.13 2177bfb#diff-5b3d7257e10f4efeecb9517a1b6d0c28R260

I use it like this

func ToInt32(number sql.NullInt32) *int32 {
	var res int32
	if n, _ := number.Value(); n != nil {
		res = n.(int32)
		return &res
	} else {
		return nil
	}
}

and wrap results from database

tWrapper := &TWrapper{
	Id:        wrappers.ToInt32(t.Id),
}

@kardianos
Copy link
Contributor

Don't use value method, that is for driver. Use field.

@nicola-spb
Copy link

I use sqlx. What is field? Can you give example please?

@kardianos
Copy link
Contributor

Use https://godoc.org/database/sql#NullInt32.Int32

The field Int32 in the NullInt32 struct.

@nicola-spb
Copy link

Ok. Thank you.

P.S. It's better of course. I rewrote functions.

@golang golang locked and limited conversation to collaborators Sep 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants