FreeTDS Development Group

[Frediano Ziglio <freddy77 AT gmail.com>]

I'm trying to merge it. Some hunks scary me a bit (well, the patch was
reversed but it's just an extra option to patch).

--- freetds-0.91.dev.20120624-bcpfix/include/tds.h      2013-01-08
20:46:30.000000000 +0100
+++ freetds-0.91.dev.20120624/include/tds.h     2011-08-12
18:29:36.000000000 +0200
@@ -716,13 +716,13 @@
                        x==SYBVOID      || \
                        x==SYBUNIQUE)
 #define is_nullable_type(x) ( \
-                    x==SYBBITN      || \
+                       x==SYBBITN      || \
                      x==SYBINTN      || \
                      x==SYBFLTN      || \
                      x==SYBMONEYN    || \
                      x==SYBDATETIMN  || \
                      x==SYBVARCHAR   || \
-                     /*EVEVEV x==SYBBINARY    ||*/ \
+                     x==SYBBINARY    || \
                      x==SYBVARBINARY || \
                      x==SYBTEXT      || \
                      x==SYBNTEXT     || \

this could change a lot of things. However I must say it surely must
something wrong with it (char and binary has same representation so both
should be here or not) and actually cause no regressions in our tests. Now
there is a test (t0016) which is quite flexible so I try passing either
nullable char or not nullable char. The result scary me. It seems that not
nullable are handled like fixed type while nullable are handled by variable
(like varchar). The bad things is that I cause my test server to get to
100% cpu consumption.

diff -ur freetds-0.91.dev.20120624-bcpfix/src/tds/bulk.c
freetds-0.91.dev.20120624/src/tds/bulk.c
--- freetds-0.91.dev.20120624-bcpfix/src/tds/bulk.c     2013-05-05
08:42:37.000000000 +0200
+++ freetds-0.91.dev.20120624/src/tds/bulk.c    2011-06-02
21:19:30.000000000 +0200
@@ -584,23 +584,19 @@
                                if ((get_col_data(bcpinfo, bindcol,
offset)) != TDS_SUCCEED) {
                                        return TDS_FAIL;
                                }
-                               /* maybe the same code in bcp_moretext ? */
-                               tdsdump_log(TDS_DBG_INFO1, "is_blob_type
col=%u datalen=%u data=%x\n",
-                                                       i,
bindcol->bcp_column_data->datalen, bindcol->bcp_column_data->data);
-                               if (bindcol->bcp_column_data->datalen &&
bindcol->bcp_column_data->data) {
-                                   /* unknown but zero */
-                                   tds_put_smallint(tds, 0);
-                                   tds_put_byte(tds, bindcol->column_type);
-                                   tds_put_byte(tds, 0xff - blob_cols);
-                                   /*
-                                    * offset of txptr we stashed during
variable
-                                    * column processing
-                                    */
-                                   tds_put_smallint(tds,
bindcol->column_textpos);
-                                   tds_put_int(tds,
bindcol->bcp_column_data->datalen);
-                                   tds_put_n(tds,
bindcol->bcp_column_data->data, bindcol->bcp_column_data->datalen);
-                               } /*else server table gets into very
strange&broken state */
+                               /* unknown but zero */
+                               tds_put_smallint(tds, 0);
+                               tds_put_byte(tds, bindcol->column_type);
+                               tds_put_byte(tds, 0xff - blob_cols);
+                               /*
+                                * offset of txptr we stashed during
variable
+                                * column processing
+                                */
+                               tds_put_smallint(tds,
bindcol->column_textpos);
+                               tds_put_int(tds,
bindcol->bcp_column_data->datalen);
+                               tds_put_n(tds,
bindcol->bcp_column_data->data, bindcol->bcp_column_data->datalen);
                                blob_cols++;
+
                        }
                }


This add an extra check. But the else case is not handled at all which
could possible cause table corruption, honestly I would prefer bcp to fail
instead of corrupting my db. Why is this test necessary. It shouldn't,
get_col_data should take care of filling the db.

The adjustment array computation is quite complicated. Well, if I remember
code was already changed and code was complicated before. Up to which size
did you tried ?? I think trying till some thousand could help shed some
light. It's worrying that every 256 bytes cause every byte. So a field at
offset 1024 should take 5 byte to state the length.

Frediano



2013/5/5 Eduard Heimann <eheimann AT pe-fittinge.de>

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 05/04/13 18:51, James K. Lowden wrote:
> > On Sat, 04 May 2013 18:04:03 +0200 Eduard Heimann <
> eheimann AT pe-fittinge.de>
> > wrote:
> >
> >> Please help me to integrate my fix into the mainline of freetds. I have
> >> no experience with this kind of distributed development.
> >
> > Thank you for reporting the problem and developing a fix.  :-)
> >
> > The most developer-friendly way to supply a patch is with "git diff". If
> > you're not using git, "diff -u oldfile newfile > file.diff" works, too.
> >
> > As a last resort, simply attached your modified file to a message you
> post
> > here.
> >
> > It helps if the file is an *attachment* because email systems often wrap
> > long lines in the code, something that can be undone only by hand.
> >
> > Provided we agree with with your assessment and solution, we'll update
> the
> > master git branch.  Usually someone gets around to it within a week or
> so.
> >
> >
> > --jkl _______________________________________________ FreeTDS mailing
> list
> > FreeTDS AT lists.ibiblio.org
> > http://lists.ibiblio.org/mailman/listinfo/freetds
> >
> >
>
> Description of this fix for freetds:
> BCP-copying wide rows (>255 bytes, from file into server-table) results in
> an
> error in bcp_sendrow() and a bogus state in Sybase SQL Server. Tested
> against
> Server 10.2/32bit and Server 12.4/64bit.
> This patch is a result of reverse-engineering Sybase's bcp-library on
> datastream level.
>
>
> - --
> Eduard Heimann, Hörmannsdorf 9, D-94104 Tittling
> Tel: 08504 955249              Fax: 08504 955248
> Mail mailto:eheimann AT pe-fittinge.de  Web: http://www.pe-fittinge.de
> Eine Haustüre ohne Schloß ist wie eMail ohne http://www.GNUPG.org
> GPG key ID: 0xB542BD21 http://keyserver.ubuntu.com
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.19 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlGGE2UACgkQUzMNXFvaJEbHPACeI+qOPGQxWNRnCY4/wyN4lOx4
> KvIAn06JlDuVwLo+qVK3dpXXMO/s1mLq
> =p3R8
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> FreeTDS mailing list
> FreeTDS AT lists.ibiblio.org
> http://lists.ibiblio.org/mailman/listinfo/freetds
>
>