proposal: database/sql: allow Scan method to work with structs

[moltzaum]

Background:
Adding the ability to scan into structs would be helpful to maintain both readability and flexibility of sql scan code. At the moment the way to scan into a struct is to use Scan on each individual variable in the struct, which can lead to long lines of code . There is a package sqlx that introduces a new function "StructScan", but this method has a few limitations. The primary problem is that every result in the query must be matched to members in the struct exactly.

Related PR: #26646
I have implemented a relatively simple solution in the PR mentioned above that I don't believe to be intrusive to the current implementation, though perhaps it is for future plans, in which case I'd like to talk about design.

As I was working on this, I saw a couple github issues on performace (namely #22544) that might change the Scan function. Since I'm using reflection, that could theoretically also change performance as well. @kardianos, what are your concerns with what I wrote, and in what manner were you thinking this might get implemented? What would I be have to modify to get a merge?

[kardianos]

@moltzaum Thank you for creating this proposal.

Right now the cost of reflection is added to each scan, even if it is never used. This is kinda a problem. Second, if we hard code a single way to scan in structs, we need to document how it works, how it handles NULLs, how it handles non-default data types. Not only that, if we choose a single strategy, we risk blocking ourselves into a corner where is doesn't work for significant use cases.

I would ideally make the scanning able to be plugged in. This might need to wait until a database/sql/v2.

---

But let's back up a bit first. Your problem is with sqlx. Your example limitation is that the struct must match exactly. I don't understand why you would need to make a change to database/sql to fix this. This seems more an issue of providing dummy interface{} values to scan into and tracking what columns need to be scanned into by reading the column names.

I would like to have native scanning into structs. I don't think the database/sql package is ready for that.

[moltzaum]

I wouldn't say my issue is with the way sqlx scans structs since I don't particularly care for reading column names. Rather, I want to scan everything sequentially since that's what I've been doing with the named queries anyways. Yes, I could try to add code to sqlx, but having a native solution was the more obvious choice.

Can you elaborate on what you mean by "if we choose a single strategy we risk blocking ourselves into a corner where it doesn't work for significant use cases"? In particular, I don't think we're limited to a single strategy, and surely all use cases can be resolved? If a single strategy is implemented and another is required, wouldn't they be independent from one another? And if a single use case is not taken care of, wouldn't it be worth it if at least a few more use cases have been covered?

[kardianos]

@moltzaum The go standard library can't play take-backs. It is not the way of the Go stdlib to "just add another way", and sometimes methods are mutually incompatible.

Let me go into a few specifics:

• NULL handling: should a null into a non-nullable field raise an error? should it set a special in-band signal value (like -1)?
• Field names: what about un-named columns, should we rely on ordering, or should it be ignored?
• Field names: should we allow struct tags to override the field names like json and xml allows?
• If we read field tags, are there other options to consider?
• Should we pass field tags options to some type of converter?
• Should structs be recursively inspected for matching fields?

---

Lastly, are you familiar with the table package: https://godoc.org/bitbucket.org/kardianos/table ?

I need to pull back again and ask: what problem are you trying to specifically solve?

[moltzaum]

I think you are overcomplicating this, potentially due to a misunderstanding of what I'm proposing. Yet you have given valid reasons for your conclusion, so the remainder of this message is simply clarifying my approach. Thanks for reviewing.

---

Null handling on the fields of a struct should simply have the same behavior as if the actual field were scanned in itself. i.e. If a struct contains a single integer, that would have the same Null-handling behavior as if you decided to scan into an integer. AFAIK the current default behavior for scanning NULL into an integer is that it raises an error. Not only is this much easier to implement, but it is consistent which makes it intuitive. If someone wants to override the behavior, all they need to do is implement Scan. Struct fields can also implement scan, so structs like NullString still work completely.

There isn't much to clarify on what I'm specifically trying to do. I just want to scan into a struct directly without too much horizontal verbosity, and I think it'd be nice to have natively in golang. If you must know my exact problem, I am currently using two queries where there realistically only needs to be one. I can scan from a result set like so: Scan(&response.f1, &response.f2, &response.f3, extraData), but it gets ridiculously long. Regarding sqlx, it is not possible to say StructScan(&response) followed by Scan(&extraData) for a single result set. Again, I'm not interested in changing how sqlx works because doing so would require answering all the questions you're asking now.

With the implementation I have, all of these questions are easy since I'm not suggesting field names or tags be used in the scan at all. If you really wanted to, you can currently 1) Keep all current functionality 2) Get new functionality and 3) Support future functionality without thinking about every possible detail of said future functionality. I don't think its a good idea to "just add another way" willy nilly, but to an extent I think it makes sense not have to worry about everything you want to add off the bat, especially if you decide to encapsulate functionality within each individual method.

[Azareal]

Is this relying on the order of the fields? That would make it pretty hard to change the order without accidentally breaking countless queries, not only that, but to figure out what broke exactly, you would have to run tests on each and every query. And even then, it might slip through.

[moltzaum]

Yes, the idea is to use the order of the fields. I don't see why you'd just change the order though. If you added a new field you'd have a similar problem with whatever method you use.

If you know what queries are related, you shouldn't have to search far. Any change you have to make would be repetitive, so you could copy and paste a bit if you really have more than one select query.

[kardianos]

@moltzaum I'll use your code as evaluating this proposal:

• Scanning into a struct based on the order of the field is a non-starter. For wide queries it is imperative that names are matched on. @Azareal also makes valid points.
• Database drivers can return structs. I strongly suspect this proposal, as is, would interact and interfere poorly (break compatibility) in these cases.

Due to these issues alone, I need to reject this proposal.