[Home]

Summary:ASTERISK-00442: PGSQL (and MySQL ?) may not log every CDR
Reporter:florian (florian)Labels:
Date Opened:2003-10-28 03:52:33.000-0600Date Closed:2004-09-25 02:49:15
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) cdr_mysql.txt.diff
( 1) postgres_cdr.sql.diff
Description:I noticed after extending the error-output of the pgsql logger that in some cases my channel string is longer than 45 characters. In such cases PGSQL does NOT log the entry.

Same should probably go for MySQL.

The database description is obviously not covering enough :-)

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

I propose making the fields bigger, but the main question is: How long can a channel description become ?
Comments:By: florian (florian) 2003-10-28 04:06:12.000-0600

To demonstrate the point:

INSERT INTO cdr (calldate,clid,src,dst,dcontext,channel,dstchannel,lastapp,lastdata,duration,billsec,disposition,amaflags,accountcode,uniqueid) VALUES ('2003-10-28 10:30:40','','','123456789','inbound', 'Zap/62-1','IAX2[virtupbx]/5','Dial','IAX2/iaxtopbx:mypass1@123.123.123.12/123456789',7,4,'ANSWERED',3,'','1067333433.0')

results in:

Oct 28 10:30:40 ERROR[18451]: File cdr_pgsql.c, Line 127 (pgsql_log): cdr_pgsql: Failed to insert call detail record into database (ERROR:  value too long for type character varying(45)

edited on: 10-28-03 04:03
By: Brian West (bkw918) 2003-10-28 10:31:50.000-0600

can you diff -u and attach?
By: Paul Cadach (pcadach) 2003-10-29 00:23:42.000-0600

MySQL (at least 3.23) will just truncate long strings, so it's PostreSQL-related problem.

mysql> create table aaa (x varchar(10));
Query OK, 0 rows affected (0.02 sec)

mysql> insert into aaa values ('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
Query OK, 1 row affected (0.03 sec)

mysql> select * from aaa;
+------------+
| x          |
+------------+
| aaaaaaaaaa |
+------------+
1 row in set (0.02 sec)

mysql>
By: florian (florian) 2003-10-29 01:53:31.000-0600

Uhm, quite frankly the fact that mysql truncates this data may make it a lot harder to match caller and callee for billing purposes. IMHO, this is baaaaad...
By: nix (nix) 2003-10-29 07:22:05.000-0600

I agree with florian. To silently truncate data is much worse than to throw an error message. Having invalid data in your DB is alot worse than not having it at all. Both my patches above fix the problem anyway, so I don't think we really need to discuss it further :-)
By: Ben Klang (bklang) 2003-11-04 15:53:22.000-0600

This issue is not strictly relating to the channel fields.  I've also had issues with other fields.
I can think of two possible resolutions (these are pgsql specific)
1) Use triggers or an other method to emulate the mysql (and pgsql before version 7.2) behavior
2) either make the fields really long (varchar(100)) or undefined (varchar)

Each has their drawbacks.  In my case, since I'm using custom audio files with reaaaaaly long names, I hit this problem in the lastdata field.
By: Ben Klang (bklang) 2003-11-04 15:55:53.000-0600

oh and in response to nix: I prefer to have partial (not really 'bad' data) than no CDR of the call at all.  At least with partial data, I can still make a bill.  Without any record of the call, I lose the money and the resources spent.  It also has the potential to upset customers (if he's missing a call here, might he be adding calls I didn't make?)

My $0.02.
By: nix (nix) 2003-11-04 20:19:04.000-0600

Yes, you are right, losing billing records is bad, but then the csv file should have them all.
I am of the opinion that a DB should contain Data, on no data, not something that may be data but could be crap because it may have truncated it. Data should never be thrown away unless you ask it to be...
I believe we have a bug that needs to be fixed in the schema for both DB's, not emulate broken MySQL behaviour in Postgres also :-)
I think someone needs to dive into the code and see what is REALLY the max value of these fields in Asterisk. (Not me, its 4am and Im going to bed)
By: Ben Klang (bklang) 2003-11-05 09:15:05.000-0600

In my case, the csv file is volatile (my asterisk servers use diskless root filesystems) and so it is disabled (it wouldn't be useful anyway).  In my case, the DB server is THE authoritative source.  I agree that the field length should be defined in the source (if it isn't already) and fixed in both DB CDR modules.
By: florian (florian) 2003-11-18 02:24:50.000-0600

Okay, so I updated the fields channel and dstchannel like Nix suggested - but this is insufficient. The same issue can occur for instance in the lastdata field (as bklang suggested). So I decided (finally) to make some time free and check whats in cdr.h. This is what I propose and now use in my own PostgreSQL database:

CREATE TABLE cdr (
       AcctId          BIGSERIAL PRIMARY KEY,
       calldate        TIMESTAMP with time zone NOT NULL DEFAULT now(),
       clid            VARCHAR(80) NOT NULL default '',
       src             VARCHAR(80) NOT NULL default '',
       dst             VARCHAR(80) NOT NULL default '',
       dcontext        VARCHAR(80) NOT NULL default '',
       channel         VARCHAR(80) NOT NULL default '',
       dstchannel      VARCHAR(80) NOT NULL default '',
       lastapp         VARCHAR(80) NOT NULL default '',
       lastdata        VARCHAR(80) NOT NULL default '',
       duration        INTEGER NOT NULL default '0',
       billsec         INTEGER NOT NULL default '0',
       disposition     VARCHAR(32) NOT NULL default '',
       amaflags        INTEGER NOT NULL default '0',
       accountcode     VARCHAR(20) NOT NULL default '',
       uniqueid        VARCHAR(32) NOT NULL default ''
);

Ofcourse this structure size should also be applied to the MySQL table.

edited on: 11-18-03 02:24

edited on: 11-18-03 02:25
By: Brian West (bkw918) 2003-11-20 12:52:49.000-0600

nix please attach the diffs.  

Thanks,
Brian
By: nix (nix) 2003-12-01 09:19:18.000-0600

OK here are the patches for Postgres and MySQL with the correct field lengths as per cdr.h

Does anyone know why all the fields are set NOT NULL default ''
This seems kind of stupid to me. Why not just leave NULL data as NULL? Is there some frontend to this table that relies on not getting NULL data from the DB?

edited on: 12-01-03 09:23

edited on: 12-01-03 09:25
By: Brian West (bkw918) 2003-12-02 14:05:17.000-0600

That still doesn't correct the issue.  It just makes it go away for now.  

char *tmp[100];

if(strlen(cdr->uniqueid) > 46)
{
   memset(tmp,0,100);
   strncpy(tmp, cdr->uniqueid, 45);
   strcpy(cdr->uniqueid, tmp);
}

Granted this isn't tested.. but something like this is what is needed to fix this properly.

If we fix it.. we fix it right.  No half ass fix.
By: nix (nix) 2003-12-02 14:30:08.000-0600

ok. Well there are 2 issues. One is that the SQL field lengths do not match the length of the variables in the code.
My patch fixes that.

As for checking the length of data so you don't have internal buffer overflows, that should obviously be done also if it is not already. I have not yet looked into that. By all means you or someone else should check this also. Regardless, my patch to the default sql schemas should go in..

Is there some issue I am not seeing here?
By: Brian West (bkw918) 2003-12-02 17:23:30.000-0600

These are in CVS... But I would still like to revisit this issue and try to provide a better fix for this.