Support returning any from callbacks #1046
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1046 +/- ##
==========================================
+ Coverage 46.04% 46.16% +0.12%
==========================================
Files 11 11
Lines 1490 1499 +9
==========================================
+ Hits 686 692 +6
Misses 667 667
- Partials 137 140 +3
Continue to review full report at Codecov.
|
|
|
||
| if typ.NumMethod() == 0 { | ||
| return callbackRetAny, nil | ||
| } |
There was a problem hiding this comment.
I added a test for this part. But I'm not sure how to add a unit test for the callbackRetAny (I tested it manually end-to-end).
There was a problem hiding this comment.
You can write a test that registers and uses such an aggregator, similar to this test.
There was a problem hiding this comment.
Perfect, thank you!
Add test Fix test
callback.go
Outdated
| @@ -353,13 +353,27 @@ func callbackRetNil(ctx *C.sqlite3_context, v reflect.Value) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func callbackRetAny(ctx *C.sqlite3_context, v reflect.Value) error { | |||
There was a problem hiding this comment.
Probably should be callbackRetGeneric for consistency with callbackArgGeneric. Or rename the other one to callbackArgAny.
There was a problem hiding this comment.
Adjusted, thanks!
callback.go
Outdated
| @@ -353,13 +353,27 @@ func callbackRetNil(ctx *C.sqlite3_context, v reflect.Value) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func callbackRetAny(ctx *C.sqlite3_context, v reflect.Value) error { | |||
| cb, err := callbackRet(v.Elem().Type()) | |||
There was a problem hiding this comment.
It is possible that the callback returned nil, in which case v.Elem() will return the invalid value. Presumably that needs to translate to SQLite NULL.
There was a problem hiding this comment.
Do you mean I should have a check after the err check like
if cb == nil {
return nil
}
?
There was a problem hiding this comment.
I think it would be this:
if v.IsNil() {
C.sqlite3_result_null(ctx)
return nil
}
ve := v.Elem()
cb, err := callbackRet(ve.Type())
if err != nil {
return err
}
return cb(ctx, ve)There was a problem hiding this comment.
Applied, thank you!
|
@rittneje I'm okay to merge. |
|
Thanks folks! |
Feel free to take this over or close this or make suggestions or whatever!
Closes #1045