Add support for godoc.org/cloud.google.com/go/civil

[pjebs]

It would be good if this package could automatically scan to

civil.Time (https://godoc.org/cloud.google.com/go/civil#Time) time data type
civil.Date (https://godoc.org/cloud.google.com/go/civil#Date) Date data type

[methane]

Why civil is so special, as standard library?

[renthraysk]

In mysql, time can be negative whereas civil.Time can't represent that.

[pjebs]

All the fields in Time are signed ints.

[renthraysk]

Documentation specifies valid range of values, and all are [0,...] , also other methods like civil.Time String() produce nonsense "-12:-10:00".

mysql's time is more suited scanning to a time.Duration.

[pjebs]

Perhaps mysql package could introduce a new struct which wraps civil.Time with a negative field and also implements Stringer

[pjebs]

package mysql

type Time struct {
Negative bool
Time *civil.Time
}

[methane]

How many other DB drivers support it?
Why don't you wrap it and implement Scanner by yourself?
When there are N time packages in the world, should we support all of them?
If no, why civil is so special?

[pjebs]

For one thing it's by google. Secondly, it's very popular - even if it's not used with mysql or databases per se.

Perhaps this package should basically incorporate it inside. The underlying issue raised (although decorated as a request specific to the civil package) is to incorporate a nice struct for mysql's date and time data types. The civil package is good inspiration on how it should and could be done.

[methane]

For one thing it's by google.

It's not enough reason, unless it is released on golang.org/x/. (see golang/go#17244).

Secondly, it's very popular - even if it's not used with mysql or databases per se.

very? how very?

The underlying issue raised (although decorated as a request specific to the civil package) is to incorporate a nice struct for mysql's date and time data types.

Since MySQL supports invalid date when configured to support it,
I think string is the best type when one need to support all of them.

[methane]

Anyway, we need example code explains what "support" means.
Sometime, it's simply impossible because of database/sql design.

Our goal is providing driver for database/sql. We don't support MySQL features / edge cases
which doesn't fit database/sql.

[renthraysk]

denisenkom/go-mssqldb uses civil types in go1.9+

https://github.com/denisenkom/go-mssqldb/search?q=civil&unscoped_q=civil
Though only for arguments seemingly, doesn't scan into them.

[methane]

Hmm, mssql added it because of this issue: denisenkom/go-mssqldb#371

Does this project have same issue?

[pjebs]

Civil supports invalid dates

[pjebs]

There are 2 parts to proposal:

• Accept civil.date and civil.time for placeholder args
• allow civil.date and civil.time to scan

[methane]

It's not important than mssql's case because you can use string.
Additionally, negative time support you proposed seems ugly.
I don't like it.

See denisenkom/go-mssqldb#380. I'm OK to add API to register
3rd party types to allow you pass civil types directly.
But it's after we support connecter interface.

https://github.com/denisenkom/go-mssqldb/pull/381/files#diff-04c6e90faac2675aa89e2176d2eec7d8R153

[pjebs]

@julienschmidt Not sure why methane closed it and made a decision so quickly and unilaterally. Maybe you can over turn him.

[methane]

Not sure why methane closed it and made a decision so quickly and unilaterally.

• You said "For one thing it's by google., but it's from GCP team, not Go team. It's far from "semi-standard" or "de-facto standard".

• You said "Secondly, it's very popular". But see dependency graph.

• You said "if this package could automatically scan to..." and "go-mssqldb uses civil types in go1.9+". But mssql doesn't automatically scan to civil types yet.

• mssql supports civil types for parameters for solve their own issue. It's not for support invalid date or time. It's not because civil is semi-standard. So it's not good reason for us. But you didn't explain it.

• Additionally, mssql will drop native civil support and going to pluggable approach.

• civil package is not developed for support invalid datetime.

---

When considered these, I think all you said is very biased and none of them can rationalize to add native support for civil.

Less dependency is strength of this package.
I didn't against pluggable type support in the future.
And if civil is merged into stdlib, I'm OK to support it natively.

If you really want change my conclusion, please write more polite proposal.
All your messages impress me this issue is just a random ideas without enough survey.

[pjebs]

@julienschmidt (sorry had to tag you again to bypass @methane)
golang/go#19700
golang/go#22697 (comment)

[methane]

If civil is merged into golang.org/x/time/civil, I'm OK to support it.
But I think database/sql should support it in the future.