FreeTDS Development Group

[Richard Hughes <cyreve AT gmail.com>]

On 2015-03-13 at 15:37, Frediano Ziglio wrote:
> 2015-03-12 19:36 GMT+00:00 Richard Hughes <cyreve AT gmail.com>:
>> The patch steals a large amount of code from dblib/bcp.c but, in
>> order to give myself an achievable goal, it implements *only* bcp in
>> from RAM; all of the file I/O stuff has been ripped out. I believe
>> this is a very sane and useful subset to have because my application
>> does the bcp as one step of a single larger transaction and hence
>> needs to use the existing ODBC connection. People wanting the file I/O
>> bit probably don't have this constraint and hence can just use dblib
>> or freebcp.
>>
>> A very useful document is [1] which lists the API differences from
>> dblib. The most annoying is probably that all errors must go through
>> SQLGetDiagRec rather than dblib's error handling, which is what
>> necessitated so much copy-and-paste. A future task is to look at how
>> this could be rationalized.

> Yes, coy and paste is not that great. I'm changing actually bcp.c
> (dblib) to use tds_convert instead of dbconvert. This could help
> moving code to libTDS (the common part) so we can reuse it. I remember
> was a nightmare extracting code from either dblib and ctlib in libTDS
> I don't want to do it again for dblib/odbc :)

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.

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.

> Well... probably using tdserror instead of dbperror could be a way.
> libTDS use callbacks and even for normal errors from network if calls
> these callbacks. In odbc these callbacks store errors in
> statement/connection to be retrieved by SQLGetDiagRec. I noted that
> bcp.c (used objdump) calls very few dblib functions (dbconvert,
> dbvarylen, dbperror and another I don't remember now) so I don't think
> will be much effort to move to libTDS. In bulk.c or another file.

Again, if you can divide up the work and tell me which bits you're
working on and which bits I should do then I can get started with a
minimum of merge conflicts. We might even get file I/O out of it this
way (although those BCPFILECP and BCPUNICODEFILE options look scary).
Remember to keep a browser tab with the API differences [1] open while
looking at what code can/should be shared.

>> 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".

Richard.

>> [1] https://msdn.microsoft.com/en-us/library/ms130924.aspx