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: time: add a Date type #21365

Closed
psankar opened this issue Aug 9, 2017 · 16 comments
Closed

proposal: time: add a Date type #21365

psankar opened this issue Aug 9, 2017 · 16 comments
Labels
FrozenDueToAge Proposal v2 An incompatible library change
Milestone

Comments

@psankar
Copy link

psankar commented Aug 9, 2017

We have a datatype to store time, under time.Time, however, there are plenty of places where we need to store only Date, without having to store Time information, For example: Date of Birth, College opening date, release date, etc.

Many databases provide fields to store Date alone. For example, Postgres has date type https://www.postgresql.org/docs/9.1/static/datatype-datetime.html; Mysql has DATE type https://dev.mysql.com/doc/refman/5.7/en/datetime.html

Javascript has input types for <input type="date"> which shows a datepicker which can be used for Date input. Demo: https://www.w3schools.com/html/tryit.asp?filename=tryhtml_input_date

If a web application has a date input field, the javascript front end can show dates, the database can save dates, but just for the sake of the Golang backend server, we are forced to write a custom Golang type that receives a string from the JS frontend, converts the string into a time.Time struct and then again, trim the time while storing into the database. A js frontend and a db backend can handle Date type natively, while a Golang backend could not.

I propose that we add native support for a date package or a time.Date struct. In case, we choose the latter approach, we need to break the API by removing the existing time.Date() function, which even though called as a "Date" function, accepts "hours, minutes, seconds, nanoseconds and Locations" as input parameters. I propose this for Go 2.

@dmitshur
Copy link
Contributor

dmitshur commented Aug 9, 2017

Can you elaborate on the challenges or problems with defining your own Date struct, one that fits the needs of your project?

For example, in https://godoc.org/github.com/shurcooL/resume/component#Date, I needed a very similar Date type as you described. It has only the precision of a year, and possibly a month.

I was able to implement it as a custom type:

// Date represents an imprecise date.
type Date struct {
	Year  int        // Year. E.g., 2009.
	Month time.Month // Month is 1 - 12. 0 means unspecified.
}

Which worked well for the needs of that project.

It also implements some methods that only my project needed. I doubt that a time.Date would've worked for my specific needs, despite being very close to the description you've provided in nature.

@psankar
Copy link
Author

psankar commented Aug 9, 2017

The only problem with defining my own Date struct is that, I need to define that.

It is a common feature that many will find useful, (like Context or Mux or CookieJar) I would even argue that native support for Date is more important and useful than native support for Complex numbers, which golang supports already natively.

Defining a Date type is also not very trivial. It is easy to miss things or make mistakes, that it would be reliable if it comes from the stdlib. For example, we need to implement support to verify if the date is actually valid (Say flagging errors for: Feb 29 on non-leap-year or January 45th, etc.) We also need to write MarshalJSON, UnmarshalJSON too. The database drivers (pq for postgres for example) could directly use the Date type instead of an approximate time.Time field to store and retrieve the Date data in the database.

@mvdan mvdan changed the title A new datatype for storing Date time: add a Date type Aug 9, 2017
@cznic
Copy link
Contributor

cznic commented Aug 9, 2017

Please explain how's the proposed time.Date type different from the existing time.Time type. AFAICS, they are the same thing, see https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Date.

could directly use the Date type instead of an approximate time.Time

What do you mean by approximate? time.Time's resolution is 1 nanosecond. Javascript's Date has a 1000 times worse resolution - 1 millisecond.

BTW: In a database, you can store the result of time.UTC().UnixNano().

@mvdan
Copy link
Member

mvdan commented Aug 9, 2017

I would even argue that native support for Date is more important and useful than native support for Complex numbers, which golang supports already natively.

Please note that you're mixing the standard library with the language itself. They're different things.

I also don't understand how this would be different or significantly better over the current time.Time.

Marking as Go2, since it would need to break the current time.Date func.

@mvdan mvdan added the v2 An incompatible library change label Aug 9, 2017
@psankar
Copy link
Author

psankar commented Aug 9, 2017

What do you mean by approximate? time.Time's resolution is 1 nanosecond. Javascript's Date has a 1000 times worse resolution - 1 millisecond.

I was wrongly using the word "approximate" (non-native speaker). What I meant is, the driver has to use time.Time for storing a Date sql column. For a TIMESTAMP column, using time.Time is straightforward, but since the standard library does not have a Date type, the sql driver needs to use the same time.Time for both Date and TIMESTAMP/TIME sql columns. By approximate, I meant, a sql type that does not directly map to a golang type.

@mvdan
Copy link
Member

mvdan commented Aug 9, 2017

If your pain lies in SQL, perhaps database/sql (or even a third-party SQL driver package) would be a better place for this?

@psankar
Copy link
Author

psankar commented Aug 9, 2017

If your pain lies in SQL, perhaps database/sql (or even a third-party SQL driver package) would be a better place for this?

It is not only SQL. For example, in Javascript, if I do <input type="date"> then it gives a YYYY-MM-DD as value which I cannot directly convert to a time.Date element (in a HTTP API handler in golang).

For example:

type Employee struct {
 Name string
 DoB time.Date
}

func handler(res http.ResponseWriter, req *http.Request) {
    x := Employee{}
    decoder := json.NewDecoder(req.Body)
    err := decoder.Decode(&x)
}

would be easier to write/read than: Declaring Employee:DoB as time.Time writing a MarshalJSON or UnMarshalJSON for Employee struct where I need to convert the incoming string to a time.Time.

If we declare DoB to be a custom struct, then it adds a new extra dependency, which could be avoided if there is a Date package as part of the standard library.

