[sr-dev] db_sqlite branch review request

[Timo Teräs]

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