[Home]

Summary:ASTERISK-06963: [branch] New custom SQLite3 CDR driver backend (cdr_sqlite3_custom)
Reporter:Alejandro Rios P. (alerios)Labels:
Date Opened:2006-05-12 15:45:59Date Closed:2007-03-13 16:27:33
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) cdr_makefile_diff_1.2.14.patch
( 1) cdr_makefile_diff_rev30651.patch
( 2) cdr_sqlite3_custom.c
( 3) cdr_sqlite3_custom.c-3
( 4) cdr_sqlite3_custom.conf
Description:I took ideas from cdr_custom.c, cdr_sqlite.c and bug-6384 (Convert cdr_addon_mysql to use custom cdr format).
This currently supports only one mapping, but I have tested it with different table names and different sets of custom columns/values and it was ok.

I hope this can be as usefull for other people as it has been for me.

****** ADDITIONAL INFORMATION ******

I attach the following files, all in sync with revision 27087 of trunk:

cdr/cdr_sqlite3_custom.c   (driver file)
/etc/asterisk/cdr_sqlite3_custom.conf  (example configuration file)
cdr_makefile_diff_rev27087.patch  (cdr/Makefile diff)

I also had to run the following commands to get it compiled:
./configure --with-sqlite3
make menuselect  (and select the driver on the CDR section)

Comments:By: Serge Vecher (serge-v) 2006-05-12 16:28:01

alerios: can you please get a disclaimer on file with Digium?

There as a patch in the bug tracker that also deals with SQLite 3 support for CDR. Can you please look at it (bug ASTERISK-6576) and see if there is a potential for collaboration here?

Thank you for contributing.

By: Alejandro Rios P. (alerios) 2006-05-12 16:55:56

vechers: I'm already printing the disclaimer so I can fax it tomorrow.

Regarding bug ASTERISK-6576, I think we are after different goals, since my first objective is to make sqlite cdr records in a custom way, like in cdr_custom.so.  Using sqlite or sqlite3 is just a secondary matter from my point of view, since only some function names and parammeters change between one version and the other.

Anyways, I'll stay tunned to see if there's a way to co-operate or something.

Thanks for the comments.



By: Moises Silva (moy) 2006-05-13 10:39:15

Vechers and alerios:

Since my needs are simpler, I think it makes sense to close my issue and keep this one. No problem with that :)

I just one this in Asterisk as soon as possible.

Regards

By: Alejandro Rios P. (alerios) 2006-05-16 19:35:03

vechers: the fax with disclaimer was sent today. ¿Do I need to do something else on this matter?

By: Serge Vecher (serge-v) 2006-05-25 21:43:58

alerios: can you please fix up the formatting
1) spaces in 'if((' => 'if (('
2) no curly brackets in single line if statements

thanks.

By: Alejandro Rios P. (alerios) 2006-05-26 18:02:33

vechers: Ok, I uploaded the new file cdr_sqlite3_custom.c.indent modified based on the indent command on [1]. I also updated the makefile patch to the 30651 revision of trunk.

Thanks.

[1] http://www.asterisk.org/doxygen/CodeGuide.html

By: Jason Parker (jparker) 2006-05-27 02:18:45

Two things I see.

1) You should probably use some of the ast_ memory functions when possible.
2) This line scares me...a lot.  Does it actually compile?
  if (zErr && strcasecmp(zErr, "table %s already exists"), table) {

By: Alejandro Rios P. (alerios) 2006-05-28 15:09:06

>north - manager
>1) You should probably use some of the ast_ memory functions when possible.

Ok, updated the file trying to use this functions (note that this is my first asterisk patch ever).

