[PATCH] Handle empty char/varchar in bcp round trip (Re: bp with empty char/varchar column broken)
"Craig A. Berry" <[email protected]>
| Newsgroups | gmane.comp.db.tds.freetds |
|---|---|
| Message-ID | <[email protected]> |
> On Nov 21, 2014, at 9:43 AM, Craig A. Berry <[email protected]> wrote: > > >> On Nov 21, 2014, at 5:58 AM, Frediano Ziglio <[email protected]> wrote: >> >> 2014-11-20 19:53 GMT+00:00 Craig A. Berry <[email protected]>: >>> 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. >> > >> Quite strange. Why does it happen only for empty strings? Should even >> happen with small ones. The function has to pad or not based on type. > > The actual length of the string received on the wire is passed as the source length to dbconvert, and dbconvert (usually) uses that as its return value, and that return value is passed to fwrite for moving data to the file. > > But if the source length is zero, dbconvert uses the destination length as its return value as long as the destination length is greater than zero. Since it's now always 256, we get 256 spaces. > >> Actually is not clear to me what an empty column should look like in >> the text bcp file. > > I'm pretty sure a zero-length column in a bcp file indicates NULL, and one or more spaces indicates empty string and will be trimmed on bulk in. I have studied this a bit more now. What the Microsoft client does consistently is output a single ASCII NUL to indicate an empty string in the database column and an empty string to indicate a database NULL. I don't have any Sybase clients to test with and I haven't found any documentation indicating how it works so I don't know if this is part of the BCP spec or just something Microsoft did. The attached patch modifies _bcp_exec_out and _bcp_read_hostfile so FreeTDS does the same thing Microsoft bcp does. Without this, we don't handle empty strings correctly round trip in bcp. ________________________________________ 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
0001-Handle-empty-char-varchar-in-bcp-round-trip.patch
(application/octet-stream, 4.8 KB)
From 15c77ffc6359bd2c27b350ecfbe3a6efb0eb102e Mon Sep 17 00:00:00 2001 From: "Craig A. Berry" <[email protected]> Date: Thu, 26 Feb 2015 18:43:39 -0600 Subject: [PATCH] Handle empty char/varchar in bcp round trip. Since e30a807 we're not tracking lengths column by column when we bulk out, which, in the case of of empty strings, causes us to pad out to the full 256 bytes of the buffer. If the declared size of the char or varchar column is less than that length, bulking back in fails with an overflow error. The fix for this wasn't simply to avoid padding as it turns out we weren't really handling empty strings correctly, or at least weren't doing what the Microsoft bcp client does, which in the absence of documentation will have to suffice. What Microsoft bcp does (and what FreeTDS's bcp does after this commit) is output a single ASCII NUL byte for an empty column on output, and then on input, handle that NUL character and convert it back to an empty string, allowing a successful round trip. Note that this is different from (but easily confused with) the handling of database NULL, which produces an empty string in the output file. --- src/dblib/bcp.c | 25 +++++++++++++++++++--- src/dblib/unittests/bcp.sql | 51 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/dblib/bcp.c b/src/dblib/bcp.c index ee01e7c..8642b21 100644 --- a/src/dblib/bcp.c +++ b/src/dblib/bcp.c @@ -888,6 +888,19 @@ _bcp_exec_out(DBPROCESS * dbproc, DBINT * rows_copied) buflen = (int)tds_strftime((TDS_CHAR *)data, 256, bcpdatefmt, &when, 3); } else { + TDS_INT destlen = datalen; + /* + * An empty string is denoted in the output file by a single ASCII NUL + * byte that we request by specifying a destination length of -1. (Not + * to be confused with a database NULL, which is denoted in the output + * file with an empty string!) + */ + if (srclen == 0 + && (curcol->column_type == SYBVARCHAR + || curcol->column_type == SYBCHAR)) { + destlen = -1; + } + /* * 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, @@ -895,7 +908,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 @@ -1271,8 +1284,14 @@ _bcp_read_hostfile(DBPROCESS * dbproc, FILE * hostfile, int *row_error) /* trim trailing blanks from character data */ if (desttype == SYBCHAR || desttype == SYBVARCHAR) { - bcpcol->bcp_column_data->datalen = rtrim((char *) bcpcol->bcp_column_data->data, - bcpcol->bcp_column_data->datalen); + /* A single NUL byte indicates an empty string. */ + if (bcpcol->bcp_column_data->datalen == 1 + && bcpcol->bcp_column_data->data[0] == '\0') { + bcpcol->bcp_column_data->datalen = 0; + } else { + bcpcol->bcp_column_data->datalen = rtrim((char *) bcpcol->bcp_column_data->data, + bcpcol->bcp_column_data->datalen); + } } } #if USING_SYBEBCNN diff --git a/src/dblib/unittests/bcp.sql b/src/dblib/unittests/bcp.sql index 3bb08b0..a7b7ce0 100644 --- a/src/dblib/unittests/bcp.sql +++ b/src/dblib/unittests/bcp.sql @@ -139,6 +139,57 @@ VALUES ( , 1234 -- not_null_smallint , 123 -- not_null_tinyint ) +INSERT all_types_bcp_unittest + ( not_null_bit + + , not_null_char + , not_null_varchar + + , not_null_datetime + , not_null_smalldatetime + + , not_null_money + , not_null_smallmoney + + , not_null_float + , not_null_real + + , not_null_decimal + , not_null_numeric + + , not_null_int + , not_null_smallint + , not_null_tinyint + + , nullable_char + , nullable_varchar + + ) +VALUES ( + 1 -- not_null_bit + + , '' -- not_null_char empty string + , '' -- not_null_varchar empty string + + , 'Dec 17 2003 3:44PM' -- not_null_datetime + , 'Dec 17 2003 3:44PM' -- not_null_smalldatetime + + , 12.34 -- not_null_money + , 12.34 -- not_null_smallmoney + + , 12.34 -- not_null_float + , 12.34 -- not_null_real + + , 12.34 -- not_null_decimal + , 12.34 -- not_null_numeric + + , 1234 -- not_null_int + , 1234 -- not_null_smallint + , 123 -- not_null_tinyint + + , '' -- nullable_char empty string + , '' -- nullable_varchar empty string +) go select colid, cast(c.name as varchar(30)) as name, c.length , ' '+ substring('NY', convert(bit,(c.status & 8))+1,1) as Nulls from syscolumns as c left join systypes as t on c.usertype = t.usertype where c.id = object_id('all_types_bcp_unittest') order by colid go -- 2.2.1