Re: bcp with empty char/varchar column broken

I've here attached a patch that fixes this and causes no new test failures.  Also inline below for readability.   It isn't exactly pretty but it at least highlights exactly where the problem is.

From f53104f5264416bee462fd05b9a1e683da0133b8 Mon Sep 17 00:00:00 2001
From: "Craig A. Berry" <[email protected]>
Date: Thu, 20 Nov 2014 14:54:59 -0600
Subject: [PATCH] Avoid padding to 256 bytes on bcp out of char/varchar.

Since e30a807 we're not tracking lengths column by column when we
bulk out but are instead using at least the full length of the
256-byte buffer.  For char and varchar columns less that 256 bytes
long, they get too much padding which then can't be bulked back in
without overflow error.

This commit changes that so the maximum amount of padding we'll do
is the declared size of the column on the server.
---
 src/dblib/bcp.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/dblib/bcp.c b/src/dblib/bcp.c
index dadffea..49c4a7e 100644
--- a/src/dblib/bcp.c
+++ b/src/dblib/bcp.c
@@ -892,6 +892,12 @@ _bcp_exec_out(DBPROCESS * dbproc, DBINT * rows_copied)
 					buflen = (int)tds_strftime((TDS_CHAR *)data, 256,
 								 bcpdatefmt, &when, 3);
 				} else {
+					TDS_INT destlen = datalen;
+					if (datalen > curcol->on_server.column_size
+					    && (curcol->column_type == SYBVARCHAR
+					        || curcol->column_type == SYBCHAR)) {
+						destlen = curcol->on_server.column_size;
+					}
 					/*
 					 * For null columns, the above work to determine the output buffer size is moot,
 					 * because bcpcol->data_size is zero, so dbconvert() won't write anything,
@@ -899,7 +905,7 @@ _bcp_exec_out(DBPROCESS * dbproc, DBINT * rows_copied)
 					 */
 					/* TODO check for text !!! */
 					buflen =  dbconvert(dbproc, srctype, src, srclen, hostcol->datatype,
-							    data, datalen);
+							    data, destlen);
 					/*
 					 * Special case:  When outputting database varchar data
 					 * (either varchar or nullable char) dbconvert may have
--
1.8.4.2



> On Nov 20, 2014, at 1:53 PM, Craig A. Berry <[email protected]> wrote:
> 
> The following worked in 0.91 but is broken as of branch-0_92-839-gd788046.  If we bulk out a table with an empty (but not NULL) char or varchar column that is less than 256 bytes long, 256 spaces get written to the output file for that column.  Which then causes an overflow error if you try to bulk it back in.
> 
> I think it was e30a807 that broke it because in src/dblib/bcp.c:_bcp_exec_out we now pass a minimum of 256 as the destlen argument to dbconvert.  We used to pass -2, instructing dbconvert to null terminate without copying anything.
> 
> I'm not sure what the correct fix is.  We could possibly pass the lesser of source length and destination length to dbconvert in _bcp_exec_out.  
> 
> Here's a reproducer:
> 
> $ ./fisql/fisql -Smyserver -Utest
> Password:
> Changed database context to 'master'.
> Changed language setting to us_english.
> 1>> use tempdb
> 2>> go
> Changed database context to 'tempdb'.
> 1>> create table testbcp (tmpval varchar(10))
> 2>> go
> 1>> insert into testbcp values ('abc')
> 2>> go
> (1 rows affected)
> 1>> insert into testbcp values ('')
> 2>> go
> (1 rows affected)
> 1>> exit
> $ ./freebcp tempdb.dbo.testbcp out tmp.bcp -Smyserver -Utest -P***** -c
> 
> Starting copy...
> 2 rows copied.
> $ perl -nle 'print length($_);' < tmp.bcp
> 3
> 256
> $ ./freebcp tempdb.dbo.testbcp in tmp.bcp -Smyserver -Utest -P***** -c
> 
> Starting copy...
> Msg 20049, Level 4
> Data conversion resulted in overflow
> 
> 1 rows copied.
> 
> ________________________________________
> Craig A. Berry
> mailto:[email protected]
> 
> "... getting out of a sonnet is much more
> difficult than getting in."
>                 Brad Leithauser
> 

________________________________________
Craig A. Berry
mailto:[email protected]

"... getting out of a sonnet is much more
 difficult than getting in."
                 Brad Leithauser

_______________________________________________
FreeTDS mailing list
[email protected]
http://lists.ibiblio.org/mailman/listinfo/freetds

From f53104f5264416bee462fd05b9a1e683da0133b8 Mon Sep 17 00:00:00 2001
From: "Craig A. Berry" <[email protected]>
Date: Thu, 20 Nov 2014 14:54:59 -0600
Subject: [PATCH] Avoid padding to 256 bytes on bcp out of char/varchar.

Since e30a807 we're not tracking lengths column by column when we
bulk out but are instead using at least the full length of the
256-byte buffer.  For char and varchar columns less that 256 bytes
long, they get too much padding which then can't be bulked back in
without overflow error.

This commit changes that so the maximum amount of padding we'll do
is the declared size of the column on the server.
---
 src/dblib/bcp.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/dblib/bcp.c b/src/dblib/bcp.c
index dadffea..49c4a7e 100644
--- a/src/dblib/bcp.c
+++ b/src/dblib/bcp.c
@@ -892,6 +892,12 @@ _bcp_exec_out(DBPROCESS * dbproc, DBINT * rows_copied)
 					buflen = (int)tds_strftime((TDS_CHAR *)data, 256,
 								 bcpdatefmt, &when, 3);
 				} else {
+					TDS_INT destlen = datalen;
+					if (datalen > curcol->on_server.column_size
+					    && (curcol->column_type == SYBVARCHAR
+					        || curcol->column_type == SYBCHAR)) {
+						destlen = curcol->on_server.column_size;
+					}
 					/*
 					 * For null columns, the above work to determine the output buffer size is moot,
 					 * because bcpcol->data_size is zero, so dbconvert() won't write anything,
@@ -899,7 +905,7 @@ _bcp_exec_out(DBPROCESS * dbproc, DBINT * rows_copied)
 					 */
 					/* TODO check for text !!! */
 					buflen =  dbconvert(dbproc, srctype, src, srclen, hostcol->datatype,
-							    data, datalen);
+							    data, destlen);
 					/*
 					 * Special case:  When outputting database varchar data
 					 * (either varchar or nullable char) dbconvert may have
-- 
1.8.4.2