30 May
2011
30 May
'11
1:48 a.m.
Hi all,
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.
So far I've now addressed the earlier comments based on the original
patch, added the docbook notes / README, Natanael Copa contributed the
kamdbctl related work, and Henning Westerholt did the automation of the
SQL script generation from the XML schema.
Unless there's major things, I'm planning to merge with master late this
week.
Thanks,
Timo
1 Jun
1 Jun
6:35 a.m.
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.
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).
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.
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.
Cheers,
Henning
6:43 a.m.
On 06/01/2011 01:35 PM, Henning Westerholt wrote:
On Monday 30 May 2011, Timo Teräs wrote:
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 :)
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. 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
8:21 a.m.
On Wednesday 01 June 2011, Timo Teräs wrote:
Hi Timo,
I think they are some more in lib/cds/dstring.h or so - but they are mainly
used in presence modules, i think - not sure if they really fit.
Well, standard its maybe a bit too much, but already a lot of modules use
this: http://sip-router.org/doxygen/sip-router/branch/master/index.html
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. [..]
Yes, this is the idea. But I just wanted to ask because otherwise it would
break for people if you set this flag and the backend then not supports it.
:-)
Best regards,
Henning
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
4962
days inactive
4964
days old
3 comments
2 participants
participants (2)
-
Henning Westerholt -
Timo Teräs