[Kamailio-Devel] [ openser-Patches-2139104 ] Fetch support db_unixodbc

Thu Oct 9 00:15:22 CEST 2008

Patches item #2139104, was opened at 2008-09-30 23:03
Message generated for change (Comment added) made by nobody
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743022&aid=2139104&group_id=139143

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: ver devel
Status: Closed
Resolution: Accepted
Priority: 5
Private: No
Submitted By: Jerome Martin (tramjoe)
Assigned to: Henning Westerholt (henningw)
Summary: Fetch support db_unixodbc

Initial Comment:
This patch implements fetch support for db_unixodbc.
It currently works for me with very large dialog tables (> 50 000 dialogs). Patch is against 1.4 rev 4989. 

Oh, BTW the tracker group 1.4.x does not exist for patches. Please add it :-)

I do not consider this patch production ready yet, but submit it for review regarding the following:

It seems that fetch support in db_mysql and db_postgresql supports the assumption in dialog module, and I guess other modules, that the row count is known before copying every row. This can NOT be assumed with db_unixodbc, for various reasons :

- unixodbc library has an SQLRowCount function that :
  . works with select only if the backend supports it. i.e. the TDS backend (MS-SQL) does NOT.
  . returns the row count for select only after fetching all rows

So the way my implementation works is by allowing an extra fetch call when there are no more rows to fetch and exit with success from that. It does work with dialog, but I have NOT tested this with other modules. Also, there might be implications here that I do not currently see, please comment.

Also, I'd appreciate if someone takes a look at malloc/free operations, I think I might have something leaking a bit currently.

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2008-10-08 22:15

Message:
Thanks Henning for taking the time to test and commit :-)

For the record, let me complement here your comment from the commit: the
patch has been tested by me with a MS-SQL server 2000 and the FreeTDS
driver for unixodbc from 0 to 50000+ rows, with fetch_rows ranges from 1 to
470.

Jerome

----------------------------------------------------------------------

Comment By: Henning Westerholt (henningw)
Date: 2008-10-08 17:40

Message:
Hi Jerome,

thanks for the update, your patch looks good. I did also a few tests with
the usrloc module, seems to work flawless so far. I tested with odbc on
sqlite3, small to bigger fetch_row size, up to 20.000 entries.

I changed the one LM_INFO log at the end to of the fetch_result function
to LM_DBG, and remove a few superflous whitespaces, but i think i'll commit
your patch unchanged otherwise. I'll later add a automatic test for the
fetch_result on usrloc to the testsuite too.

Henning

----------------------------------------------------------------------

Comment By: Jerome Martin (tramjoe)
Date: 2008-10-06 14:05

Message:
File Added: fetch_support_db_unixodbc_v2.patch

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2008-10-06 14:04

Message:
OK, here is the new patch.
I tested it with the dialog module, sweeping fetch count from 1 to 460
(the maximum number of dialogs I can fetch with 1MB pkg memory) and 0 to
11000 dialogs in DB, it works fine for me. The previous bugs I found are
solved. Also, this patch now uses a code closer to what is done in mysql
and postgres modules, aka only one big function in dbase.c. I tried to
mimic the log, error condition return codes and general coding style of
those modules implementations too. That implied exporting prototypes in
res.h and thus remove static inline from some unixodbc functions.

Please test and advise.

----------------------------------------------------------------------

Comment By: Henning Westerholt (henningw)
Date: 2008-10-06 12:47

Message:
Removing the static inline of some db_unixodbc code is not a problem. I
don't think that applying indention fixes to all modules is worth the
effort, don't want to break compatibility (e.g. for patches) because of
this reasons. Its fine to apply them to code that anyway gets changed, like
in this case. I'll wait then for your updated version.

----------------------------------------------------------------------

Comment By: Jerome Martin (tramjoe)
Date: 2008-10-06 12:01

Message:
Hi Henning,

Several things : 
1/ I will release a better version soon (like today I think), grouping the
fetch functionality in a single function in dbase.c at the cost of removing
the "static inline" of some backend functions. Also the previous patch has
a malloc issue and works only by coincidence with number of fetch lines
~>50 but not smaller ones. In fact I did not guarantee that the fetched
rows will be at the same memory address, so for big mallocs it is the case
most of the time, but not for smaller ones ... 
2/ If you like the indentation fixes I throwed in (indented line
continuation instead of aligned on the line start), then I could apply it
to the whole module codebase easily, this is part of my vim configuration
:-)

----------------------------------------------------------------------

Comment By: Henning Westerholt (henningw)
Date: 2008-10-06 11:48

Message:
Hi Jerome,

thanks for the patch. I've already integrated the indention fixes for the
functions with long parameter lists that you've suggested. I attached the
updated patch against the trunk, will take a closer look in the next days.

With regards to the differences in the fetch_result logic: Yes, i know the
actual way of doing this in the mysql and postgres modules is not that
really straight forward. In previous versions they used to differ more, i
needed to do a few compromises to get them to this state in order to be
able to benefit from the common DB core API.. I'll also do some tests with
the carrierroute and usrloc module.

Henning
File Added: fetch_support_db_unixodbc-2.patch

----------------------------------------------------------------------

Comment By: Klaus Darilion (klaus_darilion)
Date: 2008-10-01 12:30

Message:
added "ver 1.4.x" to the tracker

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743022&aid=2139104&group_id=139143