-
Notifications
You must be signed in to change notification settings - Fork 60
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
ColumnType methods should not panic if column type is unimplemented #32
Conversation
@@ -1114,7 +1114,7 @@ func makeGoLangScanType(ti typeInfo) reflect.Type { | |||
case typeVariant: | |||
return reflect.TypeOf(nil) | |||
default: | |||
panic(fmt.Sprintf("not implemented makeGoLangScanType for type %d", ti.TypeId)) | |||
return reflect.TypeOf(new(interface{})).Elem() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this common code into a helper function that also logs the type name so developers can see when this is happening if they hook up driver logs in their app.
Please add a test that shows the application-level change this enables. What functionality does this light up ? |
Please update changelog.md in the root with a description of the change. |
@kellymtrinh Are you still planning to complete this PR? thx |
Closing because I will not be able to pick this up again soon. Appreciate the review and I still think this is a good first issue! |
For issue: #31
ColumnType methods should return nil values instead of panicking when column types are unimplemented. Current behavior, which panics, means calling
ColumnTypes()
or related methods in thedatabase/sql
lib panics if I'm doing something likeselect *
on a table with an unsupported column (or trying to use the driver for generic queries). Matching the spec would let clients handle the cases.This matches the documentation in the database/sql package for drivers: