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

proposal: database/sql: add interface for Scan method on Row and Rows #33786

Closed
Fanarito opened this issue Aug 22, 2019 · 5 comments
Closed

proposal: database/sql: add interface for Scan method on Row and Rows #33786

Fanarito opened this issue Aug 22, 2019 · 5 comments

Comments

@Fanarito
Copy link

@Fanarito Fanarito commented Aug 22, 2019

Problem

Every time that I start a new project that uses database/sql I end up creating an interface for the Scan method that is present on both sql.Row and sql.Rows this allows me to make a function that accepts both sql.Row and sql.Rows and uses the Scan method to populate a struct.

Solution

I feel like this interface should be present in the database/sql package rather than it being defined by me.

type RowScanner interface {
	Scan(dest ...interface{}) error
}
@gopherbot gopherbot added this to the Proposal milestone Aug 22, 2019
@gopherbot gopherbot added the Proposal label Aug 22, 2019
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 23, 2019

@kardianos
Copy link
Contributor

@kardianos kardianos commented Aug 23, 2019

If I was to guess, you are manually hydrating each struct item through a method. That's an interesting method.

I assume you are doing something like this:

type MyDataRow struct {
    Item1 string
    Item2 int64
}
func (r *MyDataRow) fill(s RowsScanner) error {
    return s.Scan(&r.Item1, &r.Item2)
}

Can you confirm?

@Fanarito
Copy link
Author

@Fanarito Fanarito commented Aug 23, 2019

Yeah that's right

@Fanarito
Copy link
Author

@Fanarito Fanarito commented Aug 23, 2019

A full example would look something like this

type Todo struct {
	Id    string
	Title string
	Done  bool
}

const todoFields = "id, title, done"

func populateTodo(todo *Todo, s RowsScanner) error {
	return s.Scan(&todo.Id, &todo.Title, &todo.Done)
}

func FindAll(db *sql.DB) []Todo {
	query := fmt.Sprintf("SELECT %s FROM todos", todoFields)
	todos := make([]Todo, 0)
	rows, _ := db.Query(query)
	for rows.Next() {
		var todo Todo
		_ := populateTodo(&todo)
		todos = append(todos, todo)
	}
	return todos
}

func FindSingle(db *sql.DB, id string) Todo {
	query := fmt.Sprintf("SELECT %s FROM todos WHERE id = $1", todoFields)
	row := db.Query(query, id)
	var todo Todo
	_ := populateTodo(&todo, row)
	return todo
}
@kardianos
Copy link
Contributor

@kardianos kardianos commented Aug 23, 2019

Thank you for the use case.

  1. I think this illustrates is a great aspect of Go. An interface can be implicit.
  2. I don't think this is a universal issue. For instance, we still haven't added a Queryer interface yet.

I could easily see an interface defined as:

type Scanner interface {
    Scan(rows *sql.Rows) error
}

type TODO struct {...}
func (t *TODO) Scan(rows *sql.Rows) error { ... }

But this isn't to say your version would be worse or better, but I would perceive the value is more application dependent. So if you choose to marshal data in this way, make a common package that defines it for you.

I think a RowScanner would have some value, but I don't think it would pull its own weight.

@kardianos kardianos closed this Aug 23, 2019
@golang golang locked and limited conversation to collaborators Aug 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.