>2) This line scares me...a lot. Does it actually compile?
  if (zErr && strcasecmp(zErr, "table %s already exists"), table) {

Yes, sorry for that. Its corrected on the new file.

Thanks for the comments.

By: Serge Vecher (serge-v) 2006-06-05 12:47:47

alerios: your latest patch looks like it was accidentally clipped at the top of the code section. Can you please redo the patch. Thanks.

By: Alejandro Rios P. (alerios) 2006-06-06 00:49:00

vechers:  I didn't really saw what you meant by "clipped", but I uploaded the same patch with the first portion of the code without the indentation made by indent.  I hope this is what you meant.  Regards.

By: Serge Vecher (serge-v) 2006-06-09 09:47:16

alerios: never mind, I may need some new glasses here; the unusual formatting tripped me up. Here are a few comments:

1. The following block needs to come before all #include's (look at other files in cdr directory)
#include "asterisk.h"

ASTERISK_FILE_VERSION(__FILE__, "$Revision$")

2. No need for spaces between these lines:
static char *desc = "Customizable SQLite3 CDR Backend";

static char *name = "cdr_sqlite3_custom";

static sqlite3 *db = NULL;

By: Alejandro Rios P. (alerios) 2006-06-12 17:14:17

Hi vechers.

>1. The following block needs to come before all #include's (look at other files >in cdr directory)
>#include "asterisk.h"
>
>ASTERISK_FILE_VERSION(__FILE__, "$Revision$")

Ok, I changed the formating again. In fact, I based my initial code on all other cdr/* files, and none of them has that block of code at the beginning. Anyways, the requested changes are on cdr_sqlite3_custom.c-3

Thanks.

By: Russell Bryant (russell) 2006-06-30 21:37:55

I have never used it, nor am I sure if it still works, but I just noticed that asterisk-addons has res_sqlite3 provides a sqlite3 cdr handler.  I'm also not sure if it supports custom cdr mapping.  I just wanted its presence to be noted here in this issue.

By: jmls (jmls) 2006-10-31 13:34:46.000-0600

is this patch still required / desired ?

By: Alejandro Rios P. (alerios) 2006-10-31 14:34:09.000-0600

>jmls - manager
>10-31-06 13:34
> is this patch still required / desired ?

Yes please!, we even already included it on the asterisk debian package:

http://svn.debian.org/wsvn/pkg-voip/asterisk/trunk/debian/patches/cdr_sqlite3_custom.dpatch?op=file&rev=0&sc=0

By: jmls (jmls) 2006-10-31 15:48:15.000-0600

developers: can we get this into trunk, or is there something stopping it ?

By: Anthony LaMantia (alamantia) 2006-11-06 12:12:28.000-0600

i have commited code into a branch of mine that adds support for sqlite1/2/3, detection for each with configure and modfications to menuselect.

it is located here, if you have any comments/thoughts/improvements please post them here.

the branch can be found here:
view: http://svn.digium.com/view/asterisk/team/anthonyl/sqlite-support/
checkout: http://svn.digium.com/svn/asterisk/team/anthonyl/sqlite-support/

By: Alejandro Rios P. (alerios) 2006-11-07 19:06:43.000-0600

alamantia: I've checked out revision 47303 of that branch and saw that your code does not allow custom cdr fields.   Please remember that the main feature of the module I proppose is that it allows custom cdr fields.

Regards.



By: Anthony LaMantia (alamantia) 2006-11-08 13:34:27.000-0600

I am planning on integrating your custom sqlite code into the existing branch i am working on, so it can support all version of sqlite..  if you have any comments/patches/ideas.. do you mind working on them aginst the branch i posted above?

By: Santiago Ruano Rincón (santiago) 2007-01-25 15:34:55.000-0600

The previous module's version didn't compile

By: Santiago Ruano Rincón (santiago) 2007-01-25 15:36:33.000-0600

why its status was set "new" again?

By: Clod Patry (junky) 2007-02-18 00:21:48.000-0600

id like to push a bit that one, since this one is pretty old.
can we commit it?

By: Russell Bryant (russell) 2007-03-13 16:27:31

This module has been merged into the trunk in revision 58866.  However, I had to make some significant changes to protect against SQL injection attacks.  So, please try the module in trunk and make sure that it still works for you.

Thank you!!