FreeTDS Development Group

[Richard Hughes <cyreve AT gmail.com>]

On 2015-03-15 at 11:46, Frediano Ziglio wrote:
> 2015-03-13 21:09 GMT+00:00 Richard Hughes <cyreve AT gmail.com>:
>> I've been looking at the conversion code today and the only conclusion
>> I've reached is that everything I did is wrong. I now assume that I
>> should have used odbc_sql2tds() as my template but that's full of
>> ipd/apd handling that bcp simply doesn't have. Since the first bug I
>> noticed involved nvarchar, I was trying to figure out how the
>> character set conversion was supposed to work and now I'm more lost
>> than when I started.
>
> Perhaps I was too direct. I don't think what you did is wrong. First
> bcp support is quite strange in ODBC. First is a sql server only
> extension on a quite standard API. Second it use types more similar to
> dblib than ODBC. Looks like Microsoft copied bcp source into ODBC so
> you did actually the same. Nothing wrong in it. I don't know actually
> how MS handle nvarchar in bcp code. We can surely help with
> conversions. Perhaps you should focus on a problem at a time, you are
> starting solving too much stuff altogether.

Here's a v2 patch, with nvarchar and decimal now working. You were
right - I wasn't as wrong as I thought I was (at least from the point
of view of just getting it working).

>> Hopefully everything except the datetime2/numeric conversion can be
>> put into libTDS, but I'm neither knowledgeable enough nor skilled
>> enough to be able to participate in an architectural discussion. If
>> you tell me what you're planning to work on and what you need me to
>> change then I'll happily do as I'm told. In the mean time I've got a
>> lot of tests to write.
>
> Good, tests are always good. You can have a look at dblib t0016 and
> t0017 tests just to get some code to copy/paste. What the problem with
> datetime2/numeric ?

I ended up mostly stealing from dblib's predictably-named 'bcp.c'
test. It does a lot of useful stuff, and the version in the new patch
now passes for me.

That tests most server-side datatypes, so next on the list is testing
conversions from client-side datatypes. Problem is, I can't find
anywhere which documents actually what the structures you're supposed
to pass are. I've proven by experiment that Microsoft's driver expects
SQL_TIMESTAMP_STRUCT/SQL_NUMERIC_STRUCT for datetime2/numeric
respectively, but it's going to be really tedious to determine all the
types simply by trying them out on Microsoft's driver. Oh well.

>>>> The HDBC problem should be discussed. Because the driver manager has
>>>> no idea about the bcp APIs, the first parameter that all the functions
>>>> receive is a driver manager HDBC, not a TDS_DBC*.
>>>> SQLGetInfo(SQL_DRIVER_HDBC) can be used to convert, but that means we
>>>> need to find SQLGetInfo. Approaches I've thought of so far are:
>>>>
>>>> 1) dlopen(...,RTLD_NOLOAD) with various hard-coded filenames to try to
>>>> find which driver manager the application is using
>>>>
>>>> 2) dl_iterate_phdr() to look for filenames that look like the driver
>>>> manager [this is what's currently in the patch].
>>>>
>>>> 3) dlsym(RTLD_DEFAULT,...).
>>>>
>>>> 4) Create a gratuitous libtdsodbcbcp.so containing just thunker
>>>> functions whose sole purpose is to get out of the way of the default
>>>> symbol search path so that method (3) works more reliably.
>>>>
>>>> Microsoft's Windows driver uses (1) because they know there's only one
>>>> driver manager. (3) is subject to ordering problems with the symbol
>>>> search, but those problems exist anyway at link time so I'm beginning
>>>> to think that might be best.
>>>>
>>
>>> (3) should be fine as odbc drivers (like our one) should be loaded
>>> with RTLD_LOCAL flag so the SQLGetInfo from our driver is not found
>>> with RTLD_DEFAULT.
>>>
>>> Another idea would be to use a sort of "tunnel" using functions like
>>> SQLSetConnectAttr. Practically let's say we want to implement
>>> bcp_collen, we do something like
>>>
>>> typedef struct  {
>>>   DBINT cbData;
>>>   INT idxServerCol;
>>> } bcp_collen_params;
>>>
>>> RETCODE bcp_collen (
>>>         HDBC hdbc,
>>>         DBINT cbData,
>>>         INT idxServerCol)
>>> {
>>>    bcp_collen_params params = { cbData, idxServerCol };
>>>     return SQLSetConnectAttr(hdbc, SQL_SS_FUNC_BCP_COLLEN, &params,
>>> sizeof(params));
>>> }
>>
>> This actually doesn't solve the problem. That call to
>> SQLSetConnectAttr() will bind to the one in odbc.c, not the one in the
>> driver manager - it's the same problem as trying to call SQLGetInfo()
>> directly. I'll go with my option (3). It has the surprising property
>> that if you link with "gcc -ltdsodbc -lodbc" then you'll bind directly
>> to FreeTDS and be confused that attempting to use any other driver
>> doesn't work, but we'll just have to document that you must use "gcc
>> -lodbc -ltdsodbc".
>
> Well, the wrapper should be statically linked to your application so
> it can call SQLSetConnectAttr as any other normal DM function. Just my
> idea. We can work on this problem while working on others.

Ah, kind of like my option (4), but with a static library instead of a
dynamic one.


I'm beginning to consider my patch nearly 'done', in that it works
more-or-less reliably as a standalone piece of functionality (and
includes at least the functionality that I need for my application). I
expect one more version with more tests (as mentioned above) and that
might cause a little surgical bug fixing but no massive overhauls.

If you could do a full review of this version and tell me where I can
tidy it up, I'd appreciate it. I'm not too optimistic about being able
to de-duplicate this bit of functionality because, while the
high-level look is the same as dblib, just about every detail has had
some amount of tweaking. When file I/O comes in then that will no
longer be true, but currently it's a tricky problem.

Richard.

Attachment: freetds-odbc-bcp-2.patch.gz
Description: Binary data