FreeTDS Development Group

["ZIGLIO Frediano" <Frediano.Ziglio AT vodafone.com>]

> 
> On Wed, 8 Oct 2003, "ZIGLIO Frediano" <Frediano.Ziglio AT vodafone.com>
> wrote:
> > 
> > Every TDSCOLINFO has a iconv_info field. This for 
> compatibility with 
> > mssql2k (where every column can have a different type). We 
> don't use 
> > this field very much. I think we should set this field for
> > every column that require a conversion (even using TDS5!) and adjust
> > column size even for single byte column (like VARCHAR, not only
> > NVARCHAR). In this way we can just use this pointer in 
> > tds_get_char_data (read.c). 
> 
> Yes.  That was my intention.  
> 
> One thing I've learned lately: there has to be a way to turn 
> off single-byte conversions.  The client might want the data as-is.  
> 

Mmm... when should conversions be turned off ??

> > This pointer should point to a structure in 
> tds->iconv_info, however 
> > this array must be resized so the pointer could change.
> 
> Yes, but resizing is avoidable.  See below.  
> 
> > solution should use a TDSICONVINFO ** for tds->iconv_info 
> and allocate 
> > every TDSICONVINFO separately.
> 
> That's inefficient.  For a given connection, there are only a 
> few conversion pairs.  The client's charsets are invariant, 
> and the server normally encodes character data in only one or 
> two ways (that a single client cares about).
>  
> 
> For a connection, I expect the TDSICONVINFO array to be 4-6 
> elements, practically never more than 10.  If you allocate 
> per-column, you'll allocate (and deallocate) thousands of 
> TDSICONVINFO structures during the lifetime of a connection.  
> Instead of 5 or so.  
> 

Not allocated by column, only per-connection. Just every column point to
a connection conversion.

> > At this point I suggest to replace 
> iconv_info_count/iconv_info with a 
> > iconv_info_private (or something like
> > that) structure. So we separate TDSICONVINFO handling in iconv.c
> > entirely (using iconv_info in column we avoid the need to access to
> > tds->iconv_info as an array).
> 
> If TDSCOLINFO::TDSICONVINFO is set correctly, it's not 
> necessary for column processing to have access to the socket. 
>  We're really just talking about allocation, not scope.  
> 
> The only time the array size changes is when a new conversion 
> is required by a column, while processing a ROW_FMT packet.  
> It's not hard to modify the array first (if necessary) and 
> then set each column's TDSICONVINFO pointer.
> 
> 
> > Time ago I was thinking about changing TDSICONVINFO 
> structure removing 
> > charset names and just storing our canonic index. In such way we 
> > reduce memory usage and improve performance.
> 
> I like having the string, because it makes debugging easier.  
> I agree that comparing two ints is faster than strcmp(), but 
> I really don't thinks it matters.  Perhaps we can put this 
> off until version 0.70 or something, when iconv is completely 
> stable and well understood?  
> 

It's not hard to modify and we have a static array to convert from
numeric to string. I think 0.63 version.

> > Another idea using directly TDSICONVINFO (always as a 
> pointer, without 
> > pointer operations) is the possibility to define a private 
> > TDSICONVINFO separating other possible iconv implementations.
> 
> I don't think I know what you mean here.  What "other 
> possible iconv implementations" are you thinking of?  
> 
> Regards, 
> 

Just for header cleanup... can wait...

Let's some code speak... see patch attached (still some problems with
some unit tests)

freddy77

Attachment: vedi.diff.gz
Description: vedi.diff.gz