code review 6258045: database/sql: use driver.ColumnConverter everywhere con...

database/sql: use driver.ColumnConverter everywhere consistently

It was only being used for (*Stmt).Exec, not Query, and not for
the same two methods on *DB.

This unifies (*Stmt).Exec's old inline code into the old
subsetArgs function, renaming it in the process (changing the
old word "subset" to "driver", mostly converted earlier)

Fixes issue 3640

[bradfitz, 2012-05-24 20:40:11]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg

[rsc, 2012-05-29 16:30:10]

LGTM

http://codereview.appspot.com/6258045/diff/5001/src/pkg/database/sql/convert.go
File src/pkg/database/sql/convert.go (right):

http://codereview.appspot.com/6258045/diff/5001/src/pkg/database/sql/convert....
src/pkg/database/sql/convert.go:18: // Stmt.Query into driver subset Values.
s/subset //

[bradfitz, 2012-05-29 18:09:19]

*** Submitted as http://code.google.com/p/go/source/detail?r=69285ecd63e9 ***

database/sql: use driver.ColumnConverter everywhere consistently

It was only being used for (*Stmt).Exec, not Query, and not for
the same two methods on *DB.

This unifies (*Stmt).Exec's old inline code into the old
subsetArgs function, renaming it in the process (changing the
old word "subset" to "driver", mostly converted earlier)

Fixes issue 3640

R=golang-dev, rsc
CC=golang-dev
http://codereview.appspot.com/6258045

[tve_rightscale.com, 2012-12-27 01:57:57]

Pardon my ignorance, but why has this fix not been included in Go 1.0.3?

On Thursday, May 24, 2012 1:40:11 PM UTC-7, Brad Fitzpatrick wrote:
>
> Reviewers: golang-dev_googlegroups.com, 
>
> Message: 
> Hello golan...@googlegroups.com <javascript:>, 
>
> I'd like you to review this change to 
> https://go.googlecode.com/hg 
>
>
> Description: 
> database/sql: use driver.ColumnConverter everywhere consistently 
>
> It was only being used for (*Stmt).Exec, not Query, and not for 
> the same two methods on *DB. 
>
> This unifies (*Stmt).Exec's old inline code into the old 
> subsetArgs function, renaming it in the process (changing the 
> old word "subset" to "driver", mostly converted earlier) 
>
> Fixes issue 3640 
>
> Please review this at http://codereview.appspot.com/6258045/ 
>
> Affected files: 
>    M src/pkg/database/sql/convert.go 
>    M src/pkg/database/sql/fakedb_test.go 
>    M src/pkg/database/sql/sql.go 
>    M src/pkg/database/sql/sql_test.go 
>
>
>

[minux1, 2012-12-27 17:23:22]

On Thu, Dec 27, 2012 at 9:57 AM, <tve@rightscale.com> wrote:

> Pardon my ignorance, but why has this fix not been included in Go 1.0.3?
>
FTR, it is not included in go 1.0.2.
the reason is that the changes is too big to include in a point release, and
nobody explicitly ask for it.

search "69285ecd63e9" here:
https://codereview.appspot.com/6279048/diff/25005/go102

[tve_rightscale.com, 2012-12-27 18:45:27]

Interesting. I'm new to Go and I'm finding that after 2 days of poking 
around I already have to patch the distro because of a bug that was fixed 
several months before the release I installed was cut but wasn't included. 
That doesn't sound great. (Yes, I'm having a good experience otherwise :-).

So, how does one ask for this fix to be included into the next release? The 
reason I hit this problem is that database/sql's (*Stmt).Query is broken 
for drivers that define a ColumnConverter. I'm using 
gocql https://github.com/tux21b/gocql which is unusable without the fix.

BTW, what is the recommended way to patch a package like database/sql 
locally without installing the whole Go world from source?

On Thursday, December 27, 2012 9:23:01 AM UTC-8, minux wrote:
>
>
> On Thu, Dec 27, 2012 at 9:57 AM, <t...@rightscale.com <javascript:>>wrote:
>
>> Pardon my ignorance, but why has this fix not been included in Go 1.0.3?
>>
> FTR, it is not included in go 1.0.2.
> the reason is that the changes is too big to include in a point release, 
> and
> nobody explicitly ask for it.
>
> search "69285ecd63e9" here:
> https://codereview.appspot.com/6279048/diff/25005/go102
>