FreeTDS Development Group

["ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>]

> 
> On  Sun, 22 Feb 2004 at 10:03:02 +0100, Frediano Ziglio wrote:
> 
> >>>
> >>> On Feb 18, 2004, at 2:51 PM, Jonathan Monroe wrote:
> >>>
> >>>> I think I have found an issue with the way the SQL_C_BINARY
> >>> data type
> >>>> is handled in the ODBC code.
> >>> [snip]
> >>>
> >>> It just occurred to me that the patch I provided for
> >>> convert_tds2sql.c
> >>> only fixed the problem for varchar columns - other column 
> types will
> >>> still have some sort of conversion performed.  The better way of
> >>> handling it is probably to treat SQL_C_BINARY as a special
> >>> case and not
> >>> call tds_convert() at all.
> >>>
> >>> So, the new patch would look like this:
> >>>
> >>> convert_tds2sql.c:  v1.37:
> >>> 81,84c81,102
> >>> <       nRetVal = tds_convert(context, srctype, src, srclen,
> >>> nDestSybType, &ores);
> >>> <       if (nRetVal < 0)
> >>> <               return nRetVal;
> >>> <
> >>> ---
> >>>>       if (desttype == SQL_C_BINARY)
> >>>>       {
> >>>>               /* just copy the raw bytes */
> >>>>               if (destlen > 0) {
> >>>>                       cplen = (destlen > srclen) ? srclen
> >>> : destlen;
> >>>>                       assert(cplen >= 0);
> >>>>                       /* do not NULL terminate binary buffer */
> >>>>                       memcpy(dest, src, cplen);
> >>>>                       ret = cplen;
> >>>>               } else {
> >>>>                       /* if destlen == 0 we return only length */
> >>>>                       if (destlen != 0)
> >>>>                               ret = TDS_FAIL;
> >>>>               }
> >>>>       }
> >>>>       else
> >>>>       {
> >>>>               nRetVal = tds_convert(context, srctype, 
> src, srclen,
> >>> nDestSybType, &ores);
> >>>>               if (nRetVal < 0)
> >>>>                       return nRetVal;
> >>>>       }
> >>>>
> >>> 236,250c254
> >>> <               tdsdump_log(TDS_DBG_FUNC, "convert_tds2sql:
> >>> outputting
> >>> binary data destlen = %d \n", destlen);
> >>> <
> >>> <               ret = nRetVal;
> >>> <               if (destlen > 0) {
> >>> <                       cplen = (destlen > nRetVal) ? nRetVal
> >>> : destlen;
> >>> <                       assert(cplen >= 0);
> >>> <                       /* do not NULL terminate binary buffer */
> >>> <                       memcpy(dest, ores.ib, cplen);
> >>> <               } else {
> >>> <                       /* if destlen == 0 we return only 
> length */
> >>> <                       if (destlen != 0)
> >>> <                               ret = TDS_FAIL;
> >>> <               }
> >>> <
> >>> <               free(ores.ib);
> >>> ---
> >>>>               /* handled as special case above */
> >>>
> >>>
> >>> Anybody see any problems with doing it this way?
> >>>
> >>> Jonathan Monroe
> >>>
> >>
> >> You spot a real problem. I don't know if is correct to 
> handle varchar 
> >> as
> >> a binary string (ie: '012345') however I don't know the 
> real behaviour
> >> for other types. For instance how MS ODBC return a datetime?? Same
> >> format of wire (as FreeTDS does now) or converting to 
> TIMESTAMP before
> >> ?? Some tests are needed to handle this type...
> >>
> >
> > SQL_C_BINARY behavior is correct and reflect MS ODBC one. I 
> don't know
> > why but all library (dblib, CTLib and ODBC) have the same behavior.
> > I'm doing some tests for DATETIME and others type.
> 
> When you say SQL_C_BINARY behavior is correct, I assume you 
> mean after 
> incorporating the patch.
> 

No, without patch. I checked varchar and behavior (threat as binary
string) is correct. Well, MS ODBC is not 100% reference, however we try
to reflect it's behavior (when not broken). In this case behavior is the
same of dblib so I think it's for storic reasons...

> I did some testing of the non-blob data types in conjunction with 
> SQL_C_BINARY, using the patch above.  It looks like it really only 
> solves the problem for the following SQL types:  char, varchar, text, 
> bit, timestamp, and tinyint.  All the other numeric and multibyte 
> character types have byte ordering and other issues when compared to 
> the MS ODBC driver.  Some of this may be particular to big endian 
> machines such as mine - I haven't dug into the next layer of code to 
> see how endian-ness is handled.
> 
> If we assume MS ODBC is the reference, here are the returned byte 
> orders for each type for my test case:
> 
> bigint:  actually returns 35 bytes! (MS ODBC correctly 
> returns 8 bytes)

Ok. I'll write a test and fix it.

> datetime:   3 2 1 0 7 6 5 4
> decimal:  correct length is 9, MS ODBC returns 19 bytes, FreeTDS 
> returns 35 bytes

Well, FreeTDS support also ASA that require more precision. However I
think we should translate to ODBC numeric. It's correct what MS ODBC do
(extending wire).

> float:  7 6 5 4 3 2 1 0
> int:  3 2 1 0
> money:  3 2 1 0 7 6 5 4
> nchar, ntext, nvarchar:  only the first byte of each character is 
> returned (I assume this is an artifact of Latin-1 iconv 
> conversion, but 
> MS ODBC gives you access to the raw double byte characters)

Mm.... here the behavior is different. We convert characters as we get
from wire so the behavior it's exactly the same of varchar/char/text.

> numeric:  same as decimal
> real:  3 2 1 0
> smalldatetime:  1 0 3 2
> smallint:  1 0
> smallmoney:  3 2 1 0
> 

Keep in mind that endianism change the order...

> I still have to write a test case for binary, image, 
> uniqueidentifier, 
> and varbinary.
> 
> Jonathan Monroe
> 

I'll extend genparams.c test to catch these conversions (and perhaps
change name to data.c)

freddy77