If a separate Date field will be too heavy for the standard library (because it is felt that it can be trivially implemented as a custom struct), I can close the bug report. I just wanted to make a proposal, in case it may be appealing to others too. Thanks.

@cznic
Copy link
Contributor

cznic commented Aug 9, 2017

For example, in Javascript, if I do then it gives a YYYY-MM-DD as value which I cannot directly convert to a time.Date element (in a HTTP API handler in golang).

But it does not "give you" a YYYY-MM-DD. That's a particular representation of that value, which is just an integer. time.Time is just an integer as well. The two integers in JS Date and time.Time differ by a constant factor of 1000 and it's trivial to go from one of the numbers to the other and vice versa. (Considering above just UTC for the first approximation).

@psankar
Copy link
Author

psankar commented Aug 9, 2017

But it does not "give you" a YYYY-MM-DD.

May be I am not clear by "give you". What I meant is, I can write JS code, where I can set/get the value of a date input field as a string

<input type="date" min="2014-02-09" max="2014-02-11" value="2014-02-10" />
$("#myinput").val("2014-02-10"); // with jquery

Here the value, min, max are all taking a string in YYYY-MM-DD format. I can map this string value to a golang time.Time too (or vice versa) but if there is a golang time.Date (or a custom Date struct), that is what I would prefer to map this YYYY-MM-DD value, instead of a time.Time struct.

@cznic
Copy link
Contributor

cznic commented Aug 9, 2017

I can write JS code, where I can set/get the value of a date input field as a string

And you can write Go code where you can get/set a value of time.Time as a string:

You may have missed the earlier question, but let me repeat

Please explain how's the proposed time.Date type different from the existing time.Time type. AFAICS, they are the same thing, see https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Date.

Yes, the names of methods differ, the layout string is special (and better) in Go and few other minor details. But the functionality and concepts are otherwise the same. Where do you see the difference and/or missing functionality that would warrant considering adding of time.Time? So far everything you wrote can be done with time.Time rather easily.

Here the value, min, max are all taking a string in YYYY-MM-DD format. I can map this string value to a golang time.Time too (or vice versa) but if there is a golang time.Date (or a custom Date struct), that is what I would prefer to map this YYYY-MM-DD value, instead of a time.Time struct.

Checking limits is a one if statement. That's not a good reason to add a new type to the time package. And you may extend time.Time and add this one limits-checking method. It will contain that if statement discussed above.

BTW, working with times/dates as strings is best left to the last mile when inputting/presenting them. Everywhere else the number (wrapped in time.Time) is more convenient and performant.

@rogpeppe
Copy link
Contributor

rogpeppe commented Aug 9, 2017

I agree with the other posters. If you use a canonical form of time.Time where the time zone is always UTC and the time is always midnight, time.Time seems to work fine as a way of storing and parsing dates to me.

You can write a trivial wrapper around it if you like, but that doesn't seem like it would pull its weight in the standard library. For example: https://play.golang.org/p/Szfza9ZAt0
(note that for convenience I used a type alias, available in go 1.9 only)

@neild
Copy link
Contributor

neild commented Aug 9, 2017

See civil.Date in the cloud.google.com/go/civil package for an example of a civil date type:
http://godoc.org/cloud.google.com/go/civil#Date

See also #19700 ("proposal: civil time package").

I recommend the "Fundamental Concepts" section of the cctz library (C++) as a good introduction to the difference between civil and absolute time:
https://github.com/google/cctz#fundamental-concepts

I would strongly recommend against using a time.Time in UTC at midnight as a representation of a civil date or time. A time.Time represents an absolute time and using it to represent civil time is both confusing and error-prone.

@ianlancetaylor
Copy link
Contributor

A civil date package makes sense, but I would rather see it outside of the standard library before trying to bring it in.

@dsnet dsnet changed the title time: add a Date type proposal: time: add a Date type Aug 9, 2017
@dsnet dsnet added the Proposal label Aug 10, 2017
@gopherbot gopherbot added this to the Proposal milestone Aug 10, 2017
@FlorianUekermann
Copy link
Contributor

Since Ian basically asked for examples outside the stdlib. In my opinion the date type in the mentioned http://godoc.org/cloud.google.com/go/civil#Date is heavy and awkward.

See https://godoc.org/github.com/rickb777/date, https://godoc.org/github.com/RangelReale/epochdate or https://godoc.org/github.com/infobaleen/date for variations of a lighter approach. The second and the last one are good minimal examples, but rickb777's approach of wrapping it in a struct can protect against some accidents. I wrote the last one, so I may be biased.

@skabbes
Copy link

skabbes commented Oct 3, 2017

I agree wholeheartedly with @neild that instants in time (time.Time) are very different concepts from a civil Date. It it obvious that they need different types, the question is whether to include it in the stdlib.

I think for me, the problem comes down to documentation. Any reasonably experienced programmer would look at a timezone-aware "instant in time" library and have serious doubts if it could correctly implement a civil Date. Even if they were to dig into the code, nothing in there suggests that that is a feature. I think most gophers are going to not dig in any further and look for a library or write their own.

I think we can either document this feature of the time package in prose, or in code (as a time.Date type). In my opinion both accomplish the task equally well since implementing civil.Date is trivial if you know time.Time supports it, if you do not it is fairly daunting. This actually seems to be the dividing line between people in this proposal as well.

@ianlancetaylor
Copy link
Contributor

I think the ideas here are best covered by the ideas in #19700. Closing in favor of that one.

@golang golang locked and limited conversation to collaborators Mar 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests