FreeTDS Development Group

[Aaron Ucko <ucko AT ncbi.nlm.nih.gov>]

OK, I created a pull request, with a few technical comments on metadata:

https://github.com/FreeTDS/freetds/pull/219

-- Aaron Ucko <ucko at ncbi>, NCBI C++ Toolkit core development group

"Ucko, Aaron (NIH/NLM/NCBI) [E] via FreeTDS" <freetds AT lists.ibiblio.org>
writes:

> Thanks for being open to these changes!
>
> I did get this first reply, but was having a busy week and wanted to take a 
> closer look at the commits in which I patched unit tests before following 
> up.  Most of my commits to unit tests turned out to be pure formalities or 
> setup fixes, but there is one commit that could well be an issue here:
>
>     cs_convert, dbconvert: Truncate data as needed; update unit tests.
>     
>     * Automatically truncate strings that would overflow their destinations,
>       mainly for the sake of bulk insertions.  Notify the client at low
>       severity (as a warning, essentially).  Update upstream unit tests
>       (ctlib t0006, dblib t0019) accordingly.
>     * Fully honor *BINARY and IMAGE data buffer size limits in cs_convert.
>
> Running these tests against Sybase libraries (12.5, 15.7, and 16.0, with 
> identical results in all cases), I see that these changes do go against 
> upstream, but that there are already several other differences on this 
> front for each API (details attached).  I'll pull this commit out, at least 
> for now.
>
> FWIW, there was also one commit for which I had to patch 
> src/tds/unittests/convert.c, but not any higher-level unit tests:
>
>     ctlib: Implement full Unicode support for TDS 7+ (MSSQL).
>     
>     * Introduce CS_NLONGCHAR_TYPE and CS_NVARCHAR_TYPE, both corresponding 
> to
>       SYBNVARCHAR (along with CS_UNICHAR_TYPE); have ct_param automatically
>       promote CS_{LONG,VAR}CHAR_TYPE to CS_N{LONG,VAR}CHAR_TYPE for TDS 7+
>       when non-ASCII characters appear in the data.  (Unnecessary use of
>       Unicode types can make parameterized queries against 8-bit text
>       columns run very slowly.)
>     * ctlib/cs.c (cs_convert): Formally support Unicode destination types
>       (SYBNVARCHAR and SYBNTEXT).
>     * ctlib/ct.c (paraminfoalloc): When setting maximum length 
> automatically,
>       account for possible conversion to UCS-2.
>     * tds/convert.c (tds_convert), tds/tds_willconvert.*: Formally support
>       Unicode types, giving type 103 (0x67) the benefit of the doubt as
>       SYBNVARCHAR rather than SYBSENSITIVITY.
>     * tds/data.c (tds_set_param_type): Map SYBNVARCHAR to XSYBNVARCHAR for
>       TDS 7+.
>     * tds/unittests/convert.c: Accommodate Unicode types, converting ASCII
>       input strings to (native-endian, is this correct?) UCS-2 as needed.
>
> -- Aaron Ucko <ucko at ncbi>, NCBI C++ Toolkit core development group
>
> -----Original Message-----
> From: Frediano Ziglio [mailto:freddy77 AT gmail.com] 
> Sent: Wednesday, June 20, 2018 04:58
> To: FreeTDS Development Group <freetds AT lists.ibiblio.org>
> Cc: Ucko, Aaron (NIH/NLM/NCBI) [E] <ucko AT ncbi.nlm.nih.gov>
> Subject: Re: [freetds] Reintegrating years of local FreeTDS changes
>
> Hi,
>    a pull request seems good.
> Didn't read the full list, I expect some to be small, some other larger.
> I usually keep the vendor behavior if I can.
>
> Regards,
>    Frediano
>
>
> 2018-06-06 22:28 GMT+01:00 Ucko, Aaron (NIH/NLM/NCBI) [E] via FreeTDS
> <freetds AT lists.ibiblio.org>:
>> Long story short, I am the primary maintainer of a convenience (aka
>> vendorized) copy of FreeTDS's client libraries that has accumulated some
>> local changes over the years.  I have now forward-ported these changes
>> to your master branch and would belatedly like to submit them for your
>> consideration, both because they may also be of interest to other users
>> and to make future releases easier to integrate.
>>
>> Some of the developers who contributed to (the original versions of)
>> these changes have moved on to other jobs, but we were all working on
>> behalf of the US federal government, which cannot hold copyright, so
>> there shouldn't be any legal concerns; I'm just citing them in (so far
>> unpublished) Git metadata to give credit where credit's due.
>>
>> I tested these changes via your test suite against both Microsoft SQL
>> Server and Sybase ASE, and observed no regressions, just one failure
>> that turned out to be preexisting (specifically, of the ODBC utf8_2 test
>> against Microsoft SQL Server).  We currently have Microsoft SQL Server
>> 2014 and 2017 and Sybase ASE 15.5 and 16.0 on hand, and additionally
>> tested many of these changes against older server versions too.  That
>> said, in a few cases (some predating my involvement with this code), we
>> made deliberate changes to semantics.  In such cases, I've updated the
>> test suite accordingly.  However, if you don't agree with these changes,
>> I can of course instead arrange to make them conditional in some
>> fashion; please let me know whether I should do so for any or all of
>> these changes.
>>
>> I have grouped these changes into 59 patches.  Please don't take this
>> count as criticism of your code, which works well for the most part;
>> we've just been sitting on our changes for entirely too long and
>> gradually accumulating more along the way.  Also, I suspect our needs
>> are somewhat atypical -- we use the CT-LIB interface, and do so via a
>> home-grown abstraction library rather than directly from application
>> code.
>>
>> How would you like me to proceed?  Should I send the patches here, or
>> put them online through a GitHub fork?  In the latter case, should I
>> proceed to issue a formal pull request?
>>
>> Here's a quick rundown:
>>
>> * Fix miscellaneous configuration-specific issues, mostly on Windows.
>> * Enable automatic TDS protocol version detection.
>> * thread.h (tds_mutex_trylock): Disallow recursive locking on Windows.
>> * tds_iconv: Accommodate FreeBSD/Citrus iconv.
>> * net.c (tds_socket_write): Don't let fake short writes break headers.
>> * Fix unit tests that manipulate raw network connections.
>> * cancel.c: Skip return_data tests with Sybase 15+.
>> * numeric.c: Don't force TDS 5.0 (DBVERSION_100).
>> * threadsafe.c (tds_getservice): Set AI_CANONNAME where available.
>> * dblib.c: Honor dbsetversion; improve dbcolinfo and dbwritetext.
>> * odbc.c (SQLSetEnvAttr): Uncomment SQL_ATTR_OUTPUT_NTS-handling code.
>> * ct.c (ct_describe): Populate datafmt->format per Valgrind.
>> * connectparams.c (odbc_parse_connect_string): Handle client_charset.
>> * ct.c (ct_connect): Set client_charset in addition to server_charset.
>> * tds_process_tokens: For DONEPROC, always restore rows_affected.
>> * Implement blk_textxfer, over a resumable tds_bcp_send_record.
>> * query.c: Handle return codes of tds_put_data and tds_put_data_info.
>> * net.c: Improve handling of certain error conditions.
>> * ctlib: Handle CS_TIMEOUT and CS_LOGIN_TIMEOUT.
>> * ct.c (_ct_get_server_type): Handle CS_LONGCHAR_TYPE.
>> * blk.c (_blk_get_col_data): Use on-server size for destfmt.maxlength.
>> * ct.c (ct_cursor): Predict cursor type for CS_CURSOR_DECLARE.
>> * Fix server timeout handling and ctlib severity propagation.
>> * Don't log directly to stdout or stderr.
>> * blk.c: Handle CS_BLK_CANCEL in blk_done.
>> * cs_convert, dbconvert: Truncate data as needed; update unit tests.
>> * ct.c (ct_data_info): Encode/decode CS_IODESC datatype field.
>> * ctlib: Work with non-NULL empty strings if the server supports them.
>> * ctlib: Implement full Unicode support for TDS 7+ (MSSQL).
>> * Improve support for TDS 5 (Sybase), mainly around bulk transfers.
>> * Improve TDS 5 (Sybase) LONGBINARY and LONGCHAR handling.
>> * Address miscellaneous compiler warnings.
>> * Report system errors' descriptions to ctlib clients.
>> * mem.c (tds_deinit_bcpinfo): Free and reset current_row for xfers in.
>> * read.c (tds_get_n): Avoid potential hangs on short replies.
>> * tds_checks.c (tds_check_column_extra): Skip size check for BLOBs.
>> * Address Clang static analyzer errors, many long-standing.
>> * Improve CTLIB support for TDS 7.2+.
>> * cs.c (cs_convert): Fix *resultlen for conversions to SYBIMAGE.
>> * bulk.c (tds_bcp_send_record): Reject invalid NULL data.
>> * ctlib: Optimize blob conversion for bulk transfers in.
>> * Declare and implement blk_sethints(_ver95).
>> * data.c (tds_generic_row_len): Account for column_varint_size.
>> * ct.c (ct_get_data): Populate cmd->iodesc->name more efficiently.
>> * mem.c (tds_init_login): Enable use_utf16 by default.
>> * query.c (tds_fix_column_size): Allow full size for output columns.
>> * login.c (SET_FIELD_DSTR, for tds7_send_login): Cap lengths at 128.
>> * tds/u.t./common.c (try_tds_login): Pass base name to tds_set_app.
>> * Address more issues caught by LLVM/Clang scan-build.
>> * odbc.c (odbc_process_tokens): Pre-zero done_flags per Valgrind.
>> * threadsafe.c (tds_get_homedir/_WIN32): Initialize path.
>> * data.c (tds_set_param_type): Under TDS 7.1+, send SYBBIT as SYBINT1.
>> * ct.c (paraminfoalloc): Report long variable-width values.
>> * packet.c (tds_write_packet): fix a TDS_ADDITIONAL_SPACE corner case.
>> * tds5_process_dyn_result2: Don't trust Sybase to accept blob params.
>> * net.c (tds_close_socket): Don't try to close invalid sockets.
>> * stream.c (tds_datain_stream_read): Bail if tds_get_n returns NULL.
>> * login.c (tds_connect): Skip non-TCP tries to connect to a host:port.
>> * Address gaps in ctlib support for new MS date/time types.
>>
>> --
>> Aaron Ucko <ucko at ncbi>, NCBI C++ Toolkit core development group