Skip to content
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

Implement database/sql's Scanner interface. Fixes #5 #6

Merged
merged 1 commit into from
Aug 24, 2015

Conversation

jboverfelt
Copy link
Contributor

Hi! This PR adds an implementation of sql.Scanner for convenience when working with uuids in SQL databases. sql.Scanner allows the reading of uuid values from a database directly into a uuid.UUID.

The implementation of sql.Scanner provided can read values as strings (assumed to be in the format uuid.Parse accepts) or plain byte slices.

I've written test cases that provide full coverage and have tested this out with sqlite3 using a test harness that I can provide if you're interested.

Please let me know if you have any questions or if you would like me to change anything about the implementation.


import (
"database/sql"
"database/sql/driver"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be fine with Scan, but I do not want to pull a dependency in on sql or sql driver. That would force the inclusion of sql and sql/driver in every application that uses UUID's. Seems to me the functionality should live in an sql library. You can simmly have sql/uuid which has "type UUID struct { uuid.UUID } and then define your new types on (or I believe you can).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok fair enough. I'll remove the dep on sql and sql/driver. The sql dep is only there for the little check I added in to make sure it implements the interface at compile time. I'll submit an update with just Scan implemented.

@jboverfelt jboverfelt changed the title Implement database/sql's Valuer and Scanner interfaces. Fixes #5 Implement database/sql's Scanner interface. Fixes #5 Aug 4, 2015
@jboverfelt
Copy link
Contributor Author

I updated this PR to only implement sql.Scanner, thus removing the dep on database/sql. I also improved the coverage to be 100% of sql.go. Let me know if this works.

case string:
// see uuid.Parse for required string format
str := src.(string)
parsed := Parse(str)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason for the temporary variable here.

parsed := Parse(src.(string))

For consistency, probably use u instead of parsed (so it matches below)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


*uuid = u
default:
return fmt.Errorf("Scan: Unable to scan type %T into UUID", src)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Unable/unable

pborman pushed a commit that referenced this pull request Aug 24, 2015
Implement database/sql's Scanner interface. Fixes #5
@pborman pborman merged commit cccd189 into pborman:master Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants