added ability for callbacks to have time.Time as arguments

[sum12]

go-sqlite3 can store time.Time values, and also has the fetaure to register callback function to provide additionaly functions.

However these callback cannot take the mentioned time.Time columns as input as there is no "dynamic-casting" of these values from TEXT to time.Time.

With this patch that become possible letting user write custom functions to do fancy filtering these time.Time columns

[sum12]

If the features is acceptable then if required I will add tests

[mattn]

please add test.

[sum12]

I have added tests. but I dont understand why the callbacks are failing for struct{}

[coveralls]

Coverage decreased (-0.6%) to 57.348% when pulling 3d52bcbd4e40a903f720860339e2d2fd6b749717 on sum12:timecallback into 3aefd9f0a162514f66d0e4ceda3edc44e66b502e on mattn:master.

[coveralls]

Coverage decreased (-0.7%) to 57.657% when pulling 7791e15be137ea5e584c061ba3ee85f4642a6096 on sum12:timecallback into 31f5bb843b7852e28d5a66d1d539c042c3bb623a on mattn:master.

[sum12]

okay that should do it. Let me know what you think

[sum12]

Any reviews on this one ?

[mattn]

Seems good to me. cc @GJRTimmer

[sum12]

thanks for fixing travis, I have dropped the commit from the PR.

[sum12]

@GJRTimmer do you have any more reviews?

[mattn]

In recent changes, time.Time can be passed correctly. So IMO, this hack is not required. @sum12 Do you think?

[sum12]

Could you please point me to the commit that added this functionality? I looked at callback.go and it doesnot have it.

Or may be you could help me convert this hack to something that is not a hack ?

[rittneje]

@sum12 I do have a few comments on your changes.

1. Please use errors.New over fmt.Errorf if there are no format verbs (like %v).
2. Please remove "an" from the error message.
3. Please use C.GoStringN to account for embedded null bytes.
4. If the input fails to parse to a time.Time, you aren't actually returning an error. Is that the intended behavior? It seems inconsistent to complain about it not being TEXT, but to be fine with it not being an actual time string.
5. What if the input is an integer or float? It could be reasonable to interpret it as a Julian day or Unix timestamp. https://www.sqlite.org/lang_datefunc.html
6. What about blobs? Elsewhere this library tries to treat blobs and strings equivalently. (See callbackArgString.)

[sum12]

@sum12 I do have a few comments on your changes.

1. Please use `errors.New` over `fmt.Errorf` if there are no format verbs (like `%v`).

done

2. Please remove "an" from the error message.

hrm, done.

3. Please use `C.GoStringN` to account for embedded null bytes.

this one was good for learning, thanks.

4. If the input fails to parse to a `time.Time`, you aren't actually returning an error. Is that the intended behavior? It seems inconsistent to complain about it not being TEXT, but to be fine with it not being an actual time string.

make sense, corrected.

5. What if the input is an integer or float? It could be reasonable to interpret it as a Julian day or Unix timestamp. https://www.sqlite.org/lang_datefunc.html

the library does not recognize the other formats, only SQLiteTimestampFormats are recognized. I would love to add support for other formats, but in a different PR, after this.

6. What about blobs? Elsewhere this library tries to treat blobs and strings equivalently. (See `callbackArgString`.)

Accepted and corrected

Thanks for reviews, however I am still learning Go. Do you see anything else not in line ? may be @mattn or @GJRTimmer ?

[mattn]

@rittneje This is acceptable?