Re: affected rows after insert

2015-11-03 11:56 GMT+00:00 David Nichols <[email protected]>:
> On Oct 20, 2015, at 11:17 AM, David Nichols <[email protected]> wrote:
>>
>>>
>>> On Oct 20, 2015, at 9:12 AM, Frediano Ziglio <[email protected]> wrote:
>>>
>>> Il 20/Ott/2015 09:09, "David Nichols" <[email protected]> ha scritto:
>>>>
>>>> On Oct 19, 2015, at 10:20 PM, Frediano Ziglio <[email protected]> wrote:
>>>>>
>>>>> 2015-10-19 13:35 GMT+01:00 David Nichols <[email protected]>:
>>>>>>
>>>>>>> On Oct 19, 2015, at 1:53 PM, Frediano Ziglio <[email protected]>
>>> wrote:
>>>>>>>
>>>>>>> 2015-10-19 11:16 GMT+01:00 David Nichols <[email protected]>:
>>>>>>>>
>>>>>>>>> On Oct 18, 2015, at 10:06 AM, Frediano Ziglio <[email protected]>
>>> wrote:
>>>>>>>>>
>>>>>>>>> 2015-10-16 12:23 GMT+01:00 David Nichols <[email protected]>:
>>>>>>>>>>> 2015-06-09 14:48 GMT+01:00 Ondrej Holecek <ondrej.holecek at
>>> qoretechnologies.com <http://lists.ibiblio.org/mailman/listinfo/freetds>>:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I made a test with the same code but sybase lib and it works like
>>>>>>>>>>>> expected. It seems there is a bug in FreeTDS. Could you please
>>> check
>>>>>>>>>>>> my patch?
>>>>>>>>>>>>
>>>>>>>>>>>> thx,
>>>>>>>>>>>>
>>>>>>>>>>>> Ondrej
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Actually works and ctlib tests seems to pass.
>>>>>>>>>>>
>>>>>>>>>>> However DONEINPROC is returned in a lot of cases, for instance if
>>>>>>>>>>> database execute some trigger on the table so your patch can catch
>>>>>>>>>>> some strange results too.
>>>>>>>>>>>
>>>>>>>>>>> Which database version are you using?
>>>>>>>>>>>
>>>>>>>>>>> Frediano
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Ondra has not been working on this for quite some time, but I’d
>>> like to pick up this issue again please.
>>>>>>>>>>
>>>>>>>>>> We are using MS SQL Server 2008 - using tds version = 7.2  - with
>>> the patch we are able to get the affected rows reliably - without it, we
>>> are not.
>>>>>>>>>>
>>>>>>>>>> Using the same patched library build with a Sybase (SAP) ASE 16.0
>>> DB, it also gives correct results (with tds version = 5.0)
>>>>>>>>>>
>>>>>>>>>> I would be happy to continue helping with this issue if there’s a
>>> chance it could be included in freetds (or some other equally effective
>>> fix).
>>>>>>>>>>
>>>>>>>>>> thanks,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> A fix was written for this case. Do you still have some code that
>>>>>>>>> fails? Did you update FreeTDS?
>>>>>>>>>
>>>>>>>>> Frediano
>>>>>>>>
>>>>>>>> yes, I tried it with freetds from freetds-0.95.12 and git master -
>>> both failed my tests and with the patch as provded by Ondra the tests then
>>> passed.  Sorry I did not mention this in the original email.
>>>>>>>>
>>>>>>>
>>>>>>> Please try a more recent version (like 0.95.21). Also you should sent
>>>>>>> some extract of your tests. Can you post an updated patch?
>>>>>>>
>>>>>> sorry it was a typo on my side - it was 0.95.21 that I tried.  Anyway
>>> since it failed in git master then I would assume that would be more
>>> relevant than the last stable release.
>>>>>>
>>>>>> Attached please find a patch to git master sources that makes my tests
>>> work.  Please note that we are using ctlib from freetds to build a DBI
>>> driver for the Qore programming language to talk to SQL Server and Sybase
>>> DBs; the same source builds with both the sybae libs and ctlib from
>>> freetds.  Currently my tests are written in the Qore programming language,
>>> so there are quite a few layers between the Qore code that’s executed and
>>> freetds’s ctlib.
>>>>>>
>>>>>> When this simple test works (ie only with a patch ctlib); the output
>>> looks like this:
>>>>>>
>>>>>> dnichols@manatee:~/src/qore/git/module-sybase/src$
>>> simple-rows-affected-test.q
>>>>>> "Microsoft SQL Server 2008 (RTM) - 10.0.1600.22 (X64)
>>>>>>      Jul  9 2008 14:17:44
>>>>>>      Copyright (c) 1988-2008 Microsoft Corporation
>>>>>>      Developer Edition (64-bit) on Windows NT 6.1 <X64> (Build 7601:
>>> Service Pack 1)"
>>>>>> insert: 1
>>>>>> n delete: 0
>>>>>> sel: {id: [1]}
>>>>>> delete: 1
>>>>>> insert: 1
>>>>>> delete: 1
>>>>>>
>>>>>> When it doesn’t work (ie with an unpatched ctlib), the output looks
>>> like this:
>>>>>> dnichols@manatee:~/src/qore/git/module-sybase/src$
>>> simple-rows-affected-test.q
>>>>>> "Microsoft SQL Server 2008 (RTM) - 10.0.1600.22 (X64)
>>>>>>      Jul  9 2008 14:17:44
>>>>>>      Copyright (c) 1988-2008 Microsoft Corporation
>>>>>>      Developer Edition (64-bit) on Windows NT 6.1 <X64> (Build 7601:
>>> Service Pack 1)"
>>>>>> insert: 0
>>>>>> n delete: 0
>>>>>> sel: {id: [1]}
>>>>>> delete: 0
>>>>>> insert: -1
>>>>>> delete: -1
>>>>>>
>>>>>> The Qore source to this simple test is attached for your reference.
>>>>>>
>>>>>
>>>>> I applied the patch and improved a test to catch this issue. I changed
>>>>> the patch slightly to check for TDS_NO_COUNT instead of <0 and
>>>>> returning CS_CMD_SUCCEEDED as ct_res_info documentation
>>>>
>>>> My test fails with your patch; if I change it back to:
>>>>                               case _CS_RES_INIT:   /* command had no
>>> result set */
>>>>                                       if (tds->rows_affected !=
>>> TDS_NO_COUNT) {
>>>>                                               *result_type =
>>> CS_CMD_DONE;
>>>>                                               cmd->results_state =
>>> _CS_RES_INIT;
>>>>                                               return CS_SUCCEED;
>>>>                                       }
>>>>                                       break;
>>>>
>>>> then it succeeds - please note that his is a hack from my side, I’m not
>>> saying that the original approach is correct, just that my test fails
>>> unless I set CS_CMD_DONE
>>>>
>>>> thx
>>>> David
>>>>
>>>
>>> Cs_cmd_succeeded is a valid value and is expected to have rows number
>>> attached so upper layer should handle it
>>
>> OK, I see what you mean, however please note that with ctlib from Sybase/SAP ASE 16, I get the rows updated correctly with the same driver code as I use with freetds, and now it seems that I’ll need to update the driver to have a separate code path when it’s built with freetds - which seems to defeat the purpose of freetds’s ctlib.  Is it not possible to fix this issue and provide 100% backwards-compatibility with Sybase’s ctlib?
>>
>
> @Frediano: sorry to bother you with this issue, but since it’s important for me, can you please let me know about the above - should I plan on having 2 code paths in my DB driver - one for Sybase and one for FreeTDS, or should there ideally be a way that compatibility between Sybase’s ctlib and FreeTDS’s ctlib can be provided regarding this issue?
>
> thx
> David
>

Yes, it's possible. But as said is not that easy. They way query is
sent is actually different. Also it could break some compatibility. I
think this however should be addressed for next release (which I hope
will be before next year).

I added a note in my TODO list (https://trello.com/b/bk0UZNRJ/freetds-todo-list)

Frediano
_______________________________________________
FreeTDS mailing list
[email protected]
http://lists.ibiblio.org/mailman/listinfo/freetds