[Gambas-devel] Change to gb.db.odbc to use ODBC Connection Strings.

[ML]

On 2015-09-16 12:19, Benoît Minisini wrote:
> Le 16/09/2015 16:36, ML a écrit :
>> On 2015-09-15 21:24, Benoît Minisini wrote:
>>> Le 14/09/2015 17:41, ML a écrit :
>>>> Benoît,
>>>> Continuing here as you requested.
>>>> I found the problem in th driver that causes it to return 0 instead 
>>>> of -1 in the RowCount. The offending line and my patch:
>>>>    //20150914 - zxMarce: Do NOT mark the STMT as Scrollable; it 
>>>> makes SQLRowCount always return 0 rows instead of -1.
>>>>    //retcode = SQLSetStmtAttr(odbcres->odbcStatHandle, 
>>>> SQL_ATTR_CURSOR_SCROLLABLE, (SQLPOINTER) SQL_SCROLLABLE, 0);
>>>>    retcode = SQLSetStmtAttr(odbcres->odbcStatHandle, 
>>>> SQL_ATTR_CURSOR_SCROLLABLE, (SQLPOINTER) SQL_NONSCROLLABLE, 0);
>>>> It just needed a constant change from SQL_SCROLLABLE to 
>>>> SQL_NONSCROLLABLE.
>>>> Looks like the intention was to make a proper driver, with forward 
>>>> and back scroll, but for some reason it was not completed.
>>>> Now the problem propagates to Result.MoveNext and Result.Available.
>>>> They loop forever, even when there's no more data to fetch.
>>>> I made a further change in query_fill (removed a GB.Error that 
>>>> raised 'ODBC_NO_MORE_DATA' and added a return TRUE), but that did 
>>>> not get .MoveNext or .Available fixed:
>>>>    if((retcode2 == SQL_NO_DATA_FOUND) || (retcode2==SQL_NO_DATA))
>>>>    {
>>>>      //GB.Error("ODBC_END_OF_DATA!"); //20150914 - zxMarce: Removed 
>>>> so .MoveNext and .Available work?
>>>>      return TRUE; //20150914 - zxMarce: Try to make .MoveNext fail 
>>>> when no more data
>>>>    }
>>>> I think when we (you?) fix these problems, we will finally have a 
>>>> working Gambas ODBC subsystem.
>>>> Thanks,
>>>>
>>> gb.db.odbc tells Gambas if he has a function to seek through a 
>>> record at line 564 (with the no_seek connection flag).
>>> But having that function does not mean that for a specific ODBC 
>>> driver, seeking is actually possible.
>>> In the query_fill() function at line 1152 you will see what the 
>>> driver does:
>>> - If the seek function exist, then:
>>>    - If the result has been marked scrollable successfully, then 
>>> move to the specified position.
>>>    - If the result is not scrollable, then move to the next record.
>>> - Otherwise, if the seek function does not exist, then move to the 
>>> next record. If moving to the next record was not requested ('next' 
>>> argument), then return TRUE (meaning an error).
>>> So, apparently, it's just Gambas that does not handle the no_seek 
>>> connection flag at the moment.
>>> And we should force the query count to be -1 when seeking is not 
>>> possible, whatever the reason (no seek function, or unscrollable 
>>> query result). 
>> Benoît,
>> I've been investigating and came to the same conclusion you say 
>> regarding how the nested IFs work in query_fill.
>> Unfortunately, making no_seek TRUE is not the answer (tried it), but 
>> setting the ODBC Statement Handle's SQL_ATTR_CURSOR_SCROLLABLE 
>> attribute to SQL_NONSCROLLABLE instead of SQL_SCROLLABLE makes the 
>> trick of returning -1 as rowcount. I don't like forcing flag values 
>> because they may come in handy for other purposes later. On the other 
>> hand, the ODBC Statement Handle is created once per Connection.Exec 
>> call, so I'm more comfortable with changing that.
>> The problem that arises now is making Result.MoveNext (and preferably 
>> Result.Available also) work as it (they) should.
>> Changing the SCROLLABLE to NONSCROLLABLE achieves, then, 2 things:
>> 1- The rowcount returned is -1.
>> 2- The Result object has data and is not Null (with a rowcount=0 the 
>> Result object raises a "Result is not available" error).
>> Point 2 is what actually enables fetching data. The problem is now 
>> when all data has been retrieved: Result.Available is (still) TRUE 
>> and any subsequent Result.MoveNext raises an awful "ODBC_END_OF_DATA" 
>> error.
>> This not only forces the programmer to resort to convoluted 
>> techniques to fetch the data, it will also make any data-aware 
>> control fail if they rely on .MoveNext and .Available working.
>> What I did find, though, is that CResult.c is calling 
>> THIS->driver->Result.Fill and discarding the return value. Does not 
>> look wise, as the driver (at least gb.db.odbc) is actually returning 
>> whether the fill was or not successful.
>> I added another three changes to CResult.c that fixed the problem and 
>> made Result.Available work, but Result.MoveNext still raises the 
>> error, although trappable by Try res.MoveNext() in Gambas:
>>    //20150916 zxMarce: Make .Available depend on the result of 
>> driver's query_fill
>>    bool myUnavailable = FALSE;
>>    //20150916 zxMarce: Make .Available depend on the result of 
>> driver's query_fill
>>    /* THIS->driver->Result.Fill(&THIS->conn->db,
>>                                             THIS->handle,
>>                                             pos,
>>                                             THIS->buffer,
>>                                             (pos > 0) && (pos == 
>> (DELETE_MAP_virtual_to_real(THIS->dmap, THIS->pos) + 1))
>>                                            ); */
>>    myUnavailable = THIS->driver->Result.Fill(&THIS->conn->db,
>> THIS->handle,
>>                                                            pos,
>> THIS->buffer,
>>                                                           (pos > 0) 
>> && (pos == (DELETE_MAP_virtual_to_real(THIS->dmap, THIS->pos) + 1))
>> );
>>    //20150916 zxMarce: Make .Available depend on the result of 
>> driver's query_fill
>>    //THIS->available = TRUE;
>>    THIS->available = !myUnavailable;
>>    return FALSE;
>>
>> I now can fetch data without resorting to unnecessary error handling, 
>> although fixing the error risen by Result.MoveNext and making it 
>> return TRUE instead is still pending.
>> I have to clean up the code a little bit and I think I can send it so 
>> you can check it. I don't know if the "myUnavailable" patch above 
>> will break other drivers' functionality, and have no other databases 
>> than MSSQL to test.
>
> If the ODBC driver can "scroll" its query result, we must use that. 
> Why forcing the NONSCROLLABLE flag?
> As for the rest, I must first modify the gb.db component to add 
> support for "move forward only" Result object, otherwise your patches 
> are just hacks. I will tell you...
> Regards,
Benoît,

Ok, I think I misunderstood your meaning (and maybe I still do!). If I 
can help in any way, just say the word.
I just forced the attribute because it looked to me that at least MSSQL 
does not work with unixODBC correctly and I wanted to make the driver as 
general as possible.

But, rereading your post, you say that it may be Gambas misbehaving, so 
I'll wait for you.
In the meantime, I'll use what I already have because it works and I 
need it. I will not publish my changes other than here as I already did.

Thanks,
zxMarce.