Handle TimeStamp columns by converting int64 to time.Time and vice versa

[echarlus]

Since one cannot yet replace the driver.DefaultParameterConverter to properly handle type conversions (see https://github.com/golang/go/issues/18415) I've made a quick patch to automatically handle TimeStamp (int64) columns conversion to time.Time by using the column type provided by Crate within the query’s results and to convert back to int64 any time.

[sourcelevel-bot[bot]]

Hello, @echarlus! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

[herenow]

If we changed the output/parsing of time columns from int64 to time.Time, it would cause breaking changes.

Although it would be nice to parse time.Time to int64 when receiving arguments.

Let me give this some thought.

[echarlus]

Hello, the code I provided also converts time.Time found in query arguments to int64 so that Time objects can be transparently used by the upper layers. I'm not sure I'm following what you mean by "If we changed the output/parsing of time columns from int64 to time.Time, it would cause breaking changes." could you please elaborate ? Thanks !

[herenow]

Wouldn't this change the value we are assigning to each row column?

https://golang.org/pkg/database/sql/#Rows.Scan

So if we did:

// create table users ( created_at timestmap );
rows, err := db.Query("select created_at from users limit 1")

for rows.Next() {
	var created_at time.Time

	// Now scan, expects a time.Time destination value
	rows.Scan(&created_at)
}

From what I understood, this PR changes what we are scanning into the destination we pass into rows.Scan(), so if the destination is an int64, it would fail when we tried to assign a time.Time into it, wouldn't it?

Maybe I'm confused, and got this all wrong, I haven't worked on this for a while.

I'll give this code a test.

[echarlus]

Since the conversion from int64 to time.Time is performed in the query method (which is called by Query) the row type would indeed by time.Time and your sample should work as expected. So I believe that my change does not break the Scan function. Let me know what your findings show ... Thanks !

[sourcelevel-bot[bot]]

Ebert has finished reviewing this Pull Request and has found:

• 2 possible new issues (including those that may have been commented here).

But beware that this branch is 23 commits behind the herenow:master branch, and a review of an up to date branch would produce more accurate results.

You can see more details about this review at https://ebertapp.io/github/herenow/go-crate/pulls/15.

[sourcelevel-bot[bot]]

SourceLevel has finished reviewing this Pull Request and has found:

• 6 possible new issues (including those that may have been commented here).
• 2 fixed issues! 🎉

But beware that this branch is 27 commits behind the herenow:master branch, and a review of an up to date branch would produce more accurate results.

See more details about this review.