code review 8751045: odbc: Add support for MS SQL type SQL_SS_XML data type.

odbc: Add support for MS SQL type SQL_SS_XML data type.

[brainman, 2013-04-24 03:50:22]

It is good start. Please see my comments inline. Just use "hg mail 8751045" when
you are ready for review again - that will send email to all reviewers.

Thank you.

Alex

https://codereview.appspot.com/8751045/diff/2001/api/api_unix.go
File api/api_unix.go (left):

https://codereview.appspot.com/8751045/diff/2001/api/api_unix.go#oldcode72
api/api_unix.go:72: SQL_UNSIGNED_OFFSET = C.SQL_UNSIGNED_OFFSET
I cannot find this value either. Lets stress that it is invented for unix. Just
add comment, like

// TODO(lukemauldin): Not defined in sqlext.h. Using windows value, but it is
not supported.

https://codereview.appspot.com/8751045/diff/2001/mssql_test.go
File mssql_test.go (right):

https://codereview.appspot.com/8751045/diff/2001/mssql_test.go#newcode126
mssql_test.go:126: xmlData   sql.NullString
This test fails on linux here. Please, remove your changes and add xml test to
TestMSSQLTypes instead. We already have typeUnicodeTests that run on windows
only, just add one or more lines to that array with your new xml tests. Perhaps
you could rename it into typeTestsWindows since it is not unicode specific
anymore - just group the lines and comment them appropriately.

[lukemauldin, 2013-04-25 00:52:49]

Alex,

I made the changes you requested and I am testing with the "msdriver"
option of "SQL Server Native Client 11.0" and one of the existing tests is
failing:
mssql_test.go:530: test "select cast('abc' as varchar(max))" failed: expect
"abc", but got "\x00\x00\x00"
panic: runtime error: slice bounds out of range [recovered]
        panic: runtime error: slice bounds out of range

It is throwing the panic on column/odbc:217.  If I test with "msdriver"
option of "SQL Server" then I do not get the error.  Do you want me to try
to look into it before I submit the code change for review again?  Do you
know what might be causing it?

Luke


On Tue, Apr 23, 2013 at 10:50 PM, <alex.brainman@gmail.com> wrote:

> It is good start. Please see my comments inline. Just use "hg mail
> 8751045" when you are ready for review again - that will send email to
> all reviewers.
>
> Thank you.
>
> Alex
>
>
>
https://codereview.appspot.**com/8751045/diff/2001/api/api_**unix.go<https://...
> File api/api_unix.go (left):
>
> https://codereview.appspot.**com/8751045/diff/2001/api/api_**
>
unix.go#oldcode72<https://codereview.appspot.com/8751045/diff/2001/api/api_unix.go#oldcode72>
> api/api_unix.go:72: SQL_UNSIGNED_OFFSET = C.SQL_UNSIGNED_OFFSET
> I cannot find this value either. Lets stress that it is invented for
> unix. Just add comment, like
>
> // TODO(lukemauldin): Not defined in sqlext.h. Using windows value, but
> it is not supported.
>
>
https://codereview.appspot.**com/8751045/diff/2001/mssql_**test.go<https://co...
> File mssql_test.go (right):
>
> https://codereview.appspot.**com/8751045/diff/2001/mssql_**
>
test.go#newcode126<https://codereview.appspot.com/8751045/diff/2001/mssql_test.go#newcode126>
> mssql_test.go:126: xmlData   sql.NullString
> This test fails on linux here. Please, remove your changes and add xml
> test to TestMSSQLTypes instead. We already have typeUnicodeTests that
> run on windows only, just add one or more lines to that array with your
> new xml tests. Perhaps you could rename it into typeTestsWindows since
> it is not unicode specific anymore - just group the lines and comment
> them appropriately.
>
>
https://codereview.appspot.**com/8751045/<https://codereview.appspot.com/8751...
>

[lukemauldin, 2013-04-25 11:38:11]

Hello alex.brainman@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/odbc

[brainman, 2013-04-26 02:47:29]

LGTM

Thank you

Please, complete Go contributor form at
http://golang.org/doc/contribute.html#copyright. Let me know, when you are done,
so I could chase it about. And then I will submit your change.

Alex

[lukemauldin, 2013-04-26 13:04:44]

Alex,

I have submitted the individual contributor form.

Luke


On Thu, Apr 25, 2013 at 9:47 PM, <alex.brainman@gmail.com> wrote:

> LGTM
>
> Thank you
>
> Please, complete Go contributor form at
>
http://golang.org/doc/**contribute.html#copyright<http://golang.org/doc/contr....
> Let me know, when you
> are done, so I could chase it about. And then I will submit your change.
>
> Alex
>
>
https://codereview.appspot.**com/8751045/<https://codereview.appspot.com/8751...
>

[bradfitz, 2013-04-26 17:39:02]

Verified.



On Fri, Apr 26, 2013 at 6:04 AM, Luke Mauldin <lukemauldin@gmail.com> wrote:

> Alex,
>
> I have submitted the individual contributor form.
>
> Luke
>
>
> On Thu, Apr 25, 2013 at 9:47 PM, <alex.brainman@gmail.com> wrote:
>
>> LGTM
>>
>> Thank you
>>
>> Please, complete Go contributor form at
>>
http://golang.org/doc/**contribute.html#copyright<http://golang.org/doc/contr....
>> Let me know, when you
>> are done, so I could chase it about. And then I will submit your change.
>>
>> Alex
>>
>>
https://codereview.appspot.**com/8751045/<https://codereview.appspot.com/8751...
>>
>
>  --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
>

[brainman, 2013-04-27 01:15:42]

On 2013/04/26 17:39:02, bradfitz wrote:
> Verified.
> 

Thank you. Could you add Luke to CONTRIBUTORS file, please. I get this

$ hg clpatch 8751045
warning: cannot find lukemauldin@gmail.com in CONTRIBUTORS

from codereview.

Alex

[brainman, 2013-05-01 07:44:39]

*** Submitted as https://code.google.com/p/odbc/source/detail?r=8a1d76486818 ***

odbc: Add support for MS SQL type SQL_SS_XML data type.

R=alex.brainman, bradfitz
CC=golang-dev
https://codereview.appspot.com/8751045

Committer: Alex Brainman <alex.brainman@gmail.com>