This adds support for decoding varchar, text -> string
The use case is scanning a row with an interface{} type when the postgres column types are unknown.This will currently decode columns of text and numeric types as []uint8.

shrug This might make sense, though I'm slightly worried since it breaks backwards compatibility. I'm pretty sure I have one such application, and I haven't written that many Go apps in my life.

and numeric -> float.

No friggin' way. People might have money stored in "numeric" fields, and turning them into floats is one of the most horrible things you could do to anyone's money.

Fair enough, you're right about the floats, it doesn't make sense for everyone.
I couldn't think of any breakages for the strings though, do you have a case to enlighten me?

Removed the numeric casting. Just the string casting remaining now.

I couldn't think of any breakages for the strings though, do you have a case to enlighten me?

Imagine someone writing an app which reads an arbitrary number of columns from the database, perhaps something that turns a user's query into a CSV file for example. They might have written their code assuming that they never get a string back from Scan() (which is not an unreasonable assumption since they never would). Now this change just broke their app.

Further, I don't really see this as a significant improvement, either. It's trivial for anyone to do the conversion themselves, and doing it in the driver isn't a performance win either. I don't think this change is a good idea, but I'm prepared to be convinced otherwise in case I missed something.

Imagine someone writing an app which reads an arbitrary number of columns from the database

Actually that's our use case for wanting this casting. The db column types are almost always one of timestamp, bool, float, text, varchar. The problem is at scan time we don't know what the column type is, so we scan with an interface{} slice then attempt to type assert each column's value into one of the expected go types. The problem is currently with postgres text & varchar types - since the column interface value contains a []uint8 type there's nothing to say that this is really a string and that it can be cast as such. The postgres type information is lost.

doing it in the driver isn't a performance win either

This is probably true if the user is scanning a text column to []byte since it would have to be converted back to bytes after being converted to a string.

I see that we could solve this in our app by assuming that a []uint8 is always a string, but that feels wrong, especially when considering there may be more types in the future that we need to distinguish.

Imagine someone writing an app which reads an arbitrary number of columns from the database

Actually that's our use case for wanting this casting. The db column types are almost always one of timestamp, bool, float, text, varchar. The problem is at scan time we don't know what the column type is, so we scan with an interface{} slice then attempt to type assert each column's value into one of the expected go types. The problem is currently with postgres text & varchar types - since the column interface value contains a []uint8 type there's nothing to say that this is really a string and that it can be cast as such. The postgres type information is lost.

Hmm. I guess I can see some value in having that information available.

doing it in the driver isn't a performance win either
This is probably true if the user is scanning a text column to []byte since it would have to be converted back to bytes after being converted to a string.

Yeah, so it's actually a performance loss in some cases, albeit likely very small.

I see that we could solve this in our app by assuming that a []uint8 is always a string, but that feels wrong, especially when considering there may be more types in the future that we need to distinguish.

It's a bit sad that database/sql doesn't provide any way for the drivers to provide type information for a result set. I think that would solve this problem completely. What we can now do is only marginally improve some corner cases. Even after this patch, you still wouldn't have an idea when you get a []byte back from the driver; is it bytea, json, numeric or something else?

So a quick recap of the way I see this is:

• Pro: when scanning to an interface{}, there's an undocumented contract that if you got a string back, you'll know the column was varchar or text, but only assuming you're on a version of the driver after this change was applied
• Con: this might break real applications out there
• Con: this might cause a small performance loss in real applications

I think the cons still outweigh the pro (singular). The benefit is very marginal, and from my perspective this change has a better chance at breaking applications than helping new ones. I think we should focus on providing a way for the drivers to provide type information through database/sql rather than coming up with workarounds which solve only a small percentage of the cases.

Any other committers want to voice their opinions on this matter?

I wrote:

So a quick recap of the way I see this is:

Pro: when scanning to an interface{}, there's an undocumented contract that if you got a string back, you'll know the column was varchar or text, but only assuming you're on a version of the driver after this change was applied

That's not actually entirely fair. Let me try and rephrase.

When scanning to an interface{}, there's an undocumented contract that if you got a string back, you'll know the column was varchar or text. If you're on a version of the driver cut after this change was applied, you can assume that []byte is not varchar or text.

The docs say not to return string from Scan, but there is an open issue about it.

There is also an older, rejected issue.

Upstream isn't going to change the rules https://go-review.googlesource.com/#/c/19439/

Yeah, I believe we can forget this idea.

Update: this has new life - upstream just merged my change golang/go@7162c4d

@johto can you reopen this? @chris-baynes are you up for rebasing this after @johto reopens?

Yes, will push the rebase once this is reopened - congrats on getting that change pushed through!

Thanks! It was a struggle.

I also reopened #436 which is ready to merge now, though this PR should go in as well to do the varchar bit.