On 06/01/2011 01:35 PM, Henning Westerholt wrote:
On Monday 30 May 2011, Timo Teräs wrote:
The db_sqlite module is now getting ready for merge to master... It would be nice if it'd get some scrutiny from other developer before the merge, though.
Hi Timo,
I've had a quick look, just noticed some smaller things:
You implement your own str handling functions (str_dup), there are some already defined in ut.h file ([pkg,shm]_str_dup), maybe they are suitable for you.
Those are slightly different. The str_dup and str_assign I have, take in a C-string, or a pointer+length. Where as the pkg_str_dup from ut.h takes const str* as source. I did try to look if suitable functions already exists but could not find with quick search. If there's one, let me know :)
You've added some documentation in the files for the database functions, maybe you can convert them to the doxygen format (have a look to the /lib/srdb1/* files for examples, its should be not that difficult).
I think the comments were modelled mostly after the original database module which I used as starting point. It didn't have any doxygen stuff so I didn't realise it's standard. I'll take a look at this.
In your GPL headers, you use the * $Id$ macro, this is not really necessary anymore in new modules as its not evaluated from git, maybe you can remove this.
Also leftover from the older module. Will remove.
One question related to the db_sqlite_raw_query function, does sqlite really support arbitrary SQL functions? For example does is support SELECT DISTINCT, which is one query that the cr module uses.
I thought the idea is to just pass the stuff to SQL interpreter so we can use whatever the database supports or not.
SELECT DISTINCT is supported according to http://www.sqlite.org/lang_select.html
- Timo