proposal: database/sql: add NullTime support

[isakbm]

Would it be an idea to extend the Nullable types to include NullTime as well?

Something like

type NullTime struct {
    Time  time.Time
    Valid bool
}

// Scan implements the Scanner interface.
func (nt *NullTime) Scan(value interface{}) error {
    if value == nil {
        nt.Time, nt.Valid = time.Time{}, false
        return nil
    }
    nt.Valid = true
    return convertAssign(&nt.Time, value)
}

// Value implements the Valuer interface.
func (nt NullTime) Valuer() (driver.Value, error) {
    if !nt.Valid {
        return nil, nil
    }
    return nt.Time, nil
}

Just copy paste into ...go/src/database/sql/sql.go, should work.

[agnivade]

This seems to be taken from https://godoc.org/github.com/lib/pq#NullTime ?

[isakbm]

@agnivade

That's interesting, looks similar, but I was just mimicking the way NullString is handled in go/src/database/sql/sql.go

See this line

pasted here for ease :

// from go/src/database/sql/sql.go

type NullString struct {
	String string
	Valid  bool // Valid is true if String is not NULL
}

// Scan implements the Scanner interface.
func (ns *NullString) Scan(value interface{}) error {
	if value == nil {
		ns.String, ns.Valid = "", false
		return nil
	}
	ns.Valid = true
	return convertAssign(&ns.String, value)
}

// Value implements the driver Valuer interface.
func (ns NullString) Value() (driver.Value, error) {
	if !ns.Valid {
		return nil, nil
	}
	return ns.String, nil
}

The implementation in pq is different, note how in pq there is no call to convertAssign.

[mattn]

convertAssign will need to look some possible time format for each RDBMS's serialization formats like below.

https://github.com/mattn/go-nulltype/blob/3e10e98e82afdeaf9510344bc71d4d9b8eea788f/time.go#L45-L56

[isakbm]

@mattn

convertAssign already supports time.Time

see

go/src/database/sql/convert.go

Line 268 in a10b4cf

case time.Time:

[mattn]

Yes that is a part of code that driver return time.Time. I mean case that driver return string instead of time.Time. For example, NullString is possible to convert number to string. If add NullTime, most of users expect convertion between string and time.Time. But some RDBMS does not return time.Time alawys. For example select current_timestamp on SQLite3 return string. Some of database driver return Time and Date separatedly. I guess that adding NullTime into standard package is hard to do.

[isakbm]

@mattn

I see! Will have to look into this deeper. Very good point!

[agnivade]

/cc @kardianos

[beoran]

@mattn While that is true, the current situation is that NullTime is now defined separately for various databases such as postgresql, sqlite, etc, in packages all over the place. Having a NullTime in database/sql would greatly simplify this. What we need to is a mechanism to also allow parsing strings and separate fields as a time. It's then up to the driver writers contribute to the conversion what the driver returns to this type. Like this example for sqlite3: https://github.com/xo/xoutil/blob/master/xoutil.go.

[rsc]

@kardianos, any thoughts? It seems fine to me.

[gopherbot]

Change https://golang.org/cl/170699 mentions this issue: database/sql: add NullTime