Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(113)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 7880045: code review 7880045: database/sql: Add time.Time -> string/[]byte/RawBytes c...

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 9 months ago by julienschmidt
Modified:
11 years, 12 months ago
CC:
golang-codereviews
Visibility:
Public.
database/sql: Add time.Time -> string/[]byte/RawBytes conversion This conversion is intentional. It behaves just like fmt.Print(time) since it makes use of the same String() method

Patch Set 1 #

Patch Set 2 : diff -r c76d8e470353 https://code.google.com/p/go #

Patch Set 3 : diff -r c76d8e470353 https://code.google.com/p/go #

Created: 12 years, 9 months ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -0 lines) Patch
M src/pkg/database/sql/convert.go View 1 2 chunks +34 lines, -0 lines 0 comments Download
M src/pkg/database/sql/convert_test.go View 1 3 chunks +3 lines, -0 lines 0 comments Download
Total messages: 9
|
julienschmidt
Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org, golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
12 years, 9 months ago (2013年03月24日 04:57:20 UTC) #1
Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org,
golang-dev@googlegroups.com),
I'd like you to review this change to
https://code.google.com/p/go 
Sign in to reply to this message.
julienschmidt
Alternatively the fmt.Stringer interface could be used to solve this more general. But currently time.Time ...
12 years, 9 months ago (2013年03月24日 04:58:56 UTC) #2
Alternatively the fmt.Stringer interface could be used to solve this more
general. But currently time.Time is the only driver.Value which makes use of it.
Sign in to reply to this message.
rsc
Sorry, but the time for API changes has passed. Please remind us about this CL ...
12 years, 9 months ago (2013年03月25日 20:27:53 UTC) #3
Sorry, but the time for API changes has passed. Please remind us about this
CL after Go 1.1 has been released.
http://golang.org/s/go11freeze
Sign in to reply to this message.
bradfitz
Julien, Now that Go 1.1 is out, did you still want this to go in?
12 years, 7 months ago (2013年06月05日 22:25:28 UTC) #4
Julien,
Now that Go 1.1 is out, did you still want this to go in?
Sign in to reply to this message.
julienschmidt
I am not quite sure. The conversion from time.Time to other types should be added ...
12 years, 7 months ago (2013年06月05日 23:00:22 UTC) #5
I am not quite sure. The conversion from time.Time to other types should be 
added somehow, but this approach lacks the other way from string/[]bytes -> 
time.Time.
Maybe it is better to give the driver control over this formatting via a 
TimeFormatter interface or something like that.
Do you have a better idea?
On Thursday, June 6, 2013 12:25:28 AM UTC+2, Brad Fitzpatrick wrote:
>
> Julien, 
>
> Now that Go 1.1 is out, did you still want this to go in? 
>
>
> https://codereview.appspot.com/7880045/ 
>
Sign in to reply to this message.
rsc
This bothers me. Why is time.Time special?
12 years, 7 months ago (2013年06月10日 17:39:16 UTC) #6
This bothers me. Why is time.Time special?
Sign in to reply to this message.
julienschmidt
On 2013年06月10日 17:39:16, rsc wrote: > This bothers me. Why is time.Time special? According to ...
12 years, 7 months ago (2013年06月10日 19:07:20 UTC) #7
On 2013年06月10日 17:39:16, rsc wrote:
> This bothers me. Why is time.Time special?
According to the SQL standard there are two valid formats for date values:
Without timezone: YYYY-MM-DD HH:MM:SS
With timezone: YYYY-MM-DD HH:MM:SS+offset (E.g. MySQL doesn't support timezones
at all while time.Time is always localized)
But not all DBMS comply with this SQL standard. In fact, the database/sql
package is not even just for SQL databases: "Package sql provides a generic
interface around SQL (or SQL-like) databases."
There a a few open questions:
time.Time -> string/ []byte / RawBytes:
* Which format should be used? The "Go format" (what this CL would do) or the
DBMS specific format? The latter would have the advantage that the output could
be used as a input for another query again.
string etc. -> time.Time:
* How does the database/sql package get the format necessary to parse the
string(-ish) value?
* How should values without timezone information be handled? Should an error be
returned? Should UTC / the local timezone be used?
Sign in to reply to this message.
gobot
Replacing golang-dev with golang-codereviews.
12 years ago (2013年12月20日 16:21:38 UTC) #8
Replacing golang-dev with golang-codereviews.
Sign in to reply to this message.
bradfitz
R=close Feel free to re-open discussion if you're convinced this should go in, but I ...
11 years, 12 months ago (2014年01月14日 21:39:16 UTC) #9
R=close
Feel free to re-open discussion if you're convinced this should go in, but I
think the supporting only time.Time -> time.Time is fine. Then callers can
decide which string format they want and we don't have to decide and document.
Sign in to reply to this message.
|
